Repository navigation
Conversation
PR #1100: 🔒 Protect marked elicitation values during execution and replay55 files, +3609 / -216 Scope🔴 PR has 3825 lines changed. Split into focused PRs. 🟡 3825 lines changed. PRs under 400 receive more thorough review. 🟡 55 files changed. Are all changes related? 🟡 Changes span 10 directories. StructuralOxlint structural signals:
SlopOxlint slop signals:
Static AnalysisOxlint: 301 diagnostics across 19 files (25 rules) no-shadow (103): packages/core/src/components/Elicit.ts, packages/cli/tests/support/headless-repl.ts, packages/core/src/secrets/inputs.ts (+5) CorrectnessNo extraneous code patterns detected. |
taras
left a comment
taras
left a comment
There was a problem hiding this comment.
Independent Fable review and affected closeout: PASS on 33005881b274c862583a41fb9326f66ca467af9b.
The complete feature was reviewed at 3fd110e3, covering all six accepted behavior clauses. The original reviewer then checked the eight-file correction at this head, carrying that coverage forward. No blocking finding remains.
- The host-tail cleanup blocker is fixed. Both original held-prefix/no-newline and trailing-newline probes finish cleanly; the real REPL regression covers both channels and normal, silent and bound command routing.
- Ordinary missing-final-member interpolation is restored, and the stale workflow-provider comment is corrected.
- Two observations remain nonblocking: document-time Context selection differs from stronger literal wording in the boundary packet while publication enforcement captures its owner; unmarked forwarded bytes normalize invalid UTF-8, with per-chunk cost unmeasured. Their retention grants no rule waiver or claim of literal packet compliance.
Architect and Planner also pass this exact head. Focused correction evidence passes (Core 15 groups/83 steps; host 6 groups/11 steps), alongside the retained frozen matrix and its candidate attribution. Required CI remains a separate merge gate.
taras
left a comment
taras
left a comment
There was a problem hiding this comment.
Independent Fable affected closeout: PASS at 624dba0124760cd5c2a2bb1718a16ad4435567b1 (parent 33005881). The completed full review and prior F1 closeout remain retained evidence; this is the seven-file CI repair closeout, covering B1–B6 with no blocking finding.
The repair preserves the existing TypeScript target, removes a validation initialization cycle without changing error identities, omits optional secret metadata on unmarked fields, and serializes errors before the existing selective disclosure projection. The serialization is in-memory and followed by the same redaction; protocol/codec identities and the R1/F1 paths remain intact. Existing D1, X1 and UI13 assertions are unchanged. Recorded focused Node typechecking, 14 separate-process Deno/Bun initialization runs, REPL assertions and protected-input controls pass. Original Architect and Planner also pass this exact head.
Two earlier nonblocking observations remain explicit: consumer-time Context selection differs from the stronger design-packet wording, with no waiver claimed; unmarked forwarded output normalizes invalid UTF-8 and its per-chunk cost is unmeasured. No full review or UX benchmark was repeated for this repair.
Required CI2416 is pending on this corrected commit. This comment records the independent review outcome; it is not a merge approval.
Why
A runtime
<Elicit>credential can enter journals, eval snapshots, command records and displayed output. Short one-time codes are not reliably recognizable by the heuristic scanner. Authors need an explicit field-level guarantee.What changes
Before: an elicited string remains ordinary data and can be retained or displayed.
After: a top-level string property marked
writeOnly: truebecomes a protected value. Normal built-in exec interpolation receives the original string; XMD records, output and Sidekick feedback receive safe views. Unsupported conversions fail actionably before their dependent command runs.Partial continuation obtains fresh missing secret fields while keeping public answers and completed effects. Marked workflow questions use foreground live input; durable answer delivery refuses them before storage. A host without a live responder reports that requirement immediately.
How it works
An execution-owned input service keeps live values separate from field receipts. Eval history restores occurrence references rather than plaintext. Canonical command ownership carries language and protected segments together; safe command descriptions preserve protocol identities. Host filters remain installed through joined producer cleanup.
Review guide
Start with
packages/core/src/secrets/inputs.ts,input-record.tsandcomponents/Elicit.ts. Follow recording/projection insecrets/policy.ts, then host installation and the marked workflow route. Review REPL/browser masking and Sidekick answer projection at their existing owners.Look carefully at the distinction between disclosure payloads and durable identities, partial replay without repeated effects, and output after root Close or Stop. The protocol-collision regression covers the correction found in architecture review.
What must stay true
How to verify it
The frozen M1–M11 matrix covers Core, actual CLI publication modes, REPL cleanup/History, workflow delivery, ACP and web forms. Candidate-to-commit receipts preserve which inputs each command tested. Later witness assertions and the protocol-identity correction have focused passing receipts.
Documentation validation and the web bundle build pass. Architect and Planner pass
624dba01. The independent Fable review is retained, and its affected closeout passes this head. Required CI remains a delivery gate; runner-based corpus weights use the existing measurement workflow.CI run 2414 exposed four integration causes. A seven-file correction preserves the configured TypeScript target, removes a validation import cycle without moving error classes, omits secret metadata on ordinary fields, and serializes errors before selective disclosure projection. Existing D1, X1 and UI13 assertions stay unchanged. Focused Node typechecking and affected Deno, Node and Bun checks pass; complete first failed logs and candidate-to-commit receipts are retained. The corrected head requires a new CI run.
Scope
This delivery covers top-level Elicit string properties and built-in exec's supported forms. It includes REPL/browser presentation, Sidekick feedback, workflow routing and the bootstrap OTP. Unsupported marking shapes and conversions refuse. Ordinary unmarked input, native JSON behavior and the separate TempDir continuation restriction keep their existing contracts.
New abstractions
The execution-owned protected-input service serves elicitation, eval replay, command execution and host output cleanup. Its private field-reference codec preserves replay without storing plaintext. Neither exposes a reveal API or introduces a global secret registry, dependency or record-version layer.
Risks and limitations
The confidentiality boundary covers accidental disclosure through supported XMD behavior. Explicitly executed documents and installed extensions remain trusted. The external process still receives the secret it needs.
Two nonblocking review observations remain recorded: some document-time owner selection uses Context despite stronger literal wording in the design packet, while publication enforcement captures the owner; unmarked forwarded output normalizes invalid UTF-8 and its small per-chunk cost is unmeasured. These observations carry no rule waiver or claim of literal packet compliance.
Scope confirmation
Closes #462.