fleet/fleetd
3
0
Fork 0
Code Issues 75 Pull Requests Actions Packages Projects Releases 1 Wiki Activity

config-edit.sh: redact()'s continuation masking loses its anchor, so a block scalar's body still leaks #639

New Issue
Open
opened 2026-10-01 18:53:00 +02:00 by ltms · 0 comments
ltms commented 2026-10-01 18:53:00 +02:00
Owner
Copy Link
Copy Source

Found by the lead while verifying PR #636 at d7f94ca, before merging it.

This is not a regression and it does not block #636. The pre-fix code leaked these same cases
and more, so d7f94ca is a strict improvement. It is filed because d7f94ca also added a claim
that my measurements contradict, and a future session will read that claim and trust it.

The claim that is too strong

scripts/config-edit.sh, in the comment added above redact():

This needs no knowledge of the key's name and so protects a block scalar under any masked key,
present or future.

And PR #636's body:

the indentation-based continuation masking is what holds regardless of the key's name.

The mechanism masks a continuation line only while it is still anchored to a masked key line it
has already seen. I found two ways the anchor is lost, and then nothing is masked at all.

Defect A — the key line falls outside the diff hunk (the general case, the worse one)

redact is fed diff -u output. diff -u prints the changed line plus three lines of
context. A block scalar's body is routinely inside that window while its own key line is not.
With no key line, masked is never set, and the whole body prints raw.

Measured through the real script, --dry-run, no mutation, at d7f94ca:

broker:
  token: |
    SECRET-LINE-1
    SECRET-LINE-2
    SECRET-LINE-3
    SECRET-LINE-4
  nearby: changeme

scripts/config-edit.sh --config fx2.yaml --dry-run --set '.broker.nearby=newvalue' printed:

@@ -4,4 +4,4 @@
     SECRET-LINE-2
     SECRET-LINE-3
     SECRET-LINE-4
-  nearby: changeme
+  nearby: newvalue

SECRET-LINE-2, -3 and -4 leaked in full. No <redacted> appears anywhere in the output.

SECRET-LINE-1 is absent rather than masked — it fell outside the hunk entirely. Absence is
not masking, and a test that only greps for absence would read this as a pass.

Defect B — a blank line inside a block scalar ends the masking

A block scalar may contain blank lines. A blank context line in a unified diff is a single space,
so content is empty and indent is 0. indent > masked_indent is then false, masked resets,
and everything after the blank line prints raw — directly under a <redacted> marker.

Measured on redact() extracted from d7f94ca, fed this input:

broker:
  token: |
    LEAK-BEFORE-BLANK

    LEAK-AFTER-BLANK
  nearby: x

Output:

broker:
  token: <redacted>
    <redacted>

    LEAK-AFTER-BLANK      <- leaked
  nearby: x

This is the exact shape #635 calls worse than no redaction: the marker sits above the leak and
tells the reader to stop looking.

How I know the instrument was not lying

Both probes were run with controls, because an action probe that never acted reads as a clean
negative:

  • Masking works — the same harness masked CTRL-MUST-BE-MASKED in a block scalar whose key
    line was present, so a "masked" result is real.
  • Passthrough works — the harness printed CTRL-MUST-APPEAR, so it is not simply swallowing
    input.
  • The leak region reached the output — for defect A I first ran a plain diff -u and confirmed
    SECRET-LINE-* was inside the three-line window, before asserting anything about redaction.
    Without that step, "the secret never entered the diff" and "it entered and was redacted" look
    identical.
  • The pattern I used to survey the live config was checked against a planted file first (it found
    1 of 1).

Reachability — latent, not live

I checked the live fleetd/fleetd.yaml. It holds 5 block scalars, at lines 304, 330, 340, 349
and 632: the four role charters (architect, dev, reviewer, hunter) and bootstrapText.
None of those keys matches the secret-key list, so none is masked today and none is a secret.
Secrets in this config are referenced by env-var name (broker.uriEnv), not written inline.

So no secret leaks from today's config. The defect becomes live the day someone writes a
multi-line secret inline under a masked key.

What a fix has to face

redact cannot anchor on a key it was never shown. Masking computed from the diff alone cannot be
made sound. Two directions, neither free:

  1. Feed diff a large -U so a block scalar's key is always in the window. Bounded, but -U
    has no value that is always enough, and it makes every diff longer.
  2. Compute which file lines are inside a masked key's block from the candidate and backup
    themselves, then mask by line number rather than by what the hunk happens to show. Sound, and
    a bigger change than #635's shape.

Until one of those lands, the honest move is wording: say the continuation masking holds while
the masked key line is itself in the printed hunk, and stop claiming it holds in general.

