Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .changeset/nine-falcons-fix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
---

Add copilot instructions and fix-audit skill
144 changes: 144 additions & 0 deletions .github/copilot-instructions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
# GitHub Copilot Instructions — iTwin Saved Views

## Project Overview

This repository publishes two iTwin Platform npm packages — [`@itwin/saved-views-client`](https://www.npmjs.com/package/@itwin/saved-views-client) and [`@itwin/saved-views-react`](https://www.npmjs.com/package/@itwin/saved-views-react) — along with private test applications used for local development. See the [README](../README.md) for a full project overview.

## Development Setup

```shell
# Install all workspace dependencies (pnpm required; npm is blocked)
npx pnpm install

# Start the test app (frontend on http://localhost:7948 + backend, with hot reload)
pnpm start
```

To enable iTwin Platform features in the test app, configure [`packages/test-app-frontend/.env`](../packages/test-app-frontend/.env).

> **Running commands on Windows**: The default PowerShell execution policy blocks npm scripts. Use either `cmd /c "cd /d <repo-root> && <command>"` or `powershell -ExecutionPolicy Bypass -Command "<command>"`. The `cmd /c` form is preferred — it requires no policy flag and stdout/stderr are both captured cleanly when you append `2>&1`.

## Available Commands

All commands run from the **repository root** unless otherwise noted.

| Command | Description |
|---|---|
| `pnpm start` | Start both test apps concurrently with hot reload |
| `pnpm run build` | Build all published packages and test app frontend (parallel) |
| `pnpm test` | Run all tests (Vitest) |
| `pnpm run cover` | Run tests and generate a coverage report |
| `pnpm run lint` | Run ESLint on all `src/` and `test/` TypeScript files (CI enforces `--max-warnings 0`) |
| `pnpm run typecheck` | Type-check all packages (parallel) |
| `pnpm audit` | Check all dependencies for known vulnerabilities |
| `pnpm changeset` | Add a changeset for a public API or behavior change |
| `pnpm changeset --empty` | Add an empty changeset for docs or internal-only changes |

## Package Architecture

| Package | Published | Purpose |
|---|---|---|
| [`saved-views-client`](../packages/saved-views-client/) | ✅ `@itwin/saved-views-client` | TypeScript client for the iTwin Platform Saved Views REST API. Dual ESM + CJS output. |
| [`saved-views-react`](../packages/saved-views-react/) | ✅ `@itwin/saved-views-react` | React 18 components and utilities (`SavedViewTile`, `TileGrid`, `captureSavedViewData`, `applySavedView`). ESM only. |
| [`test-app-frontend`](../packages/test-app-frontend/) | ❌ private | Vite + React dev/demo app. Runs on `http://localhost:7948`. |
| [`test-app-backend`](../packages/test-app-backend/) | ❌ private | Express + iTwin iModels backend supporting the test app. |

## Code Style & Conventions

### Copyright Header

Every source file must begin with this header:

```ts
/*---------------------------------------------------------------------------------------------
* Copyright (c) Bentley Systems, Incorporated. All rights reserved.
* See LICENSE.md in the project root for license terms and full copyright notice.
*--------------------------------------------------------------------------------------------*/
```

### File Naming

- **React component files**: PascalCase — `SavedViewTile.tsx`, `SavedViewTileContext.tsx`
- **Utility, hook, and function files**: camelCase — `applySavedView.ts`, `useSavedViews.tsx`, `utils.ts`
- **Class files**: PascalCase — `ITwinSavedViewsClient.ts`
- **Model and type files**: PascalCase — `SavedView.ts`
- **Folders containing a component**: PascalCase — `SavedViewTile/`, `TileGrid/`
- **Folders for utilities or groupings**: camelCase — `translation/`
- **Barrel files**: always `index.ts`

### Naming Conventions

- **Components, classes, types, interfaces, enums**: PascalCase
- **Interface names do NOT use an `I` prefix** — use `SavedViewTileProps`, not `ISavedViewTileProps`
- **Functions, variables, React hooks**: camelCase
- **Module-level constants**: camelCase — `modelClipGroupMappings`, not `MODEL_CLIP_GROUP_MAPPINGS`
- **Enum members**: PascalCase — `RenderMode.SmoothShade`
- **Unused parameters**: prefix with `_` — `_unused`

### Import Conventions

- Use `import type { ... }` for all type-only imports:
```ts
import type { SavedView, SavedViewTag } from "../SavedView.js";
```
- All internal imports use the **`.js` extension** even though source files are `.ts`/`.tsx`:
```ts
import { SavedViewTile } from "./SavedViewTile.js";
```
- Use `export type { ... }` when re-exporting types from barrel files.

### React Component Style

- Exported components use **named function declarations**, not arrow functions:
```ts
export function SavedViewTile(props: SavedViewTileProps): ReactElement { ... }
```
- Always specify **explicit return types** on exported functions.
- Define the props interface directly above the component it belongs to.
- Document public exported APIs and component props with JSDoc `/** */` comments.

### ESLint Rules Agents Must Not Violate

The following are the most common rules that can produce lint failures. CI enforces **zero warnings**:

- Prefix unused variables and parameters with `_`
- Always use `===` / `!==`
- Trailing commas required on multiline expressions and parameter lists
- Single quotes for strings
- Never use `eval()` — it is an error
- Avoid non-null assertions (`!`) — they are a warning; only freely allowed in test files
- Handle all members in `switch` statements over enums and union types (`switch-exhaustiveness-check`)

## Testing

Tests use [Vitest](https://vitest.dev/) with the `happy-dom` environment for DOM simulation. Each package has its own `vitest.config.ts`.

- **Test files**: co-located with source or in a `test/` directory, named `*.test.ts` / `*.test.tsx`
- **Run all tests**: `pnpm test` (from repo root)
- **Run tests for one package**: `cd packages/saved-views-react && pnpm run test:cover`
- **Coverage**: `pnpm run cover` (uses `@vitest/coverage-v8`)
- Test files relax `any` and non-null assertion lint rules — these relaxations must not be used in production code

## Versioning with Changesets

This repository uses [Changesets](https://github.com/changesets/changesets) to manage versioning and changelogs. See the [README](../README.md#versioning-with-changesets) for the full workflow.

- **Public API or behavior change** → `pnpm changeset` (select semver bump and describe the change)
- **Docs-only or internal-only change** → `pnpm changeset --empty`
- Every PR to main must include a changeset; CI will fail without one

---

## MANDATORY: Pre-commit Checklist

**CRITICAL**: Before finishing any task, you MUST run all four commands below in order and fix every error and warning before considering the work done. Do not skip steps.

```shell
pnpm run build # Must produce zero compiler errors
pnpm test # Must produce zero failing tests
pnpm run lint # Must produce zero errors AND zero warnings
pnpm run typecheck # Must produce zero type errors
pnpm audit # Must produce zero high/critical vulnerabilities
```

**NO EXCEPTIONS**: Code that does not build, has failing tests, or produces lint/type errors must never be committed.
161 changes: 161 additions & 0 deletions .github/skills/fix-audit-vulnerabilities/SKILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,161 @@
---
name: fix-audit-vulnerabilities
description: 'Identify and fix high-severity npm/pnpm security vulnerabilities in the saved-views monorepo. Use when asked to "fix audit vulnerabilities", "run pnpm audit", "update audit", "fix security issues", "address CVEs", or when security advisories need to be resolved. Runs pnpm audit --audit-level high, diagnoses affected packages, applies dependency overrides or upgrades, then verifies fixes with build and tests.'
---

# Fix Audit Vulnerabilities

Workflow for identifying, fixing, and verifying high-severity security vulnerabilities in the saved-views pnpm monorepo.

## When to Use This Skill

- User asks to "fix audit vulnerabilities" or "run pnpm audit"
- User mentions "security issues", "CVEs", or "dependency vulnerabilities"
- CI audit check is failing
- User wants to "update audit" or "resolve security advisories"

## Prerequisites

- pnpm >= 10 installed
- Node >= 20
- Run commands from the **repository root** (`e:\saved-views_1\saved-views\`)
- On Windows, prefix commands with `cmd /c "cd /d <repo-root> && <command> 2>&1"`

## Step-by-Step Workflow

### Step 1: Run the Audit

Run [run-audit.ps1](./scripts/run-audit.ps1) or execute directly:

```powershell
Set-ExecutionPolicy -ExecutionPolicy RemoteSigned -Scope Process
cd e:\saved-views_1\saved-views
pnpm audit 2>&1
```

> Note: Run without `--audit-level high` so that moderate vulnerabilities are also visible — they may be fixed just as easily.

Read the output carefully. Note:
- **Package name** with the vulnerability
- **Severity** (high or critical)
- **Via** chain (which of your direct deps pulled it in)
- **Fix available** — whether a non-breaking fix exists

### Step 2: Diagnose the Affected Packages

For each vulnerable package, determine which workspace package(s) depend on it:

```powershell
pnpm why <vulnerable-package>
```

> **Caveat:** `pnpm why` only searches the current workspace root, not sub-packages. If it returns nothing, the package is still likely installed as a transitive dep — confirm by checking `pnpm-lock.yaml` directly:
> ```powershell
> Select-String -Path pnpm-lock.yaml -Pattern "^ <vulnerable-package>@"
> ```

Check the sub-package `package.json` files in:
- `packages/saved-views-client/package.json`
- `packages/saved-views-react/package.json`
- `packages/test-app-frontend/package.json`
- `packages/test-app-backend/package.json`
- Root `package.json`

### Step 3: Choose a Fix Strategy

Consult [audit-fix-guide.md](./references/audit-fix-guide.md) for strategy details.

| Situation | Strategy |
|-----------|----------|
| Direct dependency, non-breaking update available | Bump version in the relevant `package.json` |
| Transitive dependency, fix available | Add `pnpm.overrides` in root `package.json` |
| No fix available yet | Add `pnpm.overrides` to pin to the least-vulnerable version |
| Breaking major version bump needed | Update code for compatibility before bumping |

### Step 4: Apply the Fix

**Option A — Bump a direct dependency:**
Edit the relevant `package.json` and update the version range, then:
```powershell
cmd /c "cd /d e:\saved-views_1\saved-views && pnpm install 2>&1"
```

**Option B — Add/update a pnpm override** (root `package.json`):
```json
{
"pnpm": {
"overrides": {
"vulnerable-package@<fixed-version": ">=fixed-version"
}
}
}
```
Then run `pnpm install`.

> **Override key syntax:** Always use comparison-operator ranges on the key (e.g. `"pkg@<1.2.3"`, `"pkg@>=4.0.0 <5.0.5"`) rather than caret/tilde ranges (e.g. `"pkg@^4"`). Caret/tilde ranges in keys may not match correctly.
>
> **Override value syntax:** Use `">=fixed-version"` (not a pinned exact version) so future patches are still resolved. For the value, `>=` works correctly even though it looks unbounded — pnpm resolves the minimum satisfying version.
>
> **Scope overrides precisely:** If a vulnerability only affects one major version range, restrict the key accordingly (e.g. `"picomatch@>=4.0.0 <4.0.4": "4.0.4"`) rather than a bare `"picomatch"` which would force all consumers, including those on a safe `^2.x` range, to use the overridden version.

### Step 5: Consider Deleting the Lockfile for a Fresh Resolve

If some overrides don't seem to take effect, delete `pnpm-lock.yaml` and re-install to force a full dependency re-resolution:
```powershell
Remove-Item pnpm-lock.yaml
pnpm install
```
This ensures all overrides are applied from scratch rather than reusing cached resolutions.

### Step 6: Verify the Fix

Re-run the audit to confirm vulnerabilities are resolved:
```powershell
pnpm audit 2>&1
```

### Step 7: Try to Reduce Overrides

Before finishing, check whether any override can be eliminated by bumping a direct dependency instead. For each override, trace the chain with `pnpm why` and check if the direct dep that pulls it in has a newer version with the vulnerability fixed:

```powershell
pnpm view <direct-dep>@latest dependencies --json
```

Removing an override is always preferable to keeping one — it means the fix is self-maintaining.

### Step 8: Confirm Nothing is Broken

Run build, tests, and lint to make sure the fix didn't break anything:
```powershell
pnpm run build 2>&1
pnpm test 2>&1
pnpm run lint -- --max-warnings 0 2>&1
```

> **Warning — ESLint plugin upgrades:** If you upgraded `@typescript-eslint/eslint-plugin` or `@typescript-eslint/parser` as part of the fix, major version bumps (e.g. v7 → v8) can remove or rename rules, causing lint to break even if audit passes. Common v8 breakages:
> - Formatting rules removed: `@typescript-eslint/comma-dangle`, `@typescript-eslint/quotes`, `@typescript-eslint/member-delimiter-style` → replace with base eslint equivalents (`comma-dangle`, `quotes`) and drop `member-delimiter-style`
> - `@typescript-eslint/ban-types` removed → use `@typescript-eslint/no-empty-object-type` or disable it
> - `no-unused-expressions` default changed → add `["error", { "allowShortCircuit": true, "allowTernary": true }]` to restore previous behavior
> - New rules added to `recommended-type-checked` may flag existing code (e.g. `no-duplicate-type-constituents`, `only-throw-error`)
>
> Always run lint immediately after any `@typescript-eslint` upgrade and before considering the fix complete.

If type errors appear, run typecheck for details:
```powershell
pnpm run typecheck 2>&1
```

## Important Notes

- **Never use `npm`** — this project blocks npm (`"npm": "<0"` in engines). Always use `pnpm`.
- **pnpm overrides** live under the `"pnpm"` key in the root `package.json`, not `"resolutions"`.
- After editing any `package.json`, always run `pnpm install` to regenerate `pnpm-lock.yaml`.
- If a fix requires a major version bump of a published package (`saved-views-client` or `saved-views-react`), check for API breaking changes before applying.

## References

- [Audit Fix Strategy Guide](./references/audit-fix-guide.md)
- [Run Audit Script](./scripts/run-audit.ps1)
- [pnpm audit docs](https://pnpm.io/cli/audit)
- [pnpm overrides docs](https://pnpm.io/package_json#pnpmoverrides)
Loading
Loading