fix: Fix Secret-Leak Guarantee Comment and Assess the Prompt-Path Secret Leak

docs/memory/cli/index.md

@@ -10,3 +10,4 @@ description: "CLI orchestration, report writing, HTML generation, and the host p
 | [host-preflight](host-preflight.md) | hostPreflight.ts — the host environment checks the CLI runs before a session, and the platform-branched command-on-PATH probe behind them: `resolveCommandPath` locates with `where` on win32 (first output line) and `which` elsewhere, through defaulted process-boundary overrides that make both branches test-reachable without stubbing globals. |
 | [report-writer](report-writer.md) | ReportWriter (packages/cli/src/reportWriter.ts) — the run directory it emits (runner.log, input/, tests/<id>/, summary.json, run.json), the secret-redaction contract every write path crosses, device-log copy-then-redact, first-failure precedence, and the emitted-JSON key-omission contract that makes an absent optional field part of the schema |
 | [session-runner](session-runner.md) | sessionRunner.ts — prepareTestSession/executeTestOnSession/runGoal: the per-call ExecutionSessionState every phase records its acquisition into, the single finally that releases whatever is still held, the recording and device-log capture lifecycles, the guarded (never `!`-asserted) install-override preconditions and model-owned platform derivation, and the CLI's two sanctioned dependency seams (TestSessionDeps and testRunnerDependencies) |
+| [test-compiler](test-compiler.md) | compileTestObjective (packages/cli/src/testCompiler.ts) and the secret-value exposure chain it anchors: only `${variables.*}` is substituted, so the compile-time guarantee covers the compiled objective alone — once typed, a resolved secret reaches the provider through the captured hierarchy and screenshots. Prompt text is redacted per input at the AIAgent seam when runtime bindings are wired; screenshots, app-rendered transformations of a value, and INFO planner output are not. |

docs/memory/cli/report-writer.md

