From 1ddbc4e989b2240c4f1dc4adb407d4a5679e4248 Mon Sep 17 00:00:00 2001 From: t Date: Mon, 7 Sep 2026 01:50:54 +0900 Subject: [PATCH 1/3] docs(skill): keep plaintext keys out of agent recipes [skip ci] Carry #3324 from 2933cc5842 and 7734e758b7. Preserve prior exact revocation authority, benign commit/abort, and managed connect rotation. Add bounded literal recipe regression detector; this is guidance and static warning, not runtime enforcement. Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> --- skills/ocx/SKILL.md | 25 ++++- skills/ocx/references/03_recipes.md | 55 +++++++++-- skills/ocx/references/05_remote_hub.md | 6 ++ tests/ci-workflows/skill-ocx.test.ts | 132 +++++++++++++++++++++++++ 4 files changed, 209 insertions(+), 9 deletions(-) diff --git a/skills/ocx/SKILL.md b/skills/ocx/SKILL.md index a9975e7745..a5f19d861f 100644 --- a/skills/ocx/SKILL.md +++ b/skills/ocx/SKILL.md @@ -1,12 +1,12 @@ --- name: ocx -description: Drive a running opencodex (`ocx`) proxy from the CLI — account pools, provider routing, model catalog, usage and cost attribution, request logs, access keys, storage cleanup, and the management API. Use when a task involves controlling or inspecting an opencodex proxy rather than editing the opencodex codebase. Triggers: ocx, opencodex, proxy control, account pool, pause account, pool strategy, provider routing, usage report, cost attribution, access key, request log, conversation trace, storage cleanup, management API. +description: "Drive a running opencodex (`ocx`) proxy from the CLI — account pools, provider routing, model catalog, usage and cost attribution, request logs, access keys, storage cleanup, and the management API. Use when a task involves controlling or inspecting an opencodex proxy rather than editing the opencodex codebase. Triggers: ocx, opencodex, proxy control, account pool, pause account, pool strategy, provider routing, usage report, cost attribution, access key, request log, conversation trace, storage cleanup, management API." --- # Operating `ocx` `ocx` controls a locally running opencodex proxy. The CLI covers the dashboard's operational -surface, with one consent exception (starring) recorded under Consent below. `ocx capabilities` +surface, subject to Consent and Secret-bearing commands below. `ocx capabilities` lists the *declared* index, not every verb. Be precise about the gap, because guessing costs you more than reading: the capability index below @@ -90,6 +90,27 @@ starring would be useful, say so and let the user decide. The same boundary covers the session-gated `/api/codex-prompt` writes: read them with `ocx inspect codex-prompt`, and leave the writes to the dashboard. +## Secret-bearing commands + +**Do not create an access key or start an access-key rotation from an agent session.** +This covers the create and rotation-start operations under `ocx access key`, +`ocx access keys`, and `ocx api-key`, their `opencodex` equivalents and executable +wrappers, and direct POST requests to `/api/keys` and `/api/keys/rotate`. +Both text and JSON responses contain a one-time plaintext data-plane credential, +which can enter the agent transcript. Ask the user to perform that step in a +human-operated terminal outside the agent session, configure and verify the +replacement, and report only confirmation plus non-secret key/rotation IDs. +Never ask for the plaintext key in chat or offer a pipe, redirection, or API +workaround to perform the secret-returning step inside the agent session. + +Configuration confirmation is not approval to revoke the existing credential. +Identify the existing key ID and obtain separate explicit revocation approval +before committing an in-place rotation or removing an old, separately replaced key. +An existing explicit approval for that exact revocation remains valid; setup +confirmation alone does not supply it. Commit and abort return no plaintext key, +but still require authority for their state changes. Follow +[recipe 5](references/03_recipes.md#5-prepare-an-access-key-rotation-without-exposing-the-new-key). + ## Destructive verbs `storage trash restore` and `storage policy run` refuse without `--yes` (exit 2, nothing sent). diff --git a/skills/ocx/references/03_recipes.md b/skills/ocx/references/03_recipes.md index 85b734a9ce..424ad34a53 100644 --- a/skills/ocx/references/03_recipes.md +++ b/skills/ocx/references/03_recipes.md @@ -94,20 +94,61 @@ Read `accounts[]`. Two things to respect: `providers[]` and `models[]` carry `estimatedCostUsd`. Costs are estimates; `estimateReasons` in the log rows tells you why (for example `usage_estimated`, `expected_price_overlay`). -## 5. Rotate an access key and confirm it went quiet +## 5. Prepare an access-key rotation without exposing the new key ```bash ocx access key list --json -ocx access key create rotated --json # the plaintext key is in THIS response only +``` + +Creating a key or starting a rotation returns a one-time plaintext credential in both text and +JSON output. **Do not perform either operation in an agent session**, including through the +aliases, executable wrappers, or management POST routes named in +[Secret-bearing commands](../SKILL.md#secret-bearing-commands). Ask the user to perform that step +in a terminal outside the agent session, configure and verify the replacement, and report only +configuration confirmation and the non-secret key/rotation IDs. Never ask for the key itself. + +Configuration confirmation is not revocation approval. Identify the existing key ID and obtain +separate explicit revocation approval before taking either path below. An existing explicit +approval for that exact revocation remains valid; do not ask again for the same action and ID. + +For an in-place rotation, commit the pending replacement on the same ID: + +```bash +ocx access key rotate commit --json +``` + +For a separately created replacement, remove only the old ID: + +```bash ocx access key remove --yes --json -ocx access key list --json # the old id is gone; check usage on the rest ``` -Note the argument style: `create ` and `remove ` are **positionals**, not `--label` and -`--id`. `remove` also refuses without `--yes`. +After the command succeeds, inspect the matching result: + +```bash +ocx access key list --json +``` + +For an in-place rotation, the same ID remains and `pendingRotation` disappears. For a separately +created replacement, the old ID disappears. The list alone does not prove the replacement accepts +traffic; use the user's successful connection verification as that evidence. `remove ` is +positional, not `--id`, and refuses without `--yes`. + +To cancel a pending rotation, with authority to discard the replacement: + +```bash +ocx access key rotate abort --json +``` + +Abort retains the old credential and removes the pending replacement. Re-list to inspect pending +state. On stale, mismatched, or expired rotation IDs, or an uncertain commit result, inspect +non-secret state and report the refusal or uncertainty. Do not start another rotation, delete the +entry, or retrieve a secret as automatic recovery. Missing pending state alone is not proof of a +successful commit: expiry and abort also clear it. -The list carries per-key usage, so a key whose count stops advancing is genuinely unused. The -plaintext key appears once, in the `create` response, and is never retrievable again. +The list carries per-key usage. A count that stops advancing shows no recorded new usage in that +observation window; it does not prove no client still needs the key. Creation and rotation-start +return the plaintext once; list does not return the full plaintext. An `ambiguous` footer on the list means two configured keys share an id, so per-key totals do not exist for them — do not attribute usage to either. diff --git a/skills/ocx/references/05_remote_hub.md b/skills/ocx/references/05_remote_hub.md index 46b846d443..0c0596be07 100644 --- a/skills/ocx/references/05_remote_hub.md +++ b/skills/ocx/references/05_remote_hub.md @@ -114,6 +114,12 @@ The ordering is not ceremony. If the old key died at issuance, a client that had received the new key would be disconnected — and a disconnected client cannot be given a new key. So the contract is: apply the new key, verify the connection, then commit. +Raw access-key creation and rotation-start return plaintext and belong outside the agent +session; follow [recipe 5](03_recipes.md#5-prepare-an-access-key-rotation-without-exposing-the-new-key) +for the human handoff and separate revocation approval. The managed `ocx connect rotate` +flow returns non-secret status and is a distinct command, not permission to invoke the raw +secret-returning endpoint from an agent tool. + The token backup (`.prev`) is not deleted while a rotation is in flight, and commits only once both sides are confirmed to have accepted. diff --git a/tests/ci-workflows/skill-ocx.test.ts b/tests/ci-workflows/skill-ocx.test.ts index 9293dd080a..dbd9693dcc 100644 --- a/tests/ci-workflows/skill-ocx.test.ts +++ b/tests/ci-workflows/skill-ocx.test.ts @@ -163,3 +163,135 @@ describe("the consent boundary is stated, not implied", () => { expect(recipes).toContain("get approval"); }); }); + +describe("access-key recipes keep plaintext outside agent sessions", () => { + // CLI oracle: access.ts removes one --json before checking exact commit/abort tokens. + // These canonical spellings are case-sensitive; commit-old-id is a start, not a commit. + const secretBearingAccessKeyCommand = + /\b(?:ocx|opencodex)(?:\.(?:exe|mjs|cmd|ps1))?["']?\s+(?:access\s+keys?|api-key)\s+(?:create\b|rotate\b(?!\s+(?:--json\s+)?(?:commit|abort)(?=\s|$)))/gm; + const secretBearingManagementRequest = + /(?:(?:\bPOST\b|(?:--request|-X|-Method)\s+["']?POST["']?|method\s*:\s*["']POST["'])[^\n]{0,240}\/api\/keys(?:\/rotate)?(?=$|[\s"'?#])|\/api\/keys(?:\/rotate)?(?=$|[\s"'?#])[^\n]{0,240}(?:\bPOST\b|(?:--request|-X|-Method)\s+["']?POST["']?|method\s*:\s*["']POST["']))/gim; + + /** + * Early warning for literal recipes in ordinary fences and single-backtick spans. + * Not a shell/JS parser: implicit POSTs, dynamic calls, alternate Markdown and + * arbitrary multiline requests remain outside this bounded detector. + */ + function secretBearingCommandsInCode(text: string): string[] { + const spans: string[] = []; + const prose = text.replace(/```[^\n]*\n([\s\S]*?)```/g, (_all: string, body: string) => { + spans.push(body); + return ""; + }); + for (const span of prose.matchAll(/`([^`\n]+)`/g)) spans.push(span[1]!); + const matches: string[] = []; + for (const span of spans) { + const executable = span.replace(/(?:\\|`|\^)\r?\n\s*/g, " "); + matches.push(...Array.from(executable.matchAll(secretBearingAccessKeyCommand), match => match[0])); + matches.push(...Array.from(executable.matchAll(secretBearingManagementRequest), match => match[0])); + } + return matches; + } + + test("all key aliases reject creation/start and preserve non-secret commit/abort", () => { + for (const binary of ["ocx", "opencodex"]) { + for (const group of ["access key", "access keys", "api-key"]) { + const prefix = `${binary} ${group}`; + for (const action of [ + "create rotated", "create rotated --json", + "rotate old-id", "rotate old-id --json", "rotate --json old-id", + ]) { + const command = `${prefix} ${action}`; + expect(secretBearingCommandsInCode("```bash\n" + command + "\n```"), command).toHaveLength(1); + } + for (const operation of ["commit", "abort"]) { + for (const args of [ + `${operation} old-id rotation-id`, + `${operation} old-id rotation-id --json`, + `--json ${operation} old-id rotation-id`, + ]) { + const command = `${prefix} rotate ${args}`; + expect(secretBearingCommandsInCode("```bash\n" + command + "\n```"), command).toEqual([]); + } + const start = `${prefix} rotate --json ${operation}-old-id`; + expect(secretBearingCommandsInCode("`" + start + "`"), start).toHaveLength(1); + } + } + } + }); + + test("wrappers, shell continuations and inline examples cannot hide literal commands", () => { + for (const command of [ + "& ocx access keys create rotated --json", + "command ocx access key create rotated", + "env ocx api-key rotate old-id", + "& 'C:\\Tools\\opencodex.exe' api-key rotate old-id", + "node /opt/bin/ocx.mjs access key create rotated", + "ocx.cmd access key create rotated", + "& './opencodex.ps1' access keys rotate old-id", + "ocx access key \\\n create rotated --json", + "ocx access key `\r\n create rotated --json", + "ocx access key ^\n rotate old-id", + "ocx access key rotate COMMIT", + ]) { + expect(secretBearingCommandsInCode("```bash\n" + command + "\n```"), command).toHaveLength(1); + } + expect(secretBearingCommandsInCode("Run `ocx api-key create rotated --json` next.")).toHaveLength(1); + expect(secretBearingCommandsInCode("Do not run `ocx api-key create rotated --json`.")).toHaveLength(1); + expect(secretBearingCommandsInCode("Creation under `ocx access key` returns plaintext.")).toEqual([]); + }); + + test("explicit management POST recipes are detected without banning commit or abort", () => { + for (const route of ["/api/keys", "/api/keys/rotate"]) { + for (const command of [ + `POST ${route}`, + `curl -X POST http://127.0.0.1:3000${route}`, + `curl 'http://127.0.0.1:3000${route}?source=recipe' --request POST`, + `curl --request POST \\\n 'http://127.0.0.1:3000${route}#example'`, + `Invoke-RestMethod http://127.0.0.1:3000${route} -Method Post`, + `Invoke-WebRequest -Method Post http://127.0.0.1:3000${route}`, + `fetch('${route}', { method: 'POST' })`, + ]) { + expect(secretBearingCommandsInCode("```text\n" + command + "\n```"), command).toHaveLength(1); + } + } + expect(secretBearingCommandsInCode("Run `POST /api/keys` next.")).toHaveLength(1); + for (const command of [ + "ocx access key list --json", + "ocx access key remove old-id --yes --json", + "ocx connect rotate --admin-token-stdin --json", + "curl -X POST http://127.0.0.1:3000/api/keys/rotate/commit", + "curl -X DELETE http://127.0.0.1:3000/api/keys/rotate", + "curl -X DELETE http://127.0.0.1:3000/api/keys", + "curl http://127.0.0.1:3000/api/keys\ncurl -X POST http://127.0.0.1:3000/api/keys/rotate/commit", + ]) { + expect(secretBearingCommandsInCode("```bash\n" + command + "\n```"), command).toEqual([]); + } + expect(secretBearingCommandsInCode("| POST | `/api/keys/rotate` |")).toEqual([]); + }); + + test("the original unsafe recipe is detected and every shipped page is scanned", () => { + const original = "```bash\nocx access key list --json\nocx access key create rotated --json\n" + + "ocx access key remove --yes --json\nocx access key list --json\n```"; + expect(secretBearingCommandsInCode(original)).toHaveLength(1); + for (const file of ["SKILL.md", ...REFERENCES.map(ref => join("references", ref))]) { + expect(secretBearingCommandsInCode(read(file)), file).toEqual([]); + } + }); + + test("guidance distinguishes configuration confirmation from revocation authority", () => { + // Documentation presence/order only: these assertions do not prove agent behavior. + const skill = readFileSync(SKILL, "utf8"); + const recipes = read("references/03_recipes.md"); + for (const text of [skill, recipes]) { + expect(text).toMatch(/outside the agent\s+session/); + expect(text).toMatch(/configuration confirmation is not (?:revocation )?approval/i); + expect(text).toMatch(/existing explicit\s+approval for that exact revocation remains valid/); + } + const approvalAt = recipes.indexOf("separate explicit revocation approval"); + expect(approvalAt).toBeGreaterThanOrEqual(0); + for (const command of ["ocx access key rotate commit", "ocx access key remove"]) { + expect(recipes.indexOf(command)).toBeGreaterThan(approvalAt); + } + }); +}); From 17c657c88784c6053d3185f26c01177a01f92f3c Mon Sep 17 00:00:00 2001 From: t Date: Mon, 7 Sep 2026 01:53:16 +0900 Subject: [PATCH 2/3] fix(diagnostics): distinguish spill ACL timeout origins [skip ci] Add closed origin and cumulative terminal-publication counters without changing ACL decisions, retry limits, memo handling, cancellation or readiness. Refs #3522; runtime recovery remains unresolved. Final stack CI pending. Co-authored-by: Ingwannu <186453546+Ingwannu@users.noreply.github.com> --- .../content/docs/reference/management-api.md | 2 +- .../docs/troubleshooting/windows-memory.md | 17 ++- src/lib/windows-secret-acl.ts | 12 +- src/responses/state.ts | 51 +++++++- tests/responses/responses-state.test.ts | 115 ++++++++++++++++++ tests/server/memory-watchdog.test.ts | 11 +- tests/windows/windows-secret-acl.test.ts | 88 +++++++++++--- 7 files changed, 267 insertions(+), 29 deletions(-) diff --git a/docs-site/src/content/docs/reference/management-api.md b/docs-site/src/content/docs/reference/management-api.md index 2cd23eb4bd..538f81e400 100644 --- a/docs-site/src/content/docs/reference/management-api.md +++ b/docs-site/src/content/docs/reference/management-api.md @@ -339,7 +339,7 @@ whether to star the repository. | Method and path | Purpose | Notable errors | | --- | --- | --- | -| `GET /api/system/memory` | Return scalar process, heap, stream, response-state, watchdog, and active-turn metrics. Response-state diagnostics include spill-write status, consecutive failures, fixed privacy-safe failure class, and last failure/success timestamps; raw errors and paths are never returned. | — | +| `GET /api/system/memory` | Return scalar process, heap, stream, response-state, watchdog, and active-turn metrics. Response-state diagnostics include spill-write status, consecutive failures, fixed privacy-safe failure class, and last failure/success timestamps. `spillLastWriteFailureOrigin` is `retry_returned_timeout`, `timeout_memo_refusal`, or null; cumulative `spillAclRetryReturnedTimeouts` and `spillAclTimeoutMemoRefusals` count terminal failed publications. See [Windows spill diagnostics](/troubleshooting/windows-memory/) for process-local semantics. Raw errors and paths are never returned. | — | | `POST /api/system/restart` | Begin a drain-aware process restart without removing client injection | Returns 202; repeated calls report the existing drain | | `POST /api/stop` | Stop the service, restore native Codex, remove managed Grok injection, and drain the proxy | 409 service ownership conflict; 409 `respawnable_service` when a Windows Task Scheduler wrapper could respawn the proxy and the caller is not `ocx stop` (nothing is changed); 409 when the installed manager refuses to stop; 409 `service_state_unknown` when the Task Scheduler state cannot be read (nothing is changed; repair the query and retry) | | `GET /api/system/codex-app-server` | Report whether running Codex app-servers predate the current model catalog | — | diff --git a/docs-site/src/content/docs/troubleshooting/windows-memory.md b/docs-site/src/content/docs/troubleshooting/windows-memory.md index cea5b3c233..10bc3799cc 100644 --- a/docs-site/src/content/docs/troubleshooting/windows-memory.md +++ b/docs-site/src/content/docs/troubleshooting/windows-memory.md @@ -57,7 +57,22 @@ runtime the leak itself remains an upstream problem: time show whether failures are accumulating or recovering in the same process. The last failure is a fixed privacy-safe class such as `EACCES`, `ENOSPC`, `ETIMEDOUT`, or `EACLRETRYEXHAUSTED`; raw error messages and filesystem paths - are never returned. These diagnostics stay on the authenticated management + are never returned. `spillLastWriteFailureOrigin` adds a fixed origin or null: + `retry_returned_timeout` means the existing second spill attempt returned a + timeout; `timeout_memo_refusal` means the ACL helper refused through its + remembered timeout state. Other failures use null. The cumulative + `spillAclRetryReturnedTimeouts` and `spillAclTimeoutMemoRefusals` count terminal + failed publications, not individual ACL commands or transient first attempts. + Success clears the failure streak but retains the last failure fields and + cumulative counts; a later unrelated failure sets the last origin to null. + These values are process-local, so compare snapshots from the same process. + Neither origin identifies an OS command: the attempt budget can expire before + a command starts, and an optional compliance inspection can run before a memo + refusal. A separate process succeeding does not prove that the live process's + memo recovered. These observations do not add retries, clear memos, weaken + required ACLs, or automatically restart the service. + + These diagnostics stay on the authenticated management endpoint and are intentionally absent from `/healthz`, which remains a liveness signal. The dashboard's **Memory observability** card renders the memory and continuation-size fields from this endpoint and offers a confirm-gated diff --git a/src/lib/windows-secret-acl.ts b/src/lib/windows-secret-acl.ts index dbf1f06b79..dc8bb5749f 100644 --- a/src/lib/windows-secret-acl.ts +++ b/src/lib/windows-secret-acl.ts @@ -715,18 +715,22 @@ function sanitizedAclError(diagnostics: string, cause: unknown): NodeJS.ErrnoExc return error; } -function previousTimeoutError(retryConsumed: boolean): NodeJS.ErrnoException { +type TimeoutMemoRefusalError = NodeJS.ErrnoException & { + aclFailureOrigin: "timeout_memo_refusal"; +}; + +function previousTimeoutError(retryConsumed: boolean): TimeoutMemoRefusalError { if (retryConsumed) { const error = new Error( "ACL hardening skipped — the previous timeout recovery was already consumed", ) as NodeJS.ErrnoException; error.code = "EACLRETRYEXHAUSTED"; - return error; + return Object.assign(error, { aclFailureOrigin: "timeout_memo_refusal" as const }); } - return sanitizedAclError( + return Object.assign(sanitizedAclError( "ACL hardening skipped — previous attempt timed out", Object.assign(new Error("timeout"), { code: "ETIMEDOUT" }), - ); + ), { aclFailureOrigin: "timeout_memo_refusal" as const }); } /** Consume, but never reset, the single explicit recovery attempt for this key. */ diff --git a/src/responses/state.ts b/src/responses/state.ts index e581653725..f9195196a2 100644 --- a/src/responses/state.ts +++ b/src/responses/state.ts @@ -169,7 +169,10 @@ async function snapshotOnDiskMatches(path: string, payload: string, payloadBytes return false; } } -const spillCounters = { writes: 0, writeFailures: 0, readFailures: 0 }; +const spillCounters = { + writes: 0, writeFailures: 0, readFailures: 0, + aclRetryReturnedTimeouts: 0, aclTimeoutMemoRefusals: 0, +}; export type ResponseSpillWriteFailureCode = | "EACLRETRYEXHAUSTED" @@ -184,9 +187,14 @@ export type ResponseSpillWriteFailureCode = export type ResponseSpillWriteStatus = "initial" | "healthy" | "degraded"; +export type ResponseSpillWriteFailureOrigin = + | "retry_returned_timeout" + | "timeout_memo_refusal"; + interface ResponseSpillWriteHealth { consecutiveFailures: number; lastFailureCode: ResponseSpillWriteFailureCode | null; + lastFailureOrigin: ResponseSpillWriteFailureOrigin | null; lastFailureAt: number | null; lastSuccessAt: number | null; } @@ -194,6 +202,7 @@ interface ResponseSpillWriteHealth { const spillWriteHealth: ResponseSpillWriteHealth = { consecutiveFailures: 0, lastFailureCode: null, + lastFailureOrigin: null, lastFailureAt: null, lastSuccessAt: null, }; @@ -226,6 +235,20 @@ function classifySpillWriteFailure(error: unknown): ResponseSpillWriteFailureCod return "EUNKNOWN"; } +/** The spill writer preserves ACL errors in cause; only a fixed memo marker is diagnostic. */ +function spillAclMemoRefusalOrigin(error: unknown): "timeout_memo_refusal" | null { + let cursor = error; + for (let depth = 0; depth < 4 && cursor && typeof cursor === "object"; depth += 1) { + const record = cursor as { code?: unknown; aclFailureOrigin?: unknown; cause?: unknown }; + if ((record.code === "ETIMEDOUT" || record.code === "EACLRETRYEXHAUSTED") + && record.aclFailureOrigin === "timeout_memo_refusal") { + return "timeout_memo_refusal"; + } + cursor = record.cause; + } + return null; +} + function noteSpillWriteSuccess(): void { spillCounters.writes += 1; spillWriteHealth.consecutiveFailures = 0; @@ -235,11 +258,20 @@ function noteSpillWriteSuccess(): void { function noteSpillWriteFailure( error: unknown, override?: ResponseSpillWriteFailureCode, + retryOrigin: ResponseSpillWriteFailureOrigin | null = null, ): void { + const code = override ?? classifySpillWriteFailure(error); + const origin = code === "ETIMEDOUT" || code === "EACLRETRYEXHAUSTED" + ? spillAclMemoRefusalOrigin(error) ?? retryOrigin + : null; spillCounters.writeFailures += 1; spillWriteHealth.consecutiveFailures += 1; - spillWriteHealth.lastFailureCode = override ?? classifySpillWriteFailure(error); + spillWriteHealth.lastFailureCode = code; + spillWriteHealth.lastFailureOrigin = origin; spillWriteHealth.lastFailureAt = now(); + // Count terminal publications, not ACL calls or a transient first attempt. + if (origin === "retry_returned_timeout") spillCounters.aclRetryReturnedTimeouts += 1; + else if (origin === "timeout_memo_refusal") spillCounters.aclTimeoutMemoRefusals += 1; } /** * Admission-boundary observability (test-visible). directSpills: oversized @@ -418,6 +450,7 @@ async function runPendingResponseSpill(job: PendingResponseSpill): Promise const candidate = job.candidate; let ref: ResponseSpillRef | null = null; let exhaustedAclRetry = false; + let aclRetryFailureOrigin: ResponseSpillWriteFailureOrigin | null = null; try { const state = spillPayloadForResident(candidate); try { @@ -437,6 +470,9 @@ async function runPendingResponseSpill(job: PendingResponseSpill): Promise }); } catch (retryError) { exhaustedAclRetry = isAclTimeout(retryError); + // A returned timeout can also mean an exhausted budget before the next OS command. + aclRetryFailureOrigin = spillAclMemoRefusalOrigin(retryError) + ?? (exhaustedAclRetry ? "retry_returned_timeout" : null); throw retryError; } } @@ -460,7 +496,7 @@ async function runPendingResponseSpill(job: PendingResponseSpill): Promise } catch (error) { if (ref) deleteResponseSpill(ref); if (states.get(job.id) === candidate && !job.cancelled) { - noteSpillWriteFailure(error, exhaustedAclRetry ? "EACLRETRYEXHAUSTED" : undefined); + noteSpillWriteFailure(error, exhaustedAclRetry ? "EACLRETRYEXHAUSTED" : undefined, aclRetryFailureOrigin); replaceWithSpillFailure(job.id, candidate); deferSupersededSpill(job.supersededSpill); } @@ -2188,6 +2224,9 @@ export interface ResponseStateMetrics { spillWriteStatus: ResponseSpillWriteStatus; spillWriteConsecutiveFailures: number; spillLastWriteFailureCode: ResponseSpillWriteFailureCode | null; + spillLastWriteFailureOrigin: ResponseSpillWriteFailureOrigin | null; + spillAclRetryReturnedTimeouts: number; + spillAclTimeoutMemoRefusals: number; spillLastWriteFailureAt: number | null; spillLastWriteSuccessAt: number | null; spillReadFailures: number; @@ -2240,6 +2279,9 @@ export function responseStateMetrics(): ResponseStateMetrics { : "initial", spillWriteConsecutiveFailures: spillWriteHealth.consecutiveFailures, spillLastWriteFailureCode: spillWriteHealth.lastFailureCode, + spillLastWriteFailureOrigin: spillWriteHealth.lastFailureOrigin, + spillAclRetryReturnedTimeouts: spillCounters.aclRetryReturnedTimeouts, + spillAclTimeoutMemoRefusals: spillCounters.aclTimeoutMemoRefusals, spillLastWriteFailureAt: spillWriteHealth.lastFailureAt, spillLastWriteSuccessAt: spillWriteHealth.lastSuccessAt, spillReadFailures: spillCounters.readFailures, @@ -2360,8 +2402,11 @@ export function clearResponseStateMemoryForTests(): void { spillCounters.writes = 0; spillCounters.writeFailures = 0; spillCounters.readFailures = 0; + spillCounters.aclRetryReturnedTimeouts = 0; + spillCounters.aclTimeoutMemoRefusals = 0; spillWriteHealth.consecutiveFailures = 0; spillWriteHealth.lastFailureCode = null; + spillWriteHealth.lastFailureOrigin = null; spillWriteHealth.lastFailureAt = null; spillWriteHealth.lastSuccessAt = null; replayScopeMismatchDrops = 0; diff --git a/tests/responses/responses-state.test.ts b/tests/responses/responses-state.test.ts index 8ba5da86b2..464642cda7 100644 --- a/tests/responses/responses-state.test.ts +++ b/tests/responses/responses-state.test.ts @@ -1079,6 +1079,9 @@ describe("Responses previous_response_id state", () => { spillWriteFailures: 0, spillWriteStatus: "healthy", spillWriteConsecutiveFailures: 0, + spillLastWriteFailureOrigin: null, + spillAclRetryReturnedTimeouts: 0, + spillAclTimeoutMemoRefusals: 0, }); }); @@ -1112,6 +1115,9 @@ describe("Responses previous_response_id state", () => { spillWriteConsecutiveFailures: 1, spillLastWriteFailureCode: "EACLRETRYEXHAUSTED", spillLastWriteSuccessAt: null, + spillLastWriteFailureOrigin: "retry_returned_timeout", + spillAclRetryReturnedTimeouts: 1, + spillAclTimeoutMemoRefusals: 0, }); expect(metrics.spillLastWriteFailureAt).toBeGreaterThanOrEqual(0); @@ -1127,11 +1133,65 @@ describe("Responses previous_response_id state", () => { spillWriteStatus: "healthy", spillWriteConsecutiveFailures: 0, spillLastWriteFailureCode: "EACLRETRYEXHAUSTED", + spillLastWriteFailureOrigin: "retry_returned_timeout", + spillAclRetryReturnedTimeouts: 1, + spillAclTimeoutMemoRefusals: 0, }); expect(typeof recovered.spillLastWriteSuccessAt === "number" && recovered.spillLastWriteSuccessAt >= (recovered.spillLastWriteFailureAt ?? 0)).toBe(true); }); + test("Windows stable-directory memo refusals stay distinct after the runner becomes healthy", async () => { + forceWindowsAclLane(); + const previousVerify = process.env.OPENCODEX_ACL_VERIFY_EXISTING; + delete process.env.OPENCODEX_ACL_VERIFY_EXISTING; + let clock = 0; + let grantCalls = 0; + setNowForTests(() => clock); + setResponseSpillNowForTests(() => clock); + setResponseSpillAsyncAclAttemptBudgetForTests(100); + setResponseStateByteCapForTests(1_024); + const spillDir = responseSpillDirectory(); + let healthy = false; + setAsyncIcaclsRunnerForTests(async args => { + if (args[0] !== spillDir) return ICACLS_OK; + if (args.includes("/grant:r")) grantCalls += 1; + if (healthy) return ICACLS_OK; + clock += 100; + return { success: false, exitCode: null, timedOut: true, stdout: "private-acl-output" }; + }); + try { + rememberLarge("resp_stable_timeout", "x".repeat(8_000)); + await flushPendingResponseSpillsForTests(); + expect(responseStateMetrics()).toMatchObject({ + spillWrites: 0, spillWriteFailures: 1, + spillLastWriteFailureCode: "EACLRETRYEXHAUSTED", + spillLastWriteFailureOrigin: "retry_returned_timeout", + spillAclRetryReturnedTimeouts: 1, spillAclTimeoutMemoRefusals: 0, + }); + expect(grantCalls).toBe(2); + healthy = true; // Same stable directory and process; no memo reset between jobs. + for (let refusal = 1; refusal <= 2; refusal += 1) { + rememberLarge(`resp_stable_refusal_${refusal}`, "y".repeat(8_000)); + await flushPendingResponseSpillsForTests(); + expect(responseStateMetrics()).toMatchObject({ + spillWrites: 0, spillWriteFailures: 1 + refusal, + spillWriteStatus: "degraded", spillWriteConsecutiveFailures: 1 + refusal, + spillLastWriteFailureCode: "EACLRETRYEXHAUSTED", + spillLastWriteFailureOrigin: "timeout_memo_refusal", + spillAclRetryReturnedTimeouts: 1, spillAclTimeoutMemoRefusals: refusal, + spillLastWriteSuccessAt: null, spillStubCount: 0, + }); + expect(grantCalls).toBe(2); + expect(spillFileNames(home)).toHaveLength(0); + expect(spillTempNames(home)).toHaveLength(0); + } + } finally { + if (previousVerify === undefined) delete process.env.OPENCODEX_ACL_VERIFY_EXISTING; + else process.env.OPENCODEX_ACL_VERIFY_EXISTING = previousVerify; + } + }); + test("Windows async spill attempts share one bounded ACL budget across every harden", async () => { forceWindowsAclLane(); let clock = 0; @@ -2591,6 +2651,7 @@ describe("Responses previous_response_id state", () => { const { spillWriteStatus, spillLastWriteFailureCode, + spillLastWriteFailureOrigin, spillLastWriteFailureAt, spillLastWriteSuccessAt, ...numericMetrics @@ -2599,6 +2660,7 @@ describe("Responses previous_response_id state", () => { .every(value => typeof value === "number" && Number.isFinite(value))).toBe(true); expect(spillWriteStatus).toBe("healthy"); expect(spillLastWriteFailureCode).toBeNull(); + expect(spillLastWriteFailureOrigin).toBeNull(); expect(spillLastWriteFailureAt).toBeNull(); expect(typeof spillLastWriteSuccessAt === "number" && Number.isFinite(spillLastWriteSuccessAt)).toBe(true); const serialized = JSON.stringify(metrics); @@ -3359,6 +3421,9 @@ describe("Responses previous_response_id state", () => { spillWriteStatus: "initial", spillWriteConsecutiveFailures: 0, spillLastWriteFailureCode: null, + spillLastWriteFailureOrigin: null, + spillAclRetryReturnedTimeouts: 0, + spillAclTimeoutMemoRefusals: 0, spillLastWriteFailureAt: null, spillLastWriteSuccessAt: null, spillReadFailures: 0, @@ -3366,6 +3431,53 @@ describe("Responses previous_response_id state", () => { }); }); + test("spill failure origin decoding stays bounded, closed and paired with the effective code", () => { + setResponseStateByteCapForTests(1_024); + const memoError = Object.assign(new Error("private-path-and-payload"), { + code: "ETIMEDOUT", aclFailureOrigin: "timeout_memo_refusal", + }); + const cycle: { code: string; cause?: unknown; aclFailureOrigin: string } = { + code: "ETIMEDOUT", aclFailureOrigin: "private-origin", + }; + cycle.cause = cycle; + const cases = [ + { error: new Error("wrapper", { cause: memoError }), code: "ETIMEDOUT", origin: "timeout_memo_refusal" }, + { error: Object.assign(new Error("denied", { cause: memoError }), { code: "EACCES" }), code: "EACCES", origin: null }, + { error: { code: "EACLRETRYEXHAUSTED" }, code: "EACLRETRYEXHAUSTED", origin: null }, + { error: { code: "ETIMEDOUT", aclFailureOrigin: "private-origin" }, code: "ETIMEDOUT", origin: null }, + { error: { code: "ETIMEDOUT", aclFailureOrigin: ["timeout_memo_refusal"] }, code: "ETIMEDOUT", origin: null }, + { error: cycle, code: "ETIMEDOUT", origin: null }, + // Including the writer's wrapper, the marker is beyond the four-object scan. + { error: { code: "ETIMEDOUT", cause: { cause: { cause: memoError } } }, code: "ETIMEDOUT", origin: null }, + ]; + cases.forEach(({ error, code, origin }, index) => { + setSpillIoForTest({ write: () => { throw error; } }); + rememberLarge(`resp_private_origin_${index}`, "private-content".repeat(1_000)); + const metrics = responseStateMetrics(); + expect(metrics).toMatchObject({ + spillWriteFailures: index + 1, + spillLastWriteFailureCode: code, + spillLastWriteFailureOrigin: origin, + spillAclRetryReturnedTimeouts: 0, spillAclTimeoutMemoRefusals: 1, + }); + const serialized = JSON.stringify(metrics); + for (const privateValue of ["private-path-and-payload", "private-origin", "private-content", "resp_private_origin", home]) { + expect(serialized).not.toContain(privateValue); + } + }); + setSpillIoForTest(null); + rememberLarge("resp_after_origin_failures", "healthy".repeat(1_500)); + expect(responseStateMetrics()).toMatchObject({ + spillWriteStatus: "healthy", spillWriteConsecutiveFailures: 0, + spillLastWriteFailureCode: "ETIMEDOUT", spillLastWriteFailureOrigin: null, + spillAclRetryReturnedTimeouts: 0, spillAclTimeoutMemoRefusals: 1, + }); + clearResponseStateMemoryForTests(); + expect(responseStateMetrics()).toMatchObject({ + spillLastWriteFailureOrigin: null, spillAclRetryReturnedTimeouts: 0, spillAclTimeoutMemoRefusals: 0, + }); + }); + test("a successful spill clears a repeated failure streak without erasing the last failure", () => { const realNow = Date.now; let clock = 1_000; @@ -3471,6 +3583,9 @@ describe("Responses previous_response_id state", () => { spillWriteStatus: "initial", spillWriteConsecutiveFailures: 0, spillLastWriteFailureCode: null, + spillLastWriteFailureOrigin: null, + spillAclRetryReturnedTimeouts: 0, + spillAclTimeoutMemoRefusals: 0, spillLastWriteFailureAt: null, spillLastWriteSuccessAt: null, spillReadFailures: 0, diff --git a/tests/server/memory-watchdog.test.ts b/tests/server/memory-watchdog.test.ts index 80c38dd020..918503456c 100644 --- a/tests/server/memory-watchdog.test.ts +++ b/tests/server/memory-watchdog.test.ts @@ -196,6 +196,9 @@ describe("GET /api/system/memory", () => { spillWriteStatus: "initial" | "healthy" | "degraded"; spillWriteConsecutiveFailures: number; spillLastWriteFailureCode: string | null; + spillLastWriteFailureOrigin: string | null; + spillAclRetryReturnedTimeouts: number; + spillAclTimeoutMemoRefusals: number; spillLastWriteFailureAt: number | null; spillLastWriteSuccessAt: number | null; replayScopeMismatchDrops: number; @@ -221,11 +224,12 @@ describe("GET /api/system/memory", () => { // responseState is a scalar-only continuation-store attribution block: numbers plus fixed // enum/null fields (no paths, messages, tokens, or account identifiers). // The exact count is pinned on purpose: a new field must be reviewed for privacy safety - // before it reaches this surface. 17 after #3522 added spill-write health diagnostics. - expect(Object.keys(body.responseState)).toHaveLength(17); + // before it reaches this surface. 20 after #3522 added failure origins and counters. + expect(Object.keys(body.responseState)).toHaveLength(20); const { spillWriteStatus, spillLastWriteFailureCode, + spillLastWriteFailureOrigin, spillLastWriteFailureAt, spillLastWriteSuccessAt, ...numericResponseState @@ -233,6 +237,9 @@ describe("GET /api/system/memory", () => { expect(Object.values(numericResponseState) .every(value => typeof value === "number" && Number.isFinite(value))).toBe(true); expect(["initial", "healthy", "degraded"]).toContain(spillWriteStatus); + expect(spillLastWriteFailureOrigin === null || [ + "retry_returned_timeout", "timeout_memo_refusal", + ].includes(spillLastWriteFailureOrigin)).toBe(true); expect(spillLastWriteFailureCode === null || [ "EACLRETRYEXHAUSTED", "ETIMEDOUT", "EACCES", "ENOSPC", "EFBIG", "EIO", "ECAPACITY", "ELOOP", "EUNKNOWN", diff --git a/tests/windows/windows-secret-acl.test.ts b/tests/windows/windows-secret-acl.test.ts index dddbd38084..aa011bb516 100644 --- a/tests/windows/windows-secret-acl.test.ts +++ b/tests/windows/windows-secret-acl.test.ts @@ -376,6 +376,42 @@ describe("opt-in existing ACL proof", () => { expect(calls).toEqual([[target]]); }); + test("async compliance inspection can precede a memo refusal or an existing compliant success", async () => { + const target = join(testDir, "memo-compliance.json"); + writeFileSync(target, "secret"); + let clock = 0; + setNowForTests(() => clock); + setAsyncWindowsPrincipalRunnerForTests(async () => success(`${ownerSid}\n${ownerName}\n`)); + seedIdentity(); + delete process.env.OPENCODEX_ACL_VERIFY_EXISTING; + setAsyncIcaclsRunnerForTests(async () => { + clock += 100; + return { success: false, exitCode: null, timedOut: true, stdout: "" }; + }); + try { + await expect(hardenSecretPathAsync(target, { required: true, deadlineMs: 100 })) + .rejects.toMatchObject({ code: "ETIMEDOUT" }); + await expect(hardenSecretPathAsync(target, { required: true, deadlineMs: 100, retryTimedOutOnce: true })) + .rejects.toMatchObject({ code: "ETIMEDOUT" }); + process.env.OPENCODEX_ACL_VERIFY_EXISTING = "1"; + const calls: string[][] = []; + let compliant = false; + setAsyncIcaclsRunnerForTests(async args => { + calls.push(args); + return success(compliant ? `${target} ${ownerName}:(F)\r\n` : "unverified"); + }); + await expect(hardenSecretPathAsync(target, { required: true, deadlineMs: 100 })) + .rejects.toMatchObject({ code: "EACLRETRYEXHAUSTED", aclFailureOrigin: "timeout_memo_refusal" }); + expect(calls).toEqual([[target]]); // Inspection ran, but no grant was launched. + compliant = true; + await expect(hardenSecretPathAsync(target, { required: true, deadlineMs: 100 })).resolves.toEqual({ ok: true }); + expect(calls).toEqual([[target], [target]]); + expect(timedOutSecretPathCountForTests()).toBe(1); // Existing proof did not clear the memo. + } finally { + setNowForTests(null); + } + }); + test("an inherited owner ACE falls through to the mutation sequence", () => { const target = join(testDir, "inherited.json"); writeFileSync(target, "secret"); @@ -1014,7 +1050,7 @@ describe("async hardenSecretPath (issue #612)", () => { expect(timedOutSecretPathCountForTests()).toBe(0); }); - test("the explicit timeout recovery cannot be consumed more than once", async () => { + test.each(["sync", "async"] as const)("%s timeout origin distinguishes memo refusal without another recovery", async lane => { // Pinned: this asserts recovery CARDINALITY. At the 30s default the first call would // succeed on its internal retry and the cardinality claim would never be exercised. process.env.OPENCODEX_ACL_TIMEOUT_MS = "5000"; @@ -1022,27 +1058,43 @@ describe("async hardenSecretPath (issue #612)", () => { let now = 0; let grantCalls = 0; setNowForTests(() => now); - setAsyncIcaclsRunnerForTests(async args => { + const runner = (args: string[]): IcaclsResult => { if (args.includes("/grant:r")) grantCalls += 1; now += 5_000; return timeout; - }); + }; + setIcaclsRunnerForTests(runner); + setAsyncIcaclsRunnerForTests(async args => runner(args)); + const identity = { ...ok, stdout: "S-1-5-21-1-2-3-1001\nocx-test\n" }; + setWindowsPrincipalRunnerForTests(() => identity); + setAsyncWindowsPrincipalRunnerForTests(async () => identity); + const harden = async (retryTimedOutOnce = false) => lane === "sync" + ? hardenSecretPath(target, { required: true, retryTimedOutOnce }) + : hardenSecretPathAsync(target, { required: true, retryTimedOutOnce }); - await expect(hardenSecretPathAsync(target, { required: true })).rejects.toMatchObject({ - code: "ETIMEDOUT", - }); - await expect(hardenSecretPathAsync(target, { - required: true, - retryTimedOutOnce: true, - })).rejects.toMatchObject({ code: "ETIMEDOUT" }); - const callsAfterRecovery = grantCalls; - await expect(hardenSecretPathAsync(target, { - required: true, - retryTimedOutOnce: true, - })).rejects.toMatchObject({ code: "EACLRETRYEXHAUSTED" }); - expect(grantCalls).toBe(callsAfterRecovery); - expect(grantCalls).toBe(2); - expect(timedOutSecretPathCountForTests()).toBe(1); + try { + const first = await harden().catch(error => error); + expect(first).toMatchObject({ code: "ETIMEDOUT" }); + expect(first).not.toHaveProperty("aclFailureOrigin"); + await expect(harden()).rejects.toMatchObject({ + code: "ETIMEDOUT", aclFailureOrigin: "timeout_memo_refusal", + }); + expect(grantCalls).toBe(1); + const recovery = await harden(true).catch(error => error); + expect(recovery).toMatchObject({ code: "ETIMEDOUT" }); + expect(recovery).not.toHaveProperty("aclFailureOrigin"); + const callsAfterRecovery = grantCalls; + await expect(harden(true)).rejects.toMatchObject({ + code: "EACLRETRYEXHAUSTED", aclFailureOrigin: "timeout_memo_refusal", + }); + expect(grantCalls).toBe(callsAfterRecovery); + expect(grantCalls).toBe(2); + expect(timedOutSecretPathCountForTests()).toBe(1); + } finally { + setWindowsPrincipalRunnerForTests(null); + setAsyncWindowsPrincipalRunnerForTests(null); + resetWindowsPrincipalForTests(); + } }); test("optional timeout memo does not poison a later required harden of the same path", () => { From 7aceb0a9d1c24079f51afd6dee9f22170fde2f40 Mon Sep 17 00:00:00 2001 From: t Date: Mon, 7 Sep 2026 02:29:06 +0900 Subject: [PATCH 3/3] test(state): pin the reviewed memory field allowlist [skip ci] Final CI 34047664926 macOS job 101526036532 found one remaining 17-field assertion. Assert the complete reviewed 20-field API contract instead of only relaxing its count; unexpected private/replay fields still fail. Production projection is unchanged. --- tests/responses/continuation-dedup.test.ts | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/tests/responses/continuation-dedup.test.ts b/tests/responses/continuation-dedup.test.ts index 5e9efd2a74..c5b55b10fd 100644 --- a/tests/responses/continuation-dedup.test.ts +++ b/tests/responses/continuation-dedup.test.ts @@ -310,9 +310,17 @@ describe("replay overlap: contracts held elsewhere", () => { }); test("the skip counter is not published on the memory surface", () => { - // /api/system/memory pins exactly 17 privacy-reviewed scalar fields. The five - // spill-health additions are enums, counters, or timestamps — never error text. - expect(Object.keys(responseStateMetrics())).toHaveLength(17); + // Pin the reviewed public fields, not just their count: no replay-skip + // counter or arbitrary diagnostic may replace a permitted field unnoticed. + expect(Object.keys(responseStateMetrics()).sort()).toEqual([ + "count", "residentCount", "spillStubCount", "tombstoneCount", + "totalBytes", "spillPayloadBytes", "largestBytes", "oldestAgeMs", + "spillWrites", "spillWriteFailures", "spillReadFailures", + "spillWriteStatus", "spillWriteConsecutiveFailures", + "spillLastWriteFailureCode", "spillLastWriteFailureOrigin", + "spillAclRetryReturnedTimeouts", "spillAclTimeoutMemoRefusals", + "spillLastWriteFailureAt", "spillLastWriteSuccessAt", "replayScopeMismatchDrops", + ].sort()); }); test("clearing state for tests resets the skip counter", () => {