diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9fc3ed9..f833bab 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -58,6 +58,14 @@ jobs: # nobody has to act on is not a gate, and the repository already fixes # these one way — a `pnpm.overrides` entry — which is a reviewable diff. # Read what that fix does and does not do in ADR-0021 before adding one. + # + # `moderate`, not `high`, and the reason is GitHub rather than severity: + # Dependabot alerts on moderate, so a gate that started at `high` left + # every moderate advisory to accumulate in the security tab where nobody + # is obliged to look and nothing turns red. That is what the `hono` ReDoS + # did for days. Aligning the floor with the alerting cost nothing at the + # time — both trees audit clean at `low` — and the point is that the next + # one fails a pull request instead of waiting to be noticed. runs-on: ubuntu-latest steps: - uses: actions/checkout@v7 @@ -67,7 +75,7 @@ jobs: node-version: 22 cache: pnpm - run: pnpm install --frozen-lockfile - - run: pnpm audit --audit-level=high + - run: pnpm audit --audit-level=moderate # And the tree a consumer actually resolves, which the line above # cannot see: root `pnpm.overrides` rewrite resolution in this # workspace only, so an override silences the gate here and reaches diff --git a/.project/cards/T-0222-the-hono-advisory-rides-shadcn-s-mcp-sdk-into-ever.md b/.project/cards/T-0222-the-hono-advisory-rides-shadcn-s-mcp-sdk-into-ever.md new file mode 100644 index 0000000..416c749 --- /dev/null +++ b/.project/cards/T-0222-the-hono-advisory-rides-shadcn-s-mcp-sdk-into-ever.md @@ -0,0 +1,64 @@ +--- +id: T-0222 +title: The hono advisory rides shadcn's MCP SDK into every install +status: done +type: task +priority: low +area: infra +tags: [security] +scope: [package.json, .github/workflows, scripts/audit-consumer.ts] +origin: [T-0148] +created: 2026-08-07 +updated: 2026-08-07 +related: [ADR-0021, T-0148, T-0221] +verified: + at: "2026-08-07T18:39:22.286Z" + method: local + commit: 11ad498e7e3d622e51a16cdd36a50bffd820e461 + digest: "sha256:44048dbe894ec211d85b7332159e970a3337c4e4a598a79437f8e6ddc09405a6" +--- + +GHSA-8j4g-w8fx-2239, ReDoS in hono's CORS middleware via +`Access-Control-Request-Headers`, moderate, patched in 4.12.34. Dependabot alert 4 +on the default branch, and it had been sitting there for days. + +The path is a single one: `shadcn` → `@modelcontextprotocol/sdk` → +`@hono/node-server` → `hono`, and `shadcn` is a devDependency of +`@illodev/workfile`. Nothing published reaches it, so it is the honest kind of +override — the same class as `fast-uri` and `js-yaml`, and the opposite of the +`sharp` and `adm-zip` entries T-0221 had to remove. + +**The better fix was checked first and does not exist.** `docs/ui.md` teaches +`pnpm dlx shadcn@latest add `, no script invokes a local `shadcn` +binary, and removing the devDependency would have taken `hono` out of the graph +along with one of two `fast-uri` paths and one `js-yaml` path. But +`ui/src/styles.css` has `@import "shadcn/tailwind.css"`, which resolves to +`shadcn/dist/tailwind.css` and carries the `scroll-fade-*` utilities the Boards, +Memory and attachment components use. The package is load-bearing at build time, +not just tooling. + +**And the reason it kept bothering somebody rather than being caught.** The audit +gate ran at `--audit-level=high` while Dependabot alerts at `moderate`. So this +advisory could never turn anything red: it went to the security tab and stayed +there. A gate whose floor is above the floor of something that already reports +guarantees a backlog somewhere nobody is obliged to look. Both audits now block at +`moderate`, recorded in ADR-0021. + +## Acceptance criteria + +- [x] `pnpm audit` reports nothing at moderate or above. +- [x] The consumer tree gate applies the same floor. +- [x] The threshold change is recorded, with what it will cost. + +## Activity + +- 2026-08-07 18:36Z illodev@local#42eb42f5 · claimed +- 2026-08-07 18:39Z illodev@local#42eb42f5 · released + +## Notes + +- 2026-08-07 18:39Z illodev@local#42eb42f5 — Overridden hono to ^4.12.34; it resolves to 4.13.1 and pnpm audit now reports nothing at any level, low included. +Checked the better fix first and it does not exist. docs/ui.md teaches pnpm dlx shadcn@latest add, no script invokes a local shadcn binary, and dropping the devDependency would have removed hono from the graph entirely plus one of two fast-uri paths and one js-yaml path. But ui/src/styles.css imports shadcn/tailwind.css, which resolves to shadcn/dist/tailwind.css and carries the scroll-fade utilities Boards, Memory and attachment.tsx use — I confirmed the rules are in the built CSS. The package is load-bearing at build time, so the override is the right answer here rather than the lazy one. +The systemic half is the threshold. The gate ran at high while Dependabot alerts at moderate, so this advisory could not turn anything red and went to the security tab instead — which is the actual reason it went unaddressed for days rather than anything about hono. Both audits now block at moderate. It cost nothing at the time because both trees were clean at low, and the cost to expect is stated in ADR-0021 and CHG-0150: an advisory against a transitive devDependency will now turn unrelated pull requests red until an override lands. That is the trade, and it is the same one the no-allowlist posture already made. +Also corrected scripts/audit-consumer.ts, whose header still described the sharp and adm-zip overrides as present after T-0221 removed them. +- 2026-08-07 18:39Z illodev@local#42eb42f5 — local verification: pnpm audit reports no known vulnerabilities at --audit-level=moderate and at --audit-level=low, from a clean install with the hono override. pnpm why hono resolves 4.13.1 through the single shadcn > MCP SDK > @hono/node-server path. pnpm run audit:consumer clean at moderate and above with nothing below it. pnpm run check green: 465 + 10 tests pass, strictNullChecks held at 488. The shadcn removal alternative was ruled out by evidence, not assumption: the scroll-fade rules it supplies via shadcn/dist/tailwind.css are present in the built CSS. diff --git a/.project/changelog/unreleased/CHG-0150-the-audit-gates-block-at-moderate-the-floor-dependabot-already-alerts-.md b/.project/changelog/unreleased/CHG-0150-the-audit-gates-block-at-moderate-the-floor-dependabot-already-alerts-.md new file mode 100644 index 0000000..e0771bd --- /dev/null +++ b/.project/changelog/unreleased/CHG-0150-the-audit-gates-block-at-moderate-the-floor-dependabot-already-alerts-.md @@ -0,0 +1,18 @@ +--- +id: CHG-0150 +title: The audit gates block at moderate, the floor Dependabot already alerts on +type: changed +area: infra +visibility: internal +cards: [T-0222] +decisions: [ADR-0021] +tags: [security, ci] +created: 2026-08-07 +updated: 2026-08-07 +--- + +Both supply-chain gates — `pnpm audit` on the workspace and the consumer-tree audit — now fail at `moderate` instead of `high`. + +The old floor was the release gate's, which looked consistent and was not. Dependabot alerts at `moderate`, so a moderate advisory could never turn a build red: it went to the security tab and stayed there. GHSA-8j4g-w8fx-2239 in `hono` did exactly that for days before anyone acted on it. + +It cost nothing to change — both trees audited clean at `low` at the time — and what it buys is that the next one fails a pull request rather than waiting to be noticed. What it will cost is real and worth expecting: an advisory published against a transitive devDependency now turns unrelated pull requests red until an override lands. diff --git a/.project/memory/decisions/ADR-0021-a-supply-chain-gate-runs-on-pull-requests-and-audits-the-tree-a-consum.md b/.project/memory/decisions/ADR-0021-a-supply-chain-gate-runs-on-pull-requests-and-audits-the-tree-a-consum.md index ea6dce9..5aab7c7 100644 --- a/.project/memory/decisions/ADR-0021-a-supply-chain-gate-runs-on-pull-requests-and-audits-the-tree-a-consum.md +++ b/.project/memory/decisions/ADR-0021-a-supply-chain-gate-runs-on-pull-requests-and-audits-the-tree-a-consum.md @@ -45,8 +45,8 @@ and pins `sharp: ^0.34.5`. One policy for both gates, and a third gate that neither card asked for. -**Gates run on pull requests.** `pnpm audit --audit-level=high` is now its own job -in `ci.yml`, blocking, at the same threshold as the release gate. An advisory the +**Gates run on pull requests.** `pnpm audit --audit-level=moderate` is now its own +job in `ci.yml`, blocking, at the same threshold as the release gate. An advisory the ecosystem publishes will turn an unrelated pull request red through no fault of its author; that is the cost, and it buys a re-run instead of a retag. A non-blocking version was rejected: a warning nobody must act on is not a gate, and @@ -92,6 +92,16 @@ removed with it — they had become entries pinning packages no longer in the gr Worth keeping in view: a baseline would have made this a line on a list, and the list would have been the answer for as long as nobody looked at it. +**The floor is Dependabot's, not ours** (added for T-0222). It started at `high`, +which is the release gate's threshold and looked like the consistent choice. It was +not: Dependabot alerts on `moderate`, so every moderate advisory went to the +security tab, where nothing turns red and nobody is obliged to look — the `hono` +ReDoS sat there for days. A gate whose floor is above the floor of something that +already reports is a gate that guarantees a backlog somewhere else. Both audits now +block at `moderate`. It cost nothing when it was changed, because both trees were +clean at `low`, and the point is not today's reading: it is that the next moderate +fails a pull request instead of waiting to be noticed. + An override is now understood as two different things depending on where the overridden package sits. Under a devDependency it is a real fix for the only tree that matters — nothing ships. Under a published package's `dependencies` it is a diff --git a/package.json b/package.json index a7d25a7..67c0c9e 100644 --- a/package.json +++ b/package.json @@ -31,7 +31,7 @@ "check": "pnpm run build && pnpm run build:plugin && pnpm run strict && pnpm run test", "smoke:package": "pnpm --filter @illodev/workfile run smoke:package", "audit:consumer": "node ./scripts/audit-consumer.ts", - "check:release": "pnpm run check && pnpm audit --audit-level=high && pnpm run audit:consumer && pnpm run smoke:package", + "check:release": "pnpm run check && pnpm audit --audit-level=moderate && pnpm run audit:consumer && pnpm run smoke:package", "bench": "pnpm --filter @illodev/workfile run bench", "screenshots": "pnpm run build && node ./scripts/screenshots.ts", "doctor": "node packages/workfile/dist/bin/workfile.js doctor", @@ -55,6 +55,7 @@ "pnpm": { "overrides": { "fast-uri": "^3.1.5", + "hono": "^4.12.34", "js-yaml": "^4.3.1" } } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index b0392ce..a9378ae 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -6,6 +6,7 @@ settings: overrides: fast-uri: ^3.1.5 + hono: ^4.12.34 js-yaml: ^4.3.1 importers: @@ -355,7 +356,7 @@ packages: resolution: {integrity: sha512-eWpQYr67tqJLeaSUl0Q+TquuYfUdTibpOJlUMV2FfUP7+KqCC5TufnwnlXL6mobZBJbGAYRd7ZvEBDCbLInjhg==} engines: {node: '>=20'} peerDependencies: - hono: ^4 + hono: ^4.12.34 '@huggingface/tokenizers@0.1.3': resolution: {integrity: sha512-8rF/RRT10u+kn7YuUbUg0OF30K8rjTc78aHpxT+qJ1uWSqxT1MHi8+9ltwYfkFYJzT/oS+qw3JVfHtNMGAdqyA==} @@ -1988,8 +1989,8 @@ packages: resolution: {integrity: sha512-T2UbfbBEF32wiepXIsMlTW9+dDYC6wMh/t/vYA4tuOMKqWz/n3vr1NFSxQiyP+zk2mXsoMA/i/7qV6LKut1t1A==} engines: {node: '>= 0.4'} - hono@4.12.33: - resolution: {integrity: sha512-+SwvkaiJtxsiPjhy9LivY/1m7UsNqCJetM1BrZl9A5DkQhlbHQDU730mMiDPWjnoCYOM8Chf3WrCJw27kNTPFQ==} + hono@4.13.1: + resolution: {integrity: sha512-kdJoFVv2xmayw6cY09H7AbMJMt8Jn5jdlEdXsP7AGBdF2DIptVlKlOLKXP41yPip4/a3yQPv9gVcJYI8YY04dw==} engines: {node: '>=16.9.0'} http-errors@2.0.1: @@ -3366,9 +3367,9 @@ snapshots: '@fontsource-variable/geist@5.3.0': {} - '@hono/node-server@2.0.12(hono@4.12.33)': + '@hono/node-server@2.0.12(hono@4.13.1)': dependencies: - hono: 4.12.33 + hono: 4.13.1 '@huggingface/tokenizers@0.1.3': {} @@ -3393,7 +3394,7 @@ snapshots: '@modelcontextprotocol/sdk@1.30.0(zod@3.25.76)': dependencies: - '@hono/node-server': 2.0.12(hono@4.12.33) + '@hono/node-server': 2.0.12(hono@4.13.1) ajv: 8.20.0 ajv-formats: 3.0.1(ajv@8.20.0) content-type: 1.0.5 @@ -3403,7 +3404,7 @@ snapshots: eventsource-parser: 3.1.0 express: 5.2.1 express-rate-limit: 8.6.1(express@5.2.1) - hono: 4.12.33 + hono: 4.13.1 jose: 6.2.5 json-schema-typed: 8.0.2 pkce-challenge: 5.0.1 @@ -4902,7 +4903,7 @@ snapshots: dependencies: function-bind: 1.1.2 - hono@4.12.33: {} + hono@4.13.1: {} http-errors@2.0.1: dependencies: diff --git a/scripts/audit-consumer.ts b/scripts/audit-consumer.ts index 0da2272..0437b71 100644 --- a/scripts/audit-consumer.ts +++ b/scripts/audit-consumer.ts @@ -4,12 +4,13 @@ * * `pnpm audit` audits this workspace. The workspace has `pnpm.overrides`, and * overrides are a workspace-install mechanism: they rewrite resolution here and - * do not travel inside a published package. So the release gate could read zero - * high advisories while somebody running `npm i @illodev/workfile-search-local` - * resolved several — which is exactly what it did. Two of the four overrides, - * `sharp` and `adm-zip`, sit under `@huggingface/transformers`, a `dependencies` - * entry of that published package, and the overrides fixed them for nobody but - * us. + * do not travel inside a published package. So the release gate read zero high + * advisories for months while somebody running + * `npm i @illodev/workfile-search-local` resolved four. Two overrides — + * `sharp` and `adm-zip` — sat under `@huggingface/transformers`, a + * `dependencies` entry of that published package, and fixed them for nobody but + * us. T-0221 removed the dependency and those two overrides with it; what stays + * is the reason this file exists. * * This resolves what the *manifests* declare instead. The consumer manifest is * the union of every publishable package's `dependencies`, so it describes this @@ -17,7 +18,7 @@ * the overrides because it is a different install root. * * `--package-lock-only` resolves without downloading, so this costs a few - * seconds and no binaries — `sharp` and `onnxruntime-node` would otherwise pull + * seconds and no binaries — the tree it was written against would otherwise pull * platform builds worth hundreds of megabytes to tell us something the lockfile * already knows. * @@ -41,7 +42,13 @@ import { promisify } from "node:util"; const run = promisify(execFile); const repoRoot = fileURLToPath(new URL("..", import.meta.url)); -const THRESHOLD = new Set(["high", "critical"]); +/** + * Aligned with Dependabot's alerting floor rather than with a sense of what is + * serious. A gate above the floor something else already reports leaves those + * advisories to pile up where nobody must look — see the `hono` note in + * `.github/workflows/ci.yml`. + */ +const THRESHOLD = new Set(["moderate", "high", "critical"]); async function publishableDependencies() { const packagesDir = join(repoRoot, "packages"); @@ -140,7 +147,7 @@ try { if (!blocking.length) { const counts = report.metadata?.vulnerabilities || {}; console.log( - `\nConsumer tree clean at high and above. Below the threshold: ${ + `\nConsumer tree clean at moderate and above. Below the threshold: ${ Object.entries(counts) .filter(([level, count]) => level !== "total" && Number(count) > 0) .map(([level, count]) => `${count} ${level}`) @@ -151,7 +158,7 @@ try { } console.error( - `\n${blocking.length} package(s) at high or above in the tree a consumer resolves:\n` + `\n${blocking.length} package(s) at moderate or above in the tree a consumer resolves:\n` ); for (const entry of blocking.sort((left, right) => String(left.name).localeCompare(String(right.name))