@@ -21,6 +21,8 @@ Every value that reaches a file crosses `redactResolvedValue(value, bindings)` (
 
 The environment snapshots carry reference forms only — `env.snapshot.yaml` mirrors the *config* `secrets` map (the `${ENV_VAR}` placeholder form) and `env.json` carries `secretReferences` (key plus env-var name). No resolved secret value is written anywhere beneath the run directory, and `reportWriter.test.ts` holds that as a whole-`runDir` sweep over every emitted file after a full lifecycle rather than as per-site assertions.
 
+The contract is bounded in two ways worth naming, because the sweep's breadth invites reading it as global. It covers **write paths only** — the LLM prompt path is a separate seam, redacted per input inside `AIAgent` when runtime bindings are wired, and stdout INFO output crosses neither ([/cli/test-compiler.md](/cli/test-compiler.md)). And the sweep is a **byte-level** check on emitted files, so `tests/<testId>/screenshots/NNN.jpg` passes it while still showing a typed secret in its pixels: the report carries secret-bearing images even though it carries no secret strings.
+
 ## Device Log Artifact Flow
 
 The `_copyLogArtifact(testId, deviceLog, bindings)` private method handles device logs:

docs/memory/cli/test-compiler.md

@@ -0,0 +1,172 @@
+---
+type: memory
+description: "compileTestObjective (packages/cli/src/testCompiler.ts) and the secret-value exposure chain it anchors: only `${variables.*}` is substituted, so the compile-time guarantee covers the compiled objective alone — once typed, a resolved secret reaches the provider through the captured hierarchy and screenshots. Prompt text is redacted per input at the AIAgent seam when runtime bindings are wired; screenshots, app-rendered transformations of a value, and INFO planner output are not."
+---
+# Test Objective Compilation (cli)
+
+**Domain**: cli
+
+## Overview
+
+`compileTestObjective` (`packages/cli/src/testCompiler.ts`) renders a `TestDefinition` into the
+planner objective: named sections (`Test Name`, `Test Path`, `Description`, `Setup`, `Steps`,
+`Expected State`) with `${variables.*}` references interpolated, plus a trailing `Execution Rules`
+block that teaches the model to treat a `${secrets.*}` placeholder as a logical token and echo it
+verbatim. `VARIABLE_REFERENCE_PATTERN` never matches `${secrets.*}`, which is the repo's one
+compile-time secret guarantee — and that guarantee is a single link in a longer chain. This file
+carries the whole chain: what the compile-time scope covers, how a resolved secret value reaches the
+model provider at runtime anyway, where redaction is wired, and what stays exposed.
+
+## Requirements
+
+### Requirement: The compile-time guarantee covers the compiled objective and nothing else
+`VARIABLE_REFERENCE_PATTERN` MUST match `${variables.*}` only, so `${secrets.*}` tokens survive
+compilation as literal text and no secret VALUE enters the compiled test artifact or the objective
+portion of any prompt through placeholder substitution. Widening the pattern to secrets is the one
+**durable** leak — values would be baked into the compiled artifact and into every planner objective
+— which is why the guard comment on the pattern is load-bearing and MUST NOT be deleted as
+redundant.
+
+The guarantee is exact, not general: an unresolved `${secrets.KEY}` (absent from
+`bindings.secrets`) also stays a literal token, while a `${variables.*}` value that happens to equal
+a secret value still lands in the compiled text, because this pattern cannot tell the two apart.
+
+#### Scenario: a secret placeholder survives compilation
+- **GIVEN** a test whose step reads `Enter ${secrets.TOKEN} on the login screen`
+- **WHEN** `compileTestObjective` renders it
+- **THEN** the compiled objective still contains the literal `${secrets.TOKEN}` token, and the
+  appended `Execution Rules` instruct the model to echo it exactly
+
+### Requirement: A typed secret reaches the model provider through the runtime prompt path
+Once `ActionExecutor._executeType` or `_executeDeeplink` resolves a placeholder through
+`resolveRuntimePlaceholders` (`packages/common/src/repoPlaceholders.ts`) and the value lands on
+screen, the next captured accessibility hierarchy carries it, and both prompt legs can emit it:
+
+- **Grounder leg** — `Hierarchy.toPromptElementsForGrounder` emits the typed field's `text`,
+  `hintText` and `error` verbatim ([/common/hierarchy.md](/common/hierarchy.md)).
+- **Planner leg** — `toPromptElementsForPlanner` carries only `contentDesc`, and only for
+  image/button-class nodes, so it surfaces a value only where one was rendered into accessibility
+  text.
+- **Screenshots** — the planner's pre/post-action screenshots and the grounder's screenshot show the
+  same screen regardless of either element serialization.
+
+Android password fields often mask text at the accessibility layer, but that is app- and
+platform-dependent and MUST NOT be relied on: a plain-text field holding an API key, OTP or token
+carries the value verbatim. The deeplink leg has no exposure of its own — a secret resolved into an
+opened URL re-enters through whatever the resulting screen renders, so it inherits exactly the
+hierarchy and screenshot status above (the span detail logs the unresolved `rawDeeplink`).
+
+### Requirement: Prompt text is redacted per input at the AIAgent seam
+`AIAgent` (`packages/goal-executor/src/ai/AIAgent.ts`) accepts optional `bindings: RuntimeBindings`,
+wired from `createSessionExecutor` (`packages/cli/src/sessionRunner.ts`) as
+`config.runtimeBindings`. When bindings are present, `_buildPlannerPrompt` and
+`_buildGrounderPrompt` MUST replace exact occurrences of resolved secret values with their
+`${secrets.KEY}` placeholder — via `redactResolvedValue` from `@finalrun/common`, whose
+longest-value-first alternation is reused rather than reimplemented — applying it:
+
+- to every free-text input individually (`testObjective`, `act`, `history`, each `remember` entry,
+  `preContext`, `appKnowledge`);
+- to every string field of every element record and `availableApps` record, **before**
+  `JSON.stringify`;
+- to the DEBUG detail blobs (`_detailPlannerRequest`, `_detailGrounderRequest`) and the INFO-level
+  grounder `act` output (`formatGrounderRequest`, `_summarizeGrounderRequest`), which print element
+  and prompt content to a console that no write-path redaction covers.
+
+There MUST be no redaction pass over assembled prompt text. The `Hierarchy` MUST NOT be mutated —
+only serialized copies are rewritten — so index-based grounding (`flattenedHierarchy[idx]` → bounds
+→ tap point) still resolves against the real on-screen values. Absent bindings, every path is a
+strict no-op and prompt assembly is byte-identical.
+
+Seven tests in `packages/goal-executor/src/ai/test/AIAgent.test.ts` pin this, including one for each
+failure mode a pass over assembled text would introduce (see Design Decisions).
+
+#### Scenario: the typed field is redacted and still locatable
+- **GIVEN** bindings `{ secrets: { PASSWORD: 'hunter2-secret' } }` and a captured hierarchy whose
+  typed field has `text: 'hunter2-secret'`
+- **WHEN** `AIAgent` builds the grounder prompt
+- **THEN** the emitted `ui_elements` carries `${secrets.PASSWORD}` and never the raw value, while
+  `index`, `bounds`, `id` and `class` are unchanged and `flattenedHierarchy[idx]` still returns the
+  original text and bounds
+
+#### Scenario: a numeric secret colliding with a coordinate leaves structure intact
+- **GIVEN** `secrets.PIN = '1080'` and an element with `bounds: [0, 0, 1080, 240]`
+- **WHEN** the grounder prompt is built
+- **THEN** the emitted `ui_elements` still parses as JSON, `bounds` is `[0, 0, 1080, 240]`, and only
+  the field whose string value equals the secret becomes `${secrets.PIN}`
+
+### Requirement: Residual exposure is stated, not closed
+Prompts, provider-side logs and report artifacts MUST still be treated as secret-bearing. What
+remains exposed after prompt-text redaction:
+
+- **Screenshots** — sent to the provider unredacted, carried in the `llmCall` observability records,
+  and written to the run report as `tests/<testId>/screenshots/NNN.jpg`. Masking needs visual
+  field-region detection; dropping them would blind the planner.
+- **App-rendered transformations** — a value the app truncates, reformats or partially masks is no
+  longer an exact match and passes through.
+- **Structural strings** — exact matching also rewrites an element's `id`, `class` or `contentDesc`
+  when its content equals a secret value. That degrades the text the grounder reads; it cannot
+  corrupt the emitted JSON, because `index` and `bounds` are non-string fields.
+- **INFO planner reasoning** — `formatPlannerReasoning` prints the planner's `thought`, `action` and
+  `reason` to stdout raw.
+
+No full-prompt logging path exists: the detail blobs carry prompt lengths and three-element
+excerpts, and `runner.log` writes cross `ReportWriter`'s redaction
+([/cli/report-writer.md](/cli/report-writer.md)). Enabling full prompt logging, or treating provider
+logs as secret-free, is exactly the conclusion the guard comment forbids.
+
+## Design Decisions
+
+### The guarantee comment states its scope and its residuals
+**Decision**: The comment on `VARIABLE_REFERENCE_PATTERN` names its own boundary — compile-time
+scope only — then states the runtime exposure with the concrete chain
+(`ActionExecutor._executeType` → captured hierarchy → `Hierarchy.toPromptElements*` → `AIAgent`
+prompt assembly), which redaction seam covers what, and which residuals stay open. It describes the
+shipped state, never an aspirational one.
+**Why**: A confident wrong guarantee is worse than no comment: a reader who believes the invariant
+is enforced end-to-end makes decisions premised on it — enabling prompt logging, treating provider
+logs as secret-free, skipping redaction on a new prompt surface — and each is a silent disclosure.
+Because the comment exists so a future reader does not delete the control, its accuracy standard is
+the highest in the file.
+**Rejected**: A short unqualified claim that secret values never enter LLM prompts — true only of
+the compiled objective, and false in the direction that costs the most.
+*Introduced by*: 260731-gzzl-fix-secret-prompt-leak-comment
+
+### Prompt-path redaction lives at the AIAgent seam, per input, before serialization
+**Decision**: Redaction sits in `AIAgent`, applied to each free-text input and each record string
+field before assembly and `JSON.stringify`, with no pass over the assembled prompt and no mutation
+of the `Hierarchy`.
+**Why**: This is the only seam where both the hierarchy and the runtime bindings are in scope, and
+it covers every hierarchy-carrying prompt regardless of caller — planner, all grounder features,
+post-action hierarchy. Per-input application is what makes redaction structure-blind: it never sees
+serialized JSON, so it cannot rewrite it. Leaving the `Hierarchy` untouched keeps device-side
+grounding provably unaffected, since taps resolve through `flattenedHierarchy[idx]` rather than
+through prompt text.
+**Rejected**: (a) redaction inside `Hierarchy.toPromptElements*` — `packages/common` has no access
+to runtime bindings; (b) a pass over the assembled prompt text — it fails in both directions,
+rewriting structural values that happen to equal a secret (a numeric PIN equal to a bounds
+coordinate corrupts the `ui_elements` JSON) and missing exact secrets that serialization escaped
+(`"` → `\"`, `\` → `\\`); (c) no mitigation at all — the structural half of the risk is
+unit-provable, so documentation alone would under-deliver.
+*Introduced by*: 260731-gzzl-fix-secret-prompt-leak-comment
+
+### A redacted value becomes its placeholder, not an opaque mask
+**Decision**: `redactResolvedValue` substitutes the `${secrets.KEY}` token, the same logical token
+the compiled objective's `Execution Rules` already define.
+**Why**: A verify-flow that reads "the field now contains `${secrets.PASSWORD}`" reads a token whose
+semantics the model was already taught, so the redacted prompt stays reasonable-about rather than
+merely censored.
+**Rejected**: An opaque mask (`***`) — carries no meaning the Execution Rules define, and gives the
+model no way to reason about what the field holds.
+*Introduced by*: 260731-gzzl-fix-secret-prompt-leak-comment
+
+### Screenshots stay unredacted and the residual is documented
+**Decision**: No screenshot masking is attempted, in the prompt path or in the report. The exposure
+is recorded in the source comment and here instead.
+**Why**: Masking requires OCR-grade field-region detection — expensive and unreliable — and blurring
+or omitting the screenshot destroys the planner's primary signal. An honest residual leaves the next
+maintainer able to reason about the actual risk; a partial fix presented as closure recreates the
+original defect at larger scale. `ReportWriter`'s whole-`runDir` sweep is a byte-level check, so it
+proves no secret *string* reaches the report and can say nothing about pixels.
+**Rejected**: (a) OCR/region-based masking — cost and reliability; (b) omitting screenshots from
+prompts or reports — blinds the planner and removes the report's most useful failure evidence.
+*Introduced by*: 260731-gzzl-fix-secret-prompt-leak-comment

docs/memory/common/hierarchy.md

@@ -18,6 +18,13 @@ failure at runtime rather than as a test failure, so the parse contract is pinne
 mutation-verified characterization tests in `packages/common/src/models/test/Hierarchy.test.ts`
 (44 tests, green against the pre-refactor source — see [/ci/pr-quality-gate.md](/ci/pr-quality-gate.md)).
 
+Node text reaches the model provider verbatim from here — a field holding a typed secret included.
+Redaction for that is downstream and generic over string fields: `AIAgent` rewrites exact resolved
+secret values in the records these two builders return, before serializing them, whenever runtime
+bindings are wired ([/cli/test-compiler.md](/cli/test-compiler.md)). It never mutates the
+`Hierarchy`, which keeps holding the real on-screen values — so this file has no bindings access and
+needs none.
+
 ## Requirements
 
 ### Requirement: Two parse paths, deliberately not equivalent

fab/changes/260731-gzzl-fix-secret-prompt-leak-comment/.history.jsonl

@@ -0,0 +1,18 @@
+{"action":"enter","driver":"fab-new","event":"stage-transition","stage":"intake","ts":"2026-07-31T10:34:03Z"}
+{"args":"Fix factually wrong secret-leak guarantee comment in testCompiler.ts and assess/mitigate the real prompt-path secret leak it conceals","cmd":"fab-new","event":"command","ts":"2026-07-31T10:34:03Z"}
+{"delta":"+0.0","event":"confidence","score":4.5,"trigger":"calc-score","ts":"2026-07-31T10:36:18Z"}
+{"cmd":"fab-new","event":"command","ts":"2026-07-31T10:36:49Z"}
+{"delta":"+0.0","event":"confidence","score":4.5,"trigger":"calc-score","ts":"2026-07-31T10:39:48Z"}
+{"cmd":"fab-fff","event":"command","ts":"2026-07-31T10:40:22Z"}
+{"action":"enter","driver":"fab-fff","event":"stage-transition","stage":"apply","ts":"2026-07-31T10:40:34Z"}
+{"cmd":"fab-continue","event":"command","ts":"2026-07-31T10:41:47Z"}
+{"action":"enter","driver":"fab-fff","event":"stage-transition","stage":"review","ts":"2026-07-31T10:55:52Z"}
+{"event":"review","result":"failed","ts":"2026-07-31T11:07:17Z"}
+{"action":"re-entry","driver":"fab-fff","event":"stage-transition","stage":"apply","ts":"2026-07-31T11:07:17Z"}
+{"action":"re-entry","driver":"fab-fff","event":"stage-transition","stage":"review","ts":"2026-07-31T11:14:29Z"}
+{"action":"enter","driver":"fab-fff","event":"stage-transition","stage":"hydrate","ts":"2026-07-31T11:23:33Z"}
+{"event":"review","result":"passed","ts":"2026-07-31T11:23:33Z"}
+{"action":"enter","driver":"fab-fff","event":"stage-transition","stage":"ship","ts":"2026-07-31T11:29:29Z"}
+{"action":"enter","driver":"git-pr","event":"stage-transition","stage":"review-pr","ts":"2026-07-31T11:32:11Z"}
+{"cmd":"git-pr-review","event":"command","ts":"2026-07-31T11:33:52Z"}
+{"event":"review","result":"passed","ts":"2026-07-31T11:49:39Z"}

fab/changes/260731-gzzl-fix-secret-prompt-leak-comment/.status.yaml

@@ -0,0 +1,52 @@
+id: gzzl
+name: 260731-gzzl-fix-secret-prompt-leak-comment
+created: 2026-07-31T10:34:03Z
+created_by: droid-ash
+change_type: fix
+issues: []
+progress:
+    intake: done
+    apply: done
+    review: done
+    hydrate: done
+    ship: done
+    review-pr: done
+plan:
+    generated: true
+    task_count: 6
+    acceptance_count: 13
+    acceptance_completed: 13
+confidence:
+    certain: 4
+    confident: 3
+    tentative: 0
+    unresolved: 0
+    score: 4.5
+    fuzzy: true
+    dimensions:
+        signal: 83.6
+        reversibility: 80.7
+        competence: 85.0
+        disambiguation: 80.7
+stage_metrics:
+    intake: {started_at: "2026-07-31T10:34:03Z", driver: fab-new, iterations: 1, completed_at: "2026-07-31T10:40:34Z"}
+    apply: {started_at: "2026-07-31T11:07:17Z", driver: fab-fff, iterations: 2, completed_at: "2026-07-31T11:14:29Z"}
+    review: {started_at: "2026-07-31T11:14:29Z", driver: fab-fff, iterations: 2, completed_at: "2026-07-31T11:23:33Z"}
+    hydrate: {started_at: "2026-07-31T11:23:33Z", driver: fab-fff, iterations: 1, completed_at: "2026-07-31T11:29:29Z"}
+    ship: {started_at: "2026-07-31T11:29:29Z", driver: fab-fff, iterations: 1, completed_at: "2026-07-31T11:32:11Z"}
+    review-pr: {started_at: "2026-07-31T11:32:11Z", driver: git-pr, iterations: 1, completed_at: "2026-07-31T11:49:39Z"}
+prs:
+    - https://github.com/droid-ash/finalrun-agent/pull/175
+true_impact:
+    added: 889
+    deleted: 35
+    net: 854
+    tests:
+        added: 215
+        deleted: 1
+        net: 214
+    computed_at: "2026-07-31T11:32:11Z"
+    computed_at_stage: ship
+summary: 'Documented the secret-value exposure chain: new cli/test-compiler (compile-time scope, runtime prompt path, AIAgent redaction seam, residuals), scope carve-outs on cli/report-writer, downstream-redaction pointer on common/hierarchy'
+# true_impact: lazily created on first stage-finish that computes it (no placeholder here).
+last_updated: 2026-07-31T11:49:39Z