fix(vault): validate credential input safely

[steipete]

Summary

This is the bounded follow-up preserved when contributor PR #287 was superseded by the narrower DCR compatibility fix in #288.

• Convert malformed vault set JSON into source-specific usage errors that never include V8 parser text or credential-input prefixes.
• Validate tokens.expires_at and tokens.expiresAt as finite numbers at the same input boundary that already validates expires_in.

No client-information policy, token persistence format, refresh behavior, or token schema is redesigned here.

Behavior

Malformed stdin now reports only:

Vault payload from stdin is not valid JSON.

Malformed files report only the file path:

Vault payload file '<path>' is not valid JSON.

The original parser reason is deliberately discarded because V8 may quote a prefix of credential input in the diagnostic.

Expiry validation mirrors the existing isStoredOAuthTokens read guard exactly. expires_in, expires_at, and expiresAt accept any finite number, including zero, negative values, and fractions. Strings, null/serialized NaN, and numeric overflow to positive or negative infinity reject. If both aliases are present and finite, both remain accepted; this PR does not invent conflict or timestamp policy.

Unknown token fields, valid DCR client information, null-compatible client fields, and provider metadata still pass through unchanged.

Provenance

The parsing wrapper and expiry-field analysis are materially adapted from @Yigtwxx's commits in #287, especially df2956fdfb6ad9a645c706393398a5efdcd30805 and 2733f0c98895e781db609243e294792bae321f0a. Contributor credit is preserved in the commit trailer and changelog.

PR #288 intentionally did not carry these adjacent findings because it retained partial-object and null-compatible client information semantics while resolving issue #286. This PR is the separate follow-up promised in the #287 close comment.

Proof

Exact head: 38168dc5b48281d44fdec0e790be69601b82b8fb.

Built CLI redaction proof with a distinctive fake marker:

[mcporter] Vault payload from stdin is not valid JSON.

The marker, its prefix, SyntaxError, parser reason, and source contents were absent. The built CLI also rejected 1e999 as non-finite and persisted both expires_at: 0 and fractional expiresAt, alongside unrelated fake token/client metadata.

• Focused built/unit vault suite: 3 files, 40 tests passed
• pnpm docs:list — passed
• pnpm check — passed
• pnpm test — 187 files passed, 4 skipped; 1,438 tests passed, 26 skipped
• pnpm docs:site — passed
• git diff --check — passed
• Codex-backed autoreview — clean, no accepted/actionable findings
• exact-diff TruffleHog scan — 0 verified/unknown findings
• public model-identifier audit — PASS; this is not a model-bearing change

No live provider proof is required: the changed boundary is local JSON parsing, validation, and persistence using fake credentials only.

[steipete]

Exact-head maintainer proof for 38168dc5b48281d44fdec0e790be69601b82b8fb.

Redaction behavior

Current main reproduced the leak with fake credential input: V8's SyntaxError included a quoted prefix of the input and the unexpected-error stack reached stderr.

The built exact-head CLI received the same distinctive fake marker and emitted only:

[mcporter] Vault payload from stdin is not valid JSON.

The complete marker, its partial prefix, parser reason, source contents, SyntaxError, and stack were absent. The file path emits Vault payload file '<path>' is not valid JSON. without echoing file contents.

Expiry boundary and compatibility

• String expires_at / expiresAt values reject.
• JSON numeric overflow (1e999 / -1e999) rejects as non-finite.
• A JSON NaN literal rejects at the sanitized malformed-JSON boundary; a programmatic NaN shape serializes to null and is rejected by the same finite-number guard.
• expires_at: 0 and fractional expiresAt values persist and remain readable.
• Both aliases may coexist when finite, matching the existing read guard without new conflict policy.
• Unknown token fields, valid DCR arrays/numbers, null-compatible client fields, and provider metadata remain intact.

Commands and results

• pnpm exec vitest run tests/vault-validation.test.ts tests/vault-command.test.ts tests/vault-cli.integration.test.ts --reporter=verbose — 3 files, 40 tests passed.
• pnpm docs:list — passed.
• pnpm check — passed.
• pnpm test — 187 files passed, 4 skipped; 1,438 tests passed, 26 skipped.
• pnpm docs:site — passed.
• git diff --check — passed.
• Codex-backed autoreview of the complete branch diff — clean, no accepted/actionable findings.
• TruffleHog exact-diff scan — 0 verified/unknown findings.
• Public model-identifier audit — PASS; this is not a model-bearing change.

Exact-head CI run 31281799202 passed on macOS, Ubuntu, and Windows. GitHub reports CLEAN/MERGEABLE.

Contributor credit: the parsing wrapper and expiry-field analysis are materially adapted from @Yigtwxx's superseded PR #287; the commit carries Co-authored-by: Yigtwxx <yigiterdogan023@gmail.com> and the 0.13.1 changelog thanks them. This is the promised separate follow-up to #288. Gaps: none.

[clawsweeper]

Codex review: needs maintainer review before merge. Reviewed August 8, 2026, 6:33 PM ET / 22:33 UTC.

ClawSweeper review

What this changes

The PR converts malformed vault JSON into source-specific safe errors and rejects non-finite expires_at and expiresAt token values before persistence.

Regression provenance

Possible regression — probable (reviewed change; failure trace). No predecessor PR is attributed.

Merge readiness

