[plugins] discovery (PR #2): cache-key token identity, bounded cache + review follow-ups

[kreneskyp]

Context

feat/plugin-discovery (PR #2) adds src/search.ts (the plugin-discovery API: searchPlugins / createPluginSearch / createTtlCache / sourceToInstallInput). It is green (tsc, 100% coverage, eslint, quire) and was smoke-tested live (found 8 @agent-ix/spec-* packages on npm). A /code-review + /gap-analysis pass (SpecReview SR-005, reviews/26-06-27-plugin-discovery-gap-analysis.md) found the items below. None are merge-blockers, but the first two are real (privacy + memory) and one is a spec↔code divergence.

Tasks

• FND-002 — Medium (privacy) + spec divergence. createPluginSearch cacheKey (src/search.ts ~line 568) keys on token presence (token ? "auth" : "anon"). FR-010 §Behavior (spec/functional/FR-010-discovery-cache.md ~line 44) specifies "a stable non-secret discriminator of the resolved token". Two distinct tokens currently collide on "auth", so with the late-bound-token feature (FR-010-AC-5 / TC-036) a shared searcher could serve user A's authenticated (incl. private-repo) GitHub results to user B. Fix: fold a stable non-secret hash of the resolved token (e.g. first 8 hex of SHA-256, computed via node:crypto) into the key instead of the boolean. Add a test: two different non-empty tokens produce distinct cache entries (no cross-token hit).
• FND-003 — Medium (memory). createPluginSearch leaves cacheMax undefined by default (src/search.ts ~line 546), so createTtlCache sets no max and entries evict only lazily on read of an expired key — memory grows with distinct tag|query|sources|limit searches over the TTL. Fix: default cacheMax (e.g. 256) in createPluginSearch. Add a test: LRU eviction past the bound.
• FND-004 — Low (in-CDN path traversal). manifestUrl (src/search.ts ~line 421) interpolates the registry-supplied name/fullName unencoded into https://unpkg.com/{name}/{manifestPath} and https://raw.githubusercontent.com/{fullName}/HEAD/{manifestPath}. Cross-host SSRF is not reachable (fixed authority), but a name containing .. can traverse within the trusted CDN. Fix: reject/encode path segments containing .. or control chars in the registry-derived name before building the URL. (manifestPath is host-supplied → trusted.)
• FND-005 — Low. parseRate Number(header) (src/search.ts ~line 331) has no non-finite guard → surfaces NaN rate fields; treat a non-finite parse as "no rate info" (return undefined). A cache hit returns the shared SearchResponse reference (~line 576) → return a shallow clone or document immutability.
• FND-001 — process. Flip plan/Plan-001-plugin-discovery/tasks/Task-001..007 status: todo → done (they are implemented + verified). Task-008 (publish) stays pending.

Acceptance criteria

• Distinct non-empty tokens → distinct cache entries (test); no token value ever appears in a key (CON-2 still holds).
• Cache bounded by default + LRU-eviction test.
• Manifest name with .. is rejected/encoded (test).
• src/search.ts stays at 100% coverage; quire validate --scope . "spec/**/*.md" clean.
• Gap-analysis verdict clears toward PASS.

References

• PR: feat: plugin discovery (search npm/GitHub for compatible plugins) #2 · branch feat/plugin-discovery
• SpecReview SR-005 (reviews/26-06-27-plugin-discovery-gap-analysis.md)
• Spec: spec/functional/FR-010-discovery-cache.md, FR-009-compatibility-verification.md, FR-008-candidate-search.md

[kreneskyp]

Build-out complete — pushed to feat/plugin-discovery (PR #2), commit 8574fef. All findings implemented with Red-first TDD.

• FND-002 (privacy + spec divergence): cacheKey no longer keys on token presence; new tokenId() folds a stable non-secret discriminator (first 8 hex of SHA-256 via node:crypto, anon when absent) into the key — raw token never enters it (FR-008-CON-2 preserved). TC-055: distinct tokens → distinct entries, no cross-token hit.
• FND-003 (memory): createPluginSearch defaults cacheMax = 256. TC-056 LRU eviction past the bound; TC-060 explicit override honored.
• FND-004 (in-CDN path traversal): unsafeManifestName() rejects any segment with .. or control chars; manifestUrl() returns null and verifyOne drops the candidate before fetching. Scoped-pkg slashes preserved. TC-057.
• FND-005 (low): parseRate returns undefined on non-finite; cache get/set clone the SearchResponse (one level) so callers can't poison the cache. TC-058/059.
• FND-001 (process): Task-001..007 flipped to done; Task-008 (publish) stays todo.
• Spec/matrix: FR-009-AC-9/CON-1, FR-010-AC-8/9/10, FR-011-AC-6 + TC-055–060 rows.

Gates: 78 tests pass · 100% coverage (src/search.ts) · tsc + vite build clean · eslint + prettier clean · quire validate clean.

Running /code-review + quoin:gap-analysis on the PR now.

[kreneskyp]

Review gate: PASS (merge-ready). /code-review + quoin:gap-analysis on PR #2 @ 8574fef:

• All four findings (FND-002 token identity, FND-003 bounded cache, FND-004 path traversal, FND-005 NaN/clone) confirmed resolved with real, traced tests (TC-055–060). Adversarial sweep clean: no cross-token cache hit, no cross-host SSRF, cache-clone prevents container corruption.
• Gates (run independently): 78 tests pass · 100% coverage (336/282/62/303) · tsc + eslint + prettier clean · quire validate exit 0.

2 LOW follow-ups (non-blocking, not gating the release):

1. unsafeManifestName doesn't catch percent-encoded %2e%2e — only reachable if a registry response returns such a name (real npm/GitHub names can't), same-CDN, no SSRF. Optional: decode-then-check.
2. cloneResponse is shallow; nested deep-mutation could still corrupt a cached entry, so FR-010-AC-10 slightly over-claims. Optional: deep-clone results or soften AC-10 wording.

Merge-ready pending the release train (and the CODEOWNERS maintainer approval now required on main).

[kreneskyp]

Heads-up before merge: CodeQL flags 4 HIGH js/incomplete-url-substring-sanitization alerts — all in tests/search.test.ts (lines 455/507/508/604), where assertions check the manifest host via substring includes/toContain("unpkg.com"). Production src/ is clean. These will be fixed in the rebase/reconcile pass by asserting the host strictly (new URL(...).host === ... / startsWith("https://unpkg.com/")) — stricter assertions that also clear the alerts. PR #2 then rebases onto the final main (after #3 + #8) with TC renumbering.

[kreneskyp]

Merged. PR #2 squash-merged to main as b003a58. Discovery API (searchPlugins/createPluginSearch/createTtlCache/sourceToInstallInput + types) is in main; all four review findings (FND-002…005) resolved, the 4 CodeQL test-assertion alerts fixed (strict new URL().host checks), discovery TCs renumbered to TC-032…070 (contiguous/unique), 100% coverage. Third leg of the release train done — kit release (v0.2.0 tag/publish) next. (Minor cosmetic follow-up: a few discovery matrix rows still show 🚧 though their tests now exist+pass.)