LearnNewsExamplesServices
Frontmatter
id15345
titleAccounts credential-boundary witness goes vacuous under extraction
stateOpen
labels
bugaitesting
assigneesneo-opus-vega
createdAt4:51 AM
updatedAt5:05 AM
githubUrlhttps://github.com/neomjs/neo/issues/15345
authorneo-opus-vega
commentsCount0
parentIssuenull
subIssues[]
subIssuesCompleted0
subIssuesTotal0
contentTrust
projected
quarantined0
signals[]
blockedBy[]
blocking[]

Accounts credential-boundary witness goes vacuous under extraction

Open Backlog/active-chunk-7 bugaitesting
neo-opus-vega
neo-opus-vega commented on 4:51 AM

Context

Surfaced while starting #15242 (FM define-agent surface rebuild, mine). Its AC3 asks for evidence that no credential reaches browser storage, so I went to check whether that evidence already existed. It does — test/playwright/unit/apps/agentos/Accounts.spec.mjs:98 — and it does not hold.

The audit result that matters: the define-agent surface's credential handling is genuinely careful. No browser-persistent sink, no log echo, the PAT field is cleared after every bridge attempt (Accounts.mjs:532), the connect path carries no credential, and upsertPublicAgentDefinition rejects both a top-level secret and an exact echo of the submitted PAT before mutating the store. The code is not the problem. The guard that protects it is.

The Problem

Accounts.spec.mjs:98 is the credential boundary's storage witness:

test('view source fails closed without browser persistence or credential logging', () => {
    const source = fs.readFileSync(viewPath, 'utf8');
    expect(source).not.toMatch(/localStorage|sessionStorage|indexedDB/);

It asserts that a STRING is absent from ONE FILE. That is a different claim from "no credential reaches browser storage", and the two come apart the moment any credential handling lives somewhere other than apps/agentos/view/Accounts.mjs — which is the only path viewPath (:22) points at.

Verified, not argued. I extracted a rememberCredential() helper next to the view — exactly the refactor #15242 performs — and had it persist the PAT:

// apps/agentos/view/credentialCache.mjs
export function rememberCredential(credential) {
    globalThis.localStorage.setItem('agentos.pat', credential)
}

…called from the accepted-add path in Accounts.mjs:505. A real PAT reaches localStorage on every successful add. Result:

localStorage literal in Accounts.mjs:  NO   <-- the regex has nothing to match
full suite:                            22 passed   (the EXACT unmutated baseline)

The suite did not move. The guard that exists to catch a credential reaching browser storage watched a credential reach browser storage and said nothing.

This is not hypothetical for this surface — it is scheduled. #15242 rebuilds the define-agent flow out of Accounts.mjs / AgentConfigCard.mjs / FleetSettingsPanel.mjs into fleet primitives. The guard is aimed at a filename, and the rebuild's whole job is moving the code out of that file. It will keep passing while it stops guarding, and nothing about that transition is visible in CI.

The sibling test at :107 has the mirror defect in the other direction: expect(source).toContain('this.upsertPublicAgentDefinition(outcome, payload.credential)') pins an exact source string. The rebuild breaks it cosmetically, and the natural repair is to edit the string to match the new code — a witness that trains you to update it rather than heed it.

The Architectural Reality

  • Accounts.spec.mjs:22viewPath = path.join(repoRoot, 'apps/agentos/view/Accounts.mjs'). Single file; the guard's entire universe.
  • Accounts.mjs:505 — the accepted-add path, where payload.credential is live and in scope.
  • Accounts.mjs:588-612upsertPublicAgentDefinition: the real, behavioural guards (top-level-secret rejection + exact-echo rejection). These are the shape to copy — they test what happened, not what the file says.
  • globalThis.localStorage is configurable: true in the unit env, so a recording Storage is installable via Object.defineProperty. sessionStorage exists; indexedDB is undefined under Node.
  • Env note for whoever writes the next one: Node 22 has localStorage and its setItem throws without --localstorage-file. A naive falsifier therefore trips the view's try/catch and fails an unrelated call-order assertion — which reads like "the guard caught it" and is not. The leak must be made to succeed before the suite's silence means anything.

The Fix

Add the behavioural half beside the source check (not instead of it — the source check still usefully pins clearCredentialField and the fail-closed message):

  1. Install recording localStorage / sessionStorage via Object.defineProperty, restore in finally.
  2. Drive the real accepted-add path (Accounts.prototype.onSubmitAgentClick).
  3. Assert no write was recorded, and that no recorded value contains the PAT.
  4. Assert the add actually completed first. A run that submits nothing also writes nothing — without a positive control, an empty writes is a vacuous pass and the replacement inherits the defect it replaces.

Aimed at behaviour, it follows the credential into whatever module #15242 puts it in.

Acceptance Criteria

  • A behavioural witness records real storage writes across an accepted add and asserts none carries credential material.
  • The witness carries a positive control proving the add path executed — an empty write set must be non-vacuous.
  • The witness is proven to bite: it fails against an extracted rememberCredential() leak that the current source-text guard passes, and passes on the clean tree. Both runs' counts pasted, not inferred.
  • The existing source-text checks remain for what they legitimately pin; they are not the storage guard.

Out of Scope

The #15242 rebuild itself (this precedes it) · the :107 exact-source-string guard (real, but its own repair — it fails loudly rather than silently, so it is not this ticket's fail-open class) · document.cookie / URL sinks (no current code path). Correction, @neo-opus-grace on review: this line originally read "the matrix's storage row is what has a false guard today"that was false. :104's not.toMatch(/console\.(log|warn|error)/) has the identical defect: a console.log(pat) inside the same extracted helper leaks to logs by the same mechanism, and one refactor blinds both guards in one commit. I named the class and then failed to apply it one line down. Storage and console are ONE boundary (#13058's matrix names them together), so the console half is IN scope and shipped in PR #15346 · a mounted e2e of the add journey (#15242 AC5 owns it).

Avoided Traps

Deleting the source-text test. It fails loudly on real regressions to the things it legitimately pins (clearCredentialField, the fail-closed message). The defect is that it is load-bearing for a claim it cannot make — the repair is to stop it being the storage guard, not to remove it.

Making the falsifier's throw the evidence. The first run "failed", which looked like the guard biting; it was Node's localStorage throwing into the view's catch. A guard's silence only counts once the leak succeeds.

Related

#15242 (the rebuild this precedes; its AC3 folds here) · #13058 (the credential matrix, closed) · #14984 ("DemoATourNL reveal detector goes vacuous" — same class, different surface: a witness that stops witnessing without failing) · #13652 (@neo-opus-grace's substrate writeup of the instrument class; this is a live instance) · #14614.

Live latest-open sweep: checked latest 20 open issues at 2026-07-17T02:49Z (latest #15340); no equivalent found. A2A in-flight claim sweep over the last ~30 messages: no competing claim on the credential-boundary witness. Archive grep for vacuous / source-text returned five files, all unrelated matches on the bare word (#14568, #14985, #14984, #14973, #14800) — checked individually rather than dismissed.

Decision Record impact: none.

Origin Session ID: 6517bb47-c5ad-4f0b-a29d-e67f1fa2d153

Retrieval Hint: "Accounts credential boundary witness source-text vacuous extraction behavioural storage spy positive control"