⚠️ Ready for maintainer review - 1 item remains

Keep this PR open for normal maintainer merge review. The patch narrowly prevents credential fragments from reaching parser diagnostics and aligns write-time expiry validation with the existing vault read guard; no actionable patch defect was found.

Priority: P2
Reviewed head: 38168dc5b48281d44fdec0e790be69601b82b8fb

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	A focused, well-covered vault-boundary repair with convincing real CLI proof.
Proof confidence	🦞 diamond lobster (5/6)	Sufficient (terminal): The PR body provides a redacted built-CLI after-fix transcript covering safe diagnostics, invalid numeric input, and successful persistence of valid values.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Verified	Sufficient (terminal): The PR body provides a redacted built-CLI after-fix transcript covering safe diagnostics, invalid numeric input, and successful persistence of valid values.
Evidence reviewed	6 items	Current-main gap: Current main parses vault input directly, while its stored-token guard already requires both expiry aliases to be finite numbers; malformed input can therefore escape as a raw parser error and invalid aliases can be persisted. Narrow source repair: The proposed parser discards V8 diagnostics and the validator applies the same finite-number requirement to all three accepted expiry fields. Regression coverage: The added built-CLI suite verifies both input sources, absence of fake credential markers from diagnostics, non-finite JSON values, and persistence of valid finite aliases.
Findings	None	None.
Security	None	None.

How this fits together

The vault command accepts OAuth tokens and client metadata from stdin or a file, validates them, and stores them for later OAuth use. Input errors reach terminal or CI logs, while stored token expiry fields are consumed by vault and refresh paths.

flowchart LR
  A[Vault input file or stdin] --> B[JSON parsing]
  B --> C[Payload validation]
  C --> D[Credential vault storage]
  D --> E[OAuth refresh and runtime]
  B --> F[Safe usage error]

Loading

Before merge

• Complete next step (P2) - No mechanical repair is indicated; the PR has sufficient proof and needs ordinary maintainer merge review.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Implementation and regression coverage	production +26/-11, tests +206, changelog +1	The small command-boundary change is covered at both direct validation and built-CLI levels.

Technical review

Best possible solution:

Merge the focused parser-redaction and expiry-boundary repair while retaining the existing permissive handling of valid OAuth metadata.

Do we have a high-confidence way to reproduce the issue?

Yes—current main has a direct source-reproducible path: malformed vault JSON reaches raw JSON.parse, and non-finite expiry aliases pass the write validator despite failing the stored-token guard.

Is this the best way to solve the issue?

Yes—the patch fixes both defects at the vault input boundary, preserves accepted finite values and metadata, and adds direct and built-CLI regressions.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 607ec3e342ac.

Labels

Label changes:

• add P2: This is a bounded credential-input safety and validation repair with limited blast radius.
• add proof: sufficient: Contributor real behavior proof is sufficient. The PR body provides a redacted built-CLI after-fix transcript covering safe diagnostics, invalid numeric input, and successful persistence of valid values.
• add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
• add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body provides a redacted built-CLI after-fix transcript covering safe diagnostics, invalid numeric input, and successful persistence of valid values.

Label justifications:

• P2: This is a bounded credential-input safety and validation repair with limited blast radius.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body provides a redacted built-CLI after-fix transcript covering safe diagnostics, invalid numeric input, and successful persistence of valid values.
• proof: sufficient: Contributor real behavior proof is sufficient. The PR body provides a redacted built-CLI after-fix transcript covering safe diagnostics, invalid numeric input, and successful persistence of valid values.

Evidence

What I checked:

• Current-main gap: Current main parses vault input directly, while its stored-token guard already requires both expiry aliases to be finite numbers; malformed input can therefore escape as a raw parser error and invalid aliases can be persisted. (src/cli/vault-command.ts:47, 607ec3e342ac)
• Narrow source repair: The proposed parser discards V8 diagnostics and the validator applies the same finite-number requirement to all three accepted expiry fields. (src/cli/vault-command.ts:109, 38168dc5b482)
• Regression coverage: The added built-CLI suite verifies both input sources, absence of fake credential markers from diagnostics, non-finite JSON values, and persistence of valid finite aliases. (tests/vault-cli.integration.test.ts:49, 38168dc5b482)
• Feature provenance: Headless OAuth vault seeding originated in the prior feature commit, and the most recent merged vault-validation work was merged as the DCR compatibility fix. (src/cli/vault-command.ts:47, 4e8e37df2004)
• Current-main status: The PR head is not an ancestor of main, so its correction is not already included in the checked-out default branch. (src/cli/vault-command.ts:49, 38168dc5b482)
• After-fix behavior proof: The PR body records a built CLI run using fake credentials: malformed input emitted only the safe usage error, non-finite values were rejected, and valid zero/fractional aliases persisted. (38168dc5b482)

Likely related people:

• steipete: Introduced headless OAuth vault seeding and has continued work on the same vault command boundary. (role: feature originator and current contributor; confidence: high; commits: a64e29b4fea3, 38168dc5b482; files: src/cli/vault-command.ts, src/oauth-vault.ts)
• feniix: Authored the merged DCR compatibility update immediately preceding this focused follow-up. (role: recent vault-validation contributor; confidence: high; commits: 4e8e37df2004; files: src/cli/vault-command.ts, tests/vault-command.test.ts)

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.