2 Commits

Include renames

Author	SHA1	Message	Date
4 people	40064955ab	feat+test+docs: D45 — server.mjs auth integration + lib/audit.mjs (#21) ... * feat+test+docs: D45 — server.mjs auth integration + lib/audit.mjs (Phase 2 wire-up) Second Phase 2 implementation D-day. Wires the D44 lib/keys.mjs identity layer into the request flow + lands lib/audit.mjs per ADR 0007 § 6.2 + § 8. Closes ADR § 10 acceptance criteria #1 (per-key cache isolation), #2 (anonymous prod-default off), #3 (anonymous dev-mode on), #6 (post-revoke 401 within next request — full coverage with D45), #8 (audit ndjson round-trip), #10 (OLP_OWNER_TOKEN env override — full server-side coverage), #11 (providers_enabled 403 scope). Owner-vs-guest gating for /health + X-OLP-Fallback-Detail (criteria #4, #5) remains in D46 scope. NEW lib/audit.mjs (~110 lines): - appendAuditEvent(event, opts): one JSON event per line to ~/.olp/logs/audit.ndjson (file 0600, dir 0700). § 6.2 retry: warn + 1 retry; per-process drop counter + warn on second failure; NEVER throws. Per-call OLP_HOME env resolution (matches lib/keys.mjs). - getAuditDropCount(): for future /health surface. lib/keys.mjs extended: - loadAuthConfigSync({ olpHome }): reads auth block from ~/.olp/config.json with ADR § 7.2 defaults (allow_anonymous: false, owner_only_endpoints: ['/health'], fallback_detail_header_policy: 'owner_only'). Never throws; missing file / malformed JSON falls back to defaults. - _resolveOlpHome(opts): precedence opts.olpHome → process.env.OLP_HOME → ~/.olp. Per-call resolution so tests + operator deployments can redirect without code edits. server.mjs auth middleware integration: - extractToken(req): parses Authorization Bearer / x-api-key. - authenticate(req): validateKey + 401 paths (auth_required vs invalid_or_revoked_key). - isProviderEnabled(olpIdentity, providerKey): '*' = all; else array allowlist. - _authConfig loaded at startup; warn auth_allow_anonymous_enabled when true. Test seams __setAuthConfig / __resetAuthConfig. - handleChatCompletions + handleModels both gated by authenticate at top. Audit ctx built throughout; res.on('finish') appends row + fires touchLastUsed async. - IDENTITY-VS-CREDENTIALS SEPARATION: olpIdentity (new validated identity) consumed for cache namespacing + providers_enabled + audit; authContext passed to provider.spawn() REMAINS null so providers continue their own credential discovery (env / keychain / file). Per-provider per-key credential mapping is Phase 3+ per ADR § 12. - handleChatCompletions chain filtered by isProviderEnabled; empty result returns 403 key_no_provider_access. - keyId = olpIdentity.keyId (replacing hardcoded '__anonymous__'). - Audit captures fields throughout: post-auth, post-IR, post-chain (success or exhausted). Status + latency populated on res.on('finish'). TESTS — Suite 20, +15 (499 → 514): 20a-d: header parsing + valid key happy paths (Bearer / x-api-key / invalid → 401) 20e: revoked key 401 (criterion #6 end-to-end) 20f: OLP_OWNER_TOKEN env override returns 200 (criterion #10 full) 20g: allow_anonymous=true + no header returns 200 (criterion #3) 20h + 20h-extra: providers_enabled=['mistral'] for anthropic model → 403; '*' baseline returns 200 (criterion #11) 20i: per-key cache namespace isolation (criterion #1 end-to-end) 20j + 20j-401: audit.ndjson written with § 8 schema fields + PII guard; 401 path also appends (criterion #8) 20k: filesystem key last_used_at populated post-request (D45 touch wire) 20l + 20l-200: /v1/models also enforces auth TEST-MODE SETUP (test-features.mjs): - process.env.OLP_HOME = mkdtempSync(...) at module load so audit + key writes don't pollute ~/.olp/. - __setAuthConfig({ allow_anonymous: true }) after server.mjs imports so pre-D45 HTTP integration tests (Suite 18 etc.) continue to pass. - Suite 20 explicitly overrides __setAuthConfig per-case to exercise production-default-off coverage. DOCUMENTATION: - AGENTS.md: lib/keys.mjs 🟡 marker updated + NEW lib/audit.mjs entry; Implementation-status-note + shipped-set updated. - README.md: Implementation Status table gains lib/audit.mjs row + lib/keys.mjs row updated; Known limitations Multi-key auth note rewritten to reflect D45 ship + D46 follow-up; new env-vars (OLP_HOME, OLP_OWNER_TOKEN) and auth config block surfaced. - CHANGELOG.md: D45 entry under Unreleased per release_kit overlay phase_rolling_mode discipline. AUTHORITY: - ADR 0007 (multi-key auth — §§ 5/6.2/7/9.4 implementation contracts + § 10 criteria #1/#2/#3/#6/#8/#10/#11 covered). - CLAUDE.md release_kit overlay phase_rolling_mode — under Unreleased. - Phase 2 kickoff handoff (~/.cc-rules/memory/handoffs/ 2026-05-25-phase-2-kickoff.md in cc-rules d9da966). - Standing autopilot grant (~/.cc-rules/memory/auto/ standing_autopilot_phase_2.md in cc-rules bf0ed9a). Verified: 514/514 pass via npm test (no regression in 499 existing tests; 15 new Suite 20 tests all green). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix+test+docs: D45 fold-in — CI fail recovery + opus reviewer P1/P2/P3 Fresh-context opus reviewer (PR #21) returned APPROVE_WITH_MINOR with 4 findings; CI Node 24 separately reported 9 Suite 20 failures (all 200-expecting tests). Root cause of CI: Suite 20 setup did not stub CLAUDE_CODE_OAUTH_TOKEN before mock spawn, so anthropic.mjs AUTH_MISSING pre-check fired and tests 502'd. (Local Node 22 had the env from the maintainer's claude install — masked the gap.) CI FIX — Suite 20 OAuth env stub Added ensureSuite20FakeOAuth / restoreSuite20OAuth helpers in makeSuite20Server / teardownSuite20. Matches the existing pattern in Suite 9 line ~2154 (test-fake-oauth-token-for-cache-tests). P1 — Real-streaming path audit fidelity Single-hop streaming success (server.mjs ~L1050, the most common deployed shape) did not populate auditCtx.provider / tried_providers / cache_status. Audit rows for streaming requests carried provider: null. Fixed by stamping these at the top of the streaming branch and amending error_code on the two streaming failure exit paths (streaming_error_after_first_chunk + streaming_error_before_first_chunk). New regression test 20j-stream: streaming request asserts the audit row's provider, cache_status, and tried_providers fields are populated. P2 — Global test tmpdir cleanup process.env.OLP_HOME = mkdtempSync(...) at test-features.mjs module load left /var/folders/.../olp-test-home-* leak per npm test run. Fixed by process.on('exit', () => rmSync(_GLOBAL_TEST_OLP_HOME)). Best-effort; swallows errors so exit handler never throws. P3 — handleModels 401 lacks OLP diagnostic headers handleChatCompletions 401 passes olpErrorHeaders({ startMs }); handleModels 401 did not. Aligned. DEFERRED — P2 tried_providers semantics on 403 Reviewer noted that key_no_provider_access 403 stamps original chain in tried_providers, but the field name implies hops actually dispatched. Either ADR § 8 amendment or D46+ semantic fix. Marked in CHANGELOG; not in this fold-in scope. Test count: 514 → 515 (+1 streaming-audit regression test 20j-stream; 14 existing Suite 20 tests still pass). Verified locally via npm test. CI Node 24 recovery via the OAuth env stub. Authority: PR #21 fresh-context opus reviewer findings; CI Node 24 run 26382758946 failure logs; CLAUDE.md release_kit overlay phase_rolling_mode — under Unreleased. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: dtzp555 <dtzp555@gmail.com> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>	2026-05-25 14:28:45 +10:00
4 people	4b9916341b	feat+test+docs: D44 — lib/keys.mjs core landed (multi-key auth, no server wire-up yet) (#20) ... * feat+test+docs: D44 — lib/keys.mjs core landed (multi-key auth, no server wire-up yet) First Phase 2 implementation D-day. Lands the lib/keys.mjs module per ADR 0007 §§ 5 / 6.1 / 6.3 / 6.3.5 / 6.4 / 9.4. Identity / lifecycle layer for OLP API keys is now in-tree; server.mjs integration is scheduled D45 (until then, requests still use the hardcoded '__anonymous__' cache namespace — no behavioural change at v0.1.1 / D44). NEW FILE lib/keys.mjs (~437 lines, public API): - createKey({ name, owner_tier, providers_enabled, notes, olpHome }) Generates opaque 'olp_<32-byte base64url>' token (47-char total), SHA-256 hashes for manifest storage, atomically writes keys/<id>/manifest.json (file 0600, dir 0700). Returns { id, plaintext_token, manifest } — plaintext printed once, never persisted. - validateKey(plaintext, { allowAnonymous, olpHome }) Three-tier resolution per § 5 / § 7 / § 9.4: env override (OLP_OWNER_TOKEN -> __env_owner__) -> anonymous (only when allowAnonymous: true, returns __anonymous__) -> filesystem manifest lookup (constant-time hash compare via timingSafeEqual). Revoked manifests return null (caller produces 401). Per § 6.3.5: MUST hit manifest every request; no in-process validation cache. - revokeKey({ id, olpHome }) Idempotent; sets revoked_at via atomic write inside per-key lock. - listKeys({ olpHome }) Returns manifest objects with token_hash redacted. - touchLastUsed(id, { olpHome }) Async best-effort lazy update per § 6.3 revoke-dominates-touch: re-reads latest manifest inside per-key lock, NO-OPs if revoked_at is non-null, otherwise merges last_used_at preserving all other fields. Failure logs warn and never throws. Plus internal helpers: hashToken (SHA-256 hex), generateToken, generateKeyId, validateManifest (§ 4 schema validation), readManifest, writeManifestAtomic (tmpfile + fsync + rename + chmod), _withKeyLock (§ 6.4 in-process per-key write-lock chain), _safeHexCompare (timing-safe). Test-only hooks: __setTouchInterleaveHook (inject deterministic pause for race tests), __resetWriteLocks (test cleanup). NOT IN D44 (split per ADR §§ 6.2 / 9.1 separation): - audit ndjson append (§ 6.2) — request-layer concern; D45 server glue - keygen CLI bootstrap surface (§ 9.1) — D45+ separate command entry - server.mjs integration replacing hardcoded '__anonymous__' at server.mjs:502, :531 — D45 - owner-vs-guest gating for /health (server.mjs:392) + X-OLP-Fallback- Detail (server.mjs:1072, :1101) — D46 TEST COUNT: 468 -> 496 (+28 tests in new Suite 19): - 19a-d: token generation (§ 5) - 19e-j: manifest write+read + chmod 0600/0700 + schema validation (§ 4, § 6.1) - 19k-p: validateKey (filesystem / wrong / missing / anonymous / revoked / env override) (§ 5, § 6.3.5, § 9.4) - 19q-r: revokeKey idempotency + non-existent id - 19s-t: listKeys empty + redaction - 19u-x: touchLastUsed updates + NO-OP on revoked + NO-OP on anonymous/env identities + best-effort failure - 19y-1 to 19y-4: ACCEPTANCE CRITERION #7 — concurrent revoke + touch race tests: 19y-1 revoke -> touch (revoked_at survives) 19y-2 touch -> revoke (revoked_at + last_used_at both present) 19y-3 interleaved external-revoke via __setTouchInterleaveHook (deterministically reproduces the § 6.3 race the maintainer's D43-B text review caught — confirms our impl observes the revoke and NO-OPs) 19y-4 30-iteration concurrent Promise.all stress DOCS UPDATED IN THIS COMMIT: - AGENTS.md: lib/keys.mjs marker 📋 -> 🟡 'core landed at D44'; Implementation-status-note + shipped-set updated. - README.md: Implementation Status row + Known limitations 'Multi-key auth' note updated to 'core landed, server integration pending D45'. - CHANGELOG.md: D44 entry under Unreleased per release_kit overlay phase_rolling_mode discipline. AUTHORITY: - ADR 0007 (multi-key auth) — Decision: Option 2 filesystem manifest + opaque token; §§ 5/6.1/6.3/6.3.5/6.4/9.4 implementation contracts; § 10 acceptance criteria #6/#7 partially covered by D44 tests (#7 fully covered; #6 partially covered — full coverage requires D45+ server integration). - CLAUDE.md release_kit overlay phase_rolling_mode — under Unreleased. - Phase 2 kickoff handoff: ~/.cc-rules/memory/handoffs/2026-05-25-phase-2-kickoff.md (cc-rules d9da966). - CC 开发铁律 v1.6 § 10 — fresh-context opus reviewer required. Verified: 496/496 pass via npm test before commit. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * feat+test+docs: D44 fold-in — opus reviewer findings (2 P2 correctness + 2 P3 polish) Fresh-context opus reviewer (PR #20) returned APPROVE_WITH_MINOR with 4 findings — 2 P2 real correctness gaps + 2 P3 polish. All accepted; the 2 P2 fixes ship with new regression tests. P2 #1 — lib/keys.mjs _withKeyLock lock-map cleanup Prior version stored `prev.then(() => next)` as the Map tail, but the cleanup-identity check `_writeLocks.get(id) === next` could never match the derived promise. Result: Map entries leaked one-per-unique-key-id forever. Bounded impact at family scale (~5–10 entries) but a real correctness bug uncovered by reviewer empirical reproduction ("CLEANUP SKIPPED every call"). Fix: store `next` directly as the Map tail. The chain still works because new callers chain off `_writeLocks.get(id)` (the prior caller's `next`); compare-and-delete by identity correctly cleans up when the current caller is the last in queue. Regression tests: - 19x-extra: after 5 sequential touchLastUsed calls, __writeLockSize() must be 0. - 19x-extra-2: after 9 concurrent touch calls across 3 keys (3 per key), __writeLockSize() must drain to 0. P2 #2 — lib/keys.mjs validateKey non-string defensive coding Prior version threw TypeError when called with a non-string truthy plaintext (validateKey(42), validateKey({}), etc.), reaching hashToken(<non-string>) which calls createHash().update(<non-string>) which throws. Q2 (defensive-coding acceptance criterion) promised "bad inputs return null." Fix: top-of-function guard `if (plaintextToken != null && typeof plaintextToken !== 'string') return null;` Falls through to null path for non-string truthy; preserves existing null / undefined / '' handling. Regression test 19m-extra: validateKey(42), validateKey({}), validateKey([]), validateKey({ token: 'olp_xxx' }), and the same with allowAnonymous: true — all must return null without throwing. P3 #3 — 19y-3 test scope comment clarification Reviewer noted that 19y-3 simulates external revoke landing BEFORE touch's read (not BETWEEN touch's read and write — currently unreachable due to synchronous read→write in touchLastUsed). Added explanatory comment documenting: - The scenario this test does cover (pre-read external revoke). - The scenario this test does NOT cover (between-read-and-write). - Why scenario 3 is unreachable in the current impl (no await between readManifest and writeManifestAtomic). - The trigger for adding a post-read hook (any future refactor that introduces an await between read and write). P3 #4 — CHANGELOG line-count corrections D44 entry said ~330 lines (initial estimate); actual is 462 lines after fold-in. Test count claim updated from "+28 tests" to "+31 tests" (28 initial + 3 fold-in regression). New module-level export: __writeLockSize (test-only) — reports current size of in-process write-lock Map for the regression tests above. Not intended for production callers. Test count: 496 → 499 (+3 fold-in regression tests; +31 total from D44 inclusive of initial Suite 19). Verified locally via npm test. Authority: PR #20 fresh-context opus reviewer findings; CLAUDE.md release_kit overlay phase_rolling_mode — under Unreleased. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * docs: D44 fold-in #2 — CHANGELOG line-count consistency (trivial) Delta opus reviewer flagged internal CHANGELOG inconsistency: header bullet correctly stated `~462 lines` but the P3 #4 self-description bullet still said the prior fold-in corrected to `~445 lines`. Both now agree on 462 (matches `wc -l lib/keys.mjs`). No code change. Test count: 499 / 499 pass (unchanged). Authority: PR #20 delta opus reviewer trivial inconsistency note. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: dtzp555 <dtzp555@gmail.com> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>	2026-05-25 13:37:38 +10:00