Asks

  • Correct the two overstated sentences (the redact() comment and #636's body) to name the
    anchor condition.
  • Decide between fix direction 1 and 2.
  • An acceptance criterion for defect A needs a positive control proving the body reached the
    output, exactly as criterion 15a already does for the in-window case.

Related: #635, PR #636, and #638 ($VERDICT_LINE printed raw).

Found by the lead while verifying PR #636 at `d7f94ca`, before merging it. **This is not a regression and it does not block #636.** The pre-fix code leaked these same cases and more, so `d7f94ca` is a strict improvement. It is filed because `d7f94ca` also added a claim that my measurements contradict, and a future session will read that claim and trust it. ## The claim that is too strong `scripts/config-edit.sh`, in the comment added above `redact()`: > This needs no knowledge of the key's name and so protects a block scalar under any masked key, > present or future. And PR #636's body: > the indentation-based continuation masking is what holds regardless of the key's name. The mechanism masks a continuation line only while it is still *anchored* to a masked key line it has already seen. I found two ways the anchor is lost, and then nothing is masked at all. ## Defect A — the key line falls outside the diff hunk (the general case, the worse one) `redact` is fed `diff -u` output. `diff -u` prints the changed line plus **three** lines of context. A block scalar's body is routinely inside that window while its own key line is not. With no key line, `masked` is never set, and the whole body prints raw. Measured through the real script, `--dry-run`, no mutation, at `d7f94ca`: ```yaml broker: token: | SECRET-LINE-1 SECRET-LINE-2 SECRET-LINE-3 SECRET-LINE-4 nearby: changeme ``` `scripts/config-edit.sh --config fx2.yaml --dry-run --set '.broker.nearby=newvalue'` printed: ``` @@ -4,4 +4,4 @@ SECRET-LINE-2 SECRET-LINE-3 SECRET-LINE-4 - nearby: changeme + nearby: newvalue ``` `SECRET-LINE-2`, `-3` and `-4` leaked in full. No `<redacted>` appears anywhere in the output. `SECRET-LINE-1` is **absent rather than masked** — it fell outside the hunk entirely. Absence is not masking, and a test that only greps for absence would read this as a pass. ## Defect B — a blank line inside a block scalar ends the masking A block scalar may contain blank lines. A blank context line in a unified diff is a single space, so `content` is empty and `indent` is 0. `indent > masked_indent` is then false, `masked` resets, and everything after the blank line prints raw — directly under a `<redacted>` marker. Measured on `redact()` extracted from `d7f94ca`, fed this input: ```yaml broker: token: | LEAK-BEFORE-BLANK LEAK-AFTER-BLANK nearby: x ``` Output: ``` broker: token: <redacted> <redacted> LEAK-AFTER-BLANK <- leaked nearby: x ``` This is the exact shape #635 calls worse than no redaction: the marker sits above the leak and tells the reader to stop looking. ## How I know the instrument was not lying Both probes were run with controls, because an action probe that never acted reads as a clean negative: - **Masking works** — the same harness masked `CTRL-MUST-BE-MASKED` in a block scalar whose key line *was* present, so a "masked" result is real. - **Passthrough works** — the harness printed `CTRL-MUST-APPEAR`, so it is not simply swallowing input. - **The leak region reached the output** — for defect A I first ran a plain `diff -u` and confirmed `SECRET-LINE-*` was inside the three-line window, *before* asserting anything about redaction. Without that step, "the secret never entered the diff" and "it entered and was redacted" look identical. - The pattern I used to survey the live config was checked against a planted file first (it found 1 of 1). ## Reachability — latent, not live I checked the live `fleetd/fleetd.yaml`. It holds **5** block scalars, at lines 304, 330, 340, 349 and 632: the four role charters (`architect`, `dev`, `reviewer`, `hunter`) and `bootstrapText`. None of those keys matches the secret-key list, so none is masked today and none is a secret. Secrets in this config are referenced by env-var name (`broker.uriEnv`), not written inline. So **no secret leaks from today's config.** The defect becomes live the day someone writes a multi-line secret inline under a masked key. ## What a fix has to face `redact` cannot anchor on a key it was never shown. Masking computed from the diff alone cannot be made sound. Two directions, neither free: 1. Feed `diff` a large `-U` so a block scalar's key is always in the window. Bounded, but `-U` has no value that is always enough, and it makes every diff longer. 2. Compute which *file* lines are inside a masked key's block from the candidate and backup themselves, then mask by line number rather than by what the hunk happens to show. Sound, and a bigger change than #635's shape. Until one of those lands, the honest move is wording: say the continuation masking holds **while the masked key line is itself in the printed hunk**, and stop claiming it holds in general. ## Asks - [ ] Correct the two overstated sentences (the `redact()` comment and #636's body) to name the anchor condition. - [ ] Decide between fix direction 1 and 2. - [ ] An acceptance criterion for defect A needs a positive control proving the body reached the output, exactly as criterion 15a already does for the in-window case. Related: #635, PR #636, and #638 (`$VERDICT_LINE` printed raw).
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
blocked
Cannot start until something else lands. The body says what.
 needs-live-proof
Merged and green, but never shown working on the running daemon. Not the same as done.
 ready-to-delegate
Scope, files and acceptance criteria are written. A worker can be briefed from the body alone.
 silent-default
A feature that compiles, passes tests, and ships turned off. Nine recurrences and counting.
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
agent kevin ltms
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: fleet/fleetd#639
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?