Security audit follow-up: resolve all deferred items

.github/workflows/ci.yml

@@ -13,7 +13,7 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        node-version: ['20', '22']
+        node-version: ['22', '24']
     steps:
       - uses: actions/checkout@v4
 
@@ -32,6 +32,14 @@ jobs:
       - name: Test
         run: npm test
 
+      # shorthand/ is a workspace member (not a standalone install like
+      # packages/docs) but its own test suite — compaction, CRDT, and
+      # importance-scoring, ~260 specs — was never run by root `npm test`,
+      # since the root vitest.config.ts only globs the root src/. Run it
+      # explicitly so this coverage is actually checked in CI.
+      - name: Test shorthand
+        run: npm test --workspace=shorthand
+
       - name: Build core
         run: npm run build
 
@@ -47,7 +55,7 @@ jobs:
       - name: Set up Node.js
         uses: actions/setup-node@v4
         with:
-          node-version: '20'
+          node-version: '22'
           cache: npm
 
       - name: Install dependencies
@@ -71,7 +79,7 @@ jobs:
       - name: Set up Node.js
         uses: actions/setup-node@v4
         with:
-          node-version: '20'
+          node-version: '22'
           cache: npm
           cache-dependency-path: packages/docs/package-lock.json
 

AUDIT.md

@@ -70,8 +70,12 @@ rationale (see [Deferred / Not Fixed](#deferred--not-fixed-and-why)).
 
 ## Deferred / Not Fixed (and why)
 
-A few real findings were deliberately **not** acted on in this pass, each for a
-specific reason rather than oversight:
+A few real findings were deliberately **not** acted on in the first pass,
+each for a specific reason rather than oversight. All five were revisited
+in a follow-up pass and are now resolved — see
+[Follow-up](#follow-up-deferred-items-addressed-second-pass) — but the
+original reasoning for deferring each is kept below since it's still the
+right record of *why* they weren't folded into the first PR:
 
 - **`ToolClass`/`ToolProxy` importing MCP transport code (#13).** This is a
   genuine layering violation — the "durable, transport-agnostic" Tier-1
@@ -81,31 +85,127 @@ specific reason rather than oversight:
   directly, which is a real (if not huge) refactor touching a load-bearing
   class. Per this audit's brief to prefer small, reviewable, non-speculative
   changes, this is better done as its own scoped PR with its own review,
-  not folded into a security-audit branch.
+  not folded into a security-audit branch. **Update: done, see
+  [Follow-up](#follow-up-deferred-items-addressed-second-pass).**
 - **`commander`/`better-sqlite3` major bumps (#14).** Both now require
   Node ≥22, which would silently raise this project's declared
   `engines.node: >=20.0.0` floor — a real breaking change for any consumer
   still on Node 20, and one this audit isn't positioned to decide on the
   maintainer's behalf. The CI workflow added in this pass tests both Node 20
   and 22, so whenever that floor decision is made, CI will immediately
-  surface any fallout.
+  surface any fallout. **Update: maintainer chose to raise the floor, see
+  [Follow-up](#follow-up-deferred-items-addressed-second-pass).**
 - **`packages/docs`'s remaining 22 `npm audit` findings.** All require
   forcing `@docusaurus/preset-classic` to `3.10.2` while `@docusaurus/core`
   stays pinned at `3.6.3` (a mismatched, out-of-range pair) — not safe to
   take piecemeal. Needs a coordinated bump of all four `@docusaurus/*`
   packages together, verified with a real `docusaurus build`, as its own
-  change.
+  change. **Update: done, see [Follow-up](#follow-up-deferred-items-addressed-second-pass).**
 - **Stray nested lockfiles in `shorthand/` and `packages/examples/` (#15).**
   Both were touched as recently as the "adopt workspaces" commit, meaning
   someone deliberately kept them post-migration — plausibly because
   `shorthand` is also published standalone as `@shorthand/core` outside this
   monorepo and needs its own lockfile for that independent pipeline. Removing
   them without confirming that intent risked breaking a workflow this audit
-  can't see from the repo alone.
+  can't see from the repo alone. **Update: investigated and resolved, see
+  [Follow-up](#follow-up-deferred-items-addressed-second-pass).**
 - **`src/importance/` vs `shorthand/src/importance/` fork (#21).** Already
   identified and scoped in `docs/ecosystem/engineering-guide.md` as a
   "Phase 0" item with its own analysis. Redoing that analysis here would
-  duplicate, not add to, existing tracked work.
+  duplicate, not add to, existing tracked work. **Update: done, see
+  [Follow-up](#follow-up-deferred-items-addressed-second-pass).**
+
+## Follow-up: Deferred Items Addressed (second pass)
+
+The five items in [Deferred / Not Fixed](#deferred--not-fixed-and-why) above
+were revisited in a follow-up pass after the first PR merged:
+
+- **Stray nested lockfiles (#15) — resolved.** Investigated both open
+  questions directly: `@shorthand/core` is not published to npm at all
+  (registry 404), and `@smallchat/examples`, while published, doesn't need a
+  lockfile for that (publishing doesn't consume one). More importantly,
+  `npm install` run inside either directory doesn't update its local
+  lockfile — npm defers to the workspace root — so neither file was being
+  maintained by any normal workflow. This had already caused real drift:
+  `shorthand/package-lock.json` still resolved `vitest@3.2.4`, the exact
+  version with the critical CVE fixed at the root in the first pass,
+  invisible to root-level `npm audit`/CI. Both lockfiles removed.
+- **`packages/docs` remaining 22 findings — mostly resolved.** Bumped all
+  four `@docusaurus/*` packages together to `3.10.2` (from `3.6.3`),
+  verified with a real `docusaurus build`, and fixed a config deprecation
+  warning (`onBrokenMarkdownLinks` → `markdown.hooks.onBrokenMarkdownLinks`)
+  the bump surfaced. `npm audit fix` at the new version resolves down to 24
+  (6 moderate, 18 high) — `serialize-javascript`/`uuid`/`sockjs`/
+  `webpack-dev-server` transitively via `@docusaurus/bundler`'s webpack
+  tooling, with **no fix available** even at Docusaurus's current latest
+  release (confirmed via `npm audit`'s own output). These are build/dev-time
+  tooling dependencies, not shipped in the static site users receive; there
+  is nothing further to do here until Docusaurus itself updates that
+  dependency chain upstream.
+- **`ToolClass`/`ToolProxy` transport layering (#13) — resolved.** Added a
+  `ToolTransport` interface (plus `ToolTransportConnectionOptions` and a
+  `ToolTransportFactory` type) to `src/core/types.ts`, purely structural —
+  zero runtime import. `ToolProxy` no longer imports `MCPTransport`/
+  `getTransport` from `mcp/transport.ts` at all; its constructor now takes
+  an optional `transportFactory` and returns a clear "no transport
+  configured" error result (instead of silently reaching into a concrete
+  implementation) when none was injected. The two call sites that construct
+  `ToolProxy` — `compiler.ts`'s `createIMP` and `mcp/artifact.ts`'s
+  `hydrateRuntime` — now explicitly pass `getTransport` from
+  `mcp/transport.ts`, preserving identical existing behavior. Verified the
+  fix is real (not just moved) by grepping the built `dist/inference.js`,
+  `dist/core/*.js`, and `dist/runtime/*.js` for any reference to
+  `mcp/transport` — none — and added `src/inference.test.ts`, a source-scan
+  regression test that fails if any core/runtime file statically imports
+  `mcp/transport.ts` again.
+- **`src/importance/` vs `shorthand/src/importance/` fork (#21) —
+  resolved.** `diff -rq` on the two directories showed every file byte-
+  identical except `types.ts`: the local copy defined its own narrower
+  `ConversationMessage` interface instead of importing the canonical one
+  (with a `'tool'` role option, `timestamp: string | number`, and
+  `normalizeTimestamp`) that `shorthand/src/importance/types.ts` already
+  imports from `@shorthand/core`'s shared types. Also confirmed
+  `src/compaction/` and `src/crdt/` — the two satellites PR #58 actually
+  finished extracting — have no local directory at all; `src/importance/`
+  was the one left as a stale duplicate. Replaced it with a thin re-export
+  of `@shorthand/core/importance` (same pattern `src/index.ts` already uses
+  for compaction/CRDT), deleting the five duplicated implementation files
+  and their tests. The `@smallchat/core/importance` public subpath is
+  unchanged — same exported names, same behavior — so this is non-breaking
+  for anyone consuming it.
+
+  While making this change, found that root CI (added earlier in this
+  audit) never actually ran `shorthand/`'s own test suite — root
+  `vitest.config.ts` only globs the root `src/`, and nothing wired
+  `shorthand`'s ~260 specs (compaction, CRDT, importance) into `npm test`
+  or CI. This had been true even before this change (deleting the local
+  importance duplicate just made the gap visible, it didn't create it).
+  Added an explicit `npm test --workspace=shorthand` step to CI's `test`
+  job to close it.
+- **`commander`/`better-sqlite3` major bumps (#14) — resolved, with an
+  explicit maintainer decision on the Node floor.** Asked directly rather
+  than deciding unilaterally: raise `engines.node` to `>=22.0.0`, or hold at
+  `>=20.0.0` and skip the bumps. Maintainer chose to raise the floor.
+  Bumped `commander` (`^13.0.0` → `^15.0.0`), `better-sqlite3` (`^11.0.0` →
+  `^13.0.3`), and `@types/better-sqlite3` (`^7.6.12` → `^9.6.0`) in root
+  `package.json`; `better-sqlite3` and `vitest` (`^3.0.0` → `^3.2.7`, the
+  latter previously inconsistent — see the earlier dependency-CVE pass) in
+  `shorthand/package.json` too, so the whole workspace resolves to a single
+  deduped copy of each instead of two divergent ones. `engines.node` raised
+  to `>=22.0.0` everywhere it was declared (root, `shorthand/`,
+  `packages/examples/`, `packages/docs/`), and in the `package.json`
+  template `smallchat init` scaffolds for new projects (which depend on
+  `@smallchat/core` and would otherwise inherit a floor that's already
+  wrong). CI's `test` matrix dropped Node 20 in favor of `['22', '24']`;
+  the `audit` and `docs` jobs' single Node version moved from `'20'` to
+  `'22'`. `README.md`'s "Requires Node.js >= 20" updated to `>= 22`.
+
+  Verified beyond typecheck/tests: built and ran the actual CLI
+  (`smallchat --help`, `smallchat doctor`) against the new
+  `better-sqlite3@13`/`commander@15` — help text renders correctly and
+  `doctor` confirms `better-sqlite3 + sqlite-vec: working`, since a
+  `better-sqlite3` major bump is exactly the kind of change a type-check
+  alone wouldn't catch a native-binding or CLI-formatting regression in.
 
 ## Dependency Upgrade Summary
 
@@ -142,20 +242,21 @@ specific reason rather than oversight:
 
 Roughly in priority order:
 
-1. **Decide the Node floor**, then take the `commander`/`better-sqlite3`
-   major bumps (both are otherwise routine).
-2. **Scope a `ToolProxy` transport-abstraction refactor** to fix the
-   Tier-1/Tier-2 layering violation (#13) as its own PR.
+1. ~~Decide the Node floor, then take the `commander`/`better-sqlite3`
+   major bumps~~ — done, see
+   [Follow-up](#follow-up-deferred-items-addressed-second-pass). Floor is
+   now Node >=22.
+2. ~~Scope a `ToolProxy` transport-abstraction refactor~~ — done, see
+   [Follow-up](#follow-up-deferred-items-addressed-second-pass).
 3. **Test coverage:** `src/app/` (MCP Apps compile/runtime pipeline, ~1,160
    lines) still has no tests; `src/memex/resolver.ts`'s primary `resolve()`
    function is untested (only its `computeTier()` helper is); `LocalEmbedder`
    (the placeholder hash-based embedder referenced in QUICKSTART.md) has no
    tests.
-4. **Consolidate `src/importance/` with `@shorthand/core/importance`**
-   per `docs/ecosystem/engineering-guide.md`'s existing analysis.
-5. **Un-nest the stray lockfiles** in `shorthand/` and `packages/examples/`
-   once their independent-publish requirements (if any) are confirmed, or
-   document why they're intentionally separate.
+4. ~~Consolidate `src/importance/` with `@shorthand/core/importance`~~ —
+   done, see [Follow-up](#follow-up-deferred-items-addressed-second-pass).
+5. ~~Un-nest the stray lockfiles~~ — done, see
+   [Follow-up](#follow-up-deferred-items-addressed-second-pass).
 6. **Consider Git LFS** for the committed ONNX model files, or at minimum
    document the convention in ARCHITECTURE.md/README.md.
 7. **Prune stale entries** from the rate limiter's and connection pool's

CHANGELOG.md

@@ -7,6 +7,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Changed — Node floor raised to >=22 (audit follow-up, maintainer decision)
+- **`engines.node` is now `>=22.0.0`** (previously `>=20.0.0`), raised on an explicit decision rather than unilaterally, since it's a real breaking change for anyone still on Node 20. This unblocked the last deferred dependency item from the security audit: `commander` (`^13.0.0` → `^15.0.0`) and `better-sqlite3` (`^11.0.0` → `^13.0.3`, both now require Node ≥22 upstream), plus `@types/better-sqlite3` (`^7.6.12` → `^9.6.0`) to match. Applied consistently across `package.json`, `shorthand/package.json` (which also depends on `better-sqlite3` directly — now deduped to a single version instead of two divergent ones — and had its own `vitest` floor bumped to match the earlier `^3.2.7` CVE fix, which it had missed), `packages/examples/package.json`, `packages/docs/package.json`, and the `package.json` template `smallchat init` scaffolds for new projects. CI's Node matrix moved from `['20', '22']` to `['22', '24']`. README updated.
+- Verified beyond typecheck/tests: built and ran the actual CLI against the new `better-sqlite3`/`commander` — `smallchat --help` renders correctly and `smallchat doctor` confirms `better-sqlite3 + sqlite-vec: working`.
+
+### Fixed — importance-scoring fork reconciled (audit follow-up, "Phase 0")
+- **`src/importance/` was a stale, drifted duplicate of `@shorthand/core/importance`.** PR #58 ("Extract `@shorthand/core` package from compaction, CRDT, and importance modules") only actually finished the extraction for compaction and CRDT — `src/compaction/` and `src/crdt/` don't exist as local directories, they're pure re-exports. `src/importance/` was left as a full duplicate copy instead, and it had already drifted: its `types.ts` defined a narrower local `ConversationMessage` (missing the `'tool'` role, `timestamp: string | number`, and `normalizeTimestamp`) instead of the canonical shared one. Replaced `src/importance/` with a thin re-export of `@shorthand/core/importance`, matching compaction/CRDT. The public `@smallchat/core/importance` subpath is unchanged — same names, same behavior, non-breaking.
+- **CI gap found and closed while making this change:** `shorthand/`'s own ~260-spec test suite (compaction, CRDT, importance) was never run by root `npm test` or CI — root `vitest.config.ts` only globs the root `src/`. Added `npm test --workspace=shorthand` to CI's `test` job.
+
+### Changed — ToolProxy transport is now injected, not hardcoded (audit follow-up)
+- **Fixed the Tier-1/Tier-2 layering violation flagged in the security audit.** `ToolClass`/`ToolProxy` — re-exported from the "durable, transport-agnostic" `@smallchat/core/inference` entry point — statically imported `MCPTransport`/`getTransport` from `src/mcp/transport.ts` (~550 lines of HTTP/JSON-RPC/SSE/gRPC wire-protocol code), contradicting the entry point's own stated contract. Added a `ToolTransport` interface, `ToolTransportConnectionOptions`, and a `ToolTransportFactory` type to `src/core/types.ts` (type-only, zero runtime cost). `ToolProxy` now takes an optional `transportFactory` in its constructor and returns a clear "no transport configured" error result if none was injected, instead of reaching into a hardcoded implementation. `compiler.ts` and `mcp/artifact.ts` — the two places that construct `ToolProxy` — now explicitly pass `getTransport` from `mcp/transport.ts`, preserving identical existing behavior. Verified against the built output (`dist/inference.js`, `dist/core/*.js`, `dist/runtime/*.js`) that no trace of `mcp/transport` remains, and added `src/inference.test.ts` as a standing regression test for this boundary.
+
+### Dependencies — audit follow-up (deferred items)
+- **Removed `shorthand/package-lock.json` and `packages/examples/package-lock.json`.** Neither was being maintained by any normal workflow — `npm install` run inside either directory defers entirely to the workspace root and never updates the local lockfile — and this had already caused real drift: `shorthand`'s copy still resolved `vitest@3.2.4`, the exact version with the critical CVE fixed at the root below. `@shorthand/core` isn't published to npm at all, so there was no standalone-publish reason to keep it either.
+- **`packages/docs`: Docusaurus `3.6.3` → `3.10.2`** (all four `@docusaurus/*` packages bumped together), resolving down to 24 remaining `npm audit` findings (from 44), all in `@docusaurus/bundler`'s webpack toolchain with no fix available upstream yet. Also migrated the now-deprecated `onBrokenMarkdownLinks` config option to `markdown.hooks.onBrokenMarkdownLinks`, and aligned `packages/docs`'s `engines.node` from `>=18.0` to `>=20.0` (Docusaurus 3.10 itself requires Node ≥20; the old floor was already inconsistent with the rest of the monorepo and no longer accurate).
+
 ### Security — server/transport hardening (security audit pass)
 - **MCP server auth/rate-limit perimeter closed.** `GET /sse` now requires the same bearer token as the JSON-RPC endpoint when `enableAuth` is set (previously reachable by any unauthenticated client). `resources/subscribe` notifications no longer broadcast to every connected SSE client when the subscribing request carries no session id — a cross-session notification leak. The rate limiter no longer trusts the client-supplied `Mcp-Session-Id` header as a bucket key unless it names a session that actually exists, closing a trivial bypass (send a fresh header value, get a fresh bucket). `POST /oauth/token` now has its own always-on rate limiter (20 rpm) independent of `enableRateLimit`, and `OAuthManager.authenticateClient` compares secret hashes with `crypto.timingSafeEqual` instead of `!==`.
 - **Channel HTTP bridge.** `GET /sse` is no longer exempt from the shared-secret check (it streams channel events and pending tool-approval requests); only `/health` stays open. The secret comparison is now constant-time. `serializeChannelTag()` now XML-escapes `content` (previously only meta-attribute values were escaped), preventing a sender from embedding a literal `</channel>` + forged `<channel source="trusted">` to spoof a second channel event's provenance.

README.md

@@ -79,7 +79,7 @@ npm run build
 npm install @smallchat/core
 ```
 
-Requires Node.js >= 20.
+Requires Node.js >= 22.
 
 > **Swift:** the Swift implementation lives in its own repository — [github.com/johnnyclem/smallchat-swift](https://github.com/johnnyclem/smallchat-swift).
 
@@ -227,7 +227,8 @@ package) versus what's still aspirational.
 ## Development
 
 ```bash
-npm test          # ~1,150+ specs across the core runtime, compiler, embeddings, and transports
+npm test          # ~1,100+ specs across the core runtime, compiler, embeddings, and transports
+npm test --workspace=shorthand  # ~260 specs for the vendored compaction/CRDT/importance modules
 npm run dev       # Watch mode
 npm run lint      # Type check
 npm run docs:api  # Generate API reference