fleetd #639: redact()'s comment claimed more than the code does #640

Merged
ltms merged 1 commits from lead/config-edit-redact-anchor-wording into main 2026-10-01 19:00:57 +02:00
Owner

Comment text only. No behaviour change, no test change.

Follow-up to PR #636, which I merged at ea6896f. That PR's d7f94ca fixed defect 7 (a block
scalar's value leaking past a masked key line) by masking continuation lines on indentation. The
fix is real and is a strict improvement. The paragraph documenting it, however, ended with:

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

That claims more than the code delivers, and I measured two ways it fails.

The anchor can be missing. redact() is fed diff -u output, which prints three lines of
context. A block scalar's body often arrives with its key line left out of the hunk. With no key
line, masked is never set and the body prints in full — and no <redacted> appears anywhere, so
nothing signals that redaction was even attempted.

The anchor can be dropped mid-block. A blank line inside a block scalar measures as indent 0,
so indent > masked_indent is false, masked resets, and the rest of the value prints raw. This
one prints directly under a <redacted> marker.

Both reproduced through the real script with --dry-run, each with a positive control run
first
proving the secret's lines actually reached the output. Without that control, "the secret
never entered the diff" and "it entered and was correctly redacted" are indistinguishable — which
is exactly how the previous lead's first attempt at this reproduction came back clean.

Full evidence, the reachability check, and the two candidate fix directions are in #639. In
short: latent, not live — today's fleetd.yaml holds 5 block scalars and all 5 sit under
non-secret keys, and secrets in this config are referenced by env-var name rather than written
inline.

Why this is a commit and not just a ticket

#635 exists because an incomplete redactor that looks complete is worse than one that visibly
does nothing — the marker stops a reviewer from looking further. A comment that overstates the
guarantee is that same defect in prose. A future session reading redact() has no other source,
and this paragraph is the thing it would trust.

Test plan

  • scripts/test-config-edit.sh in a clean copy of the edited tree: 16 criteria + 3 extras, exit 0.
  • bash -n scripts/config-edit.sh clean.
  • Control on the edit itself: the old claim is gone (grep -c 'present or future' → 0) and the new
    text is present (grep -c 'fleetd #639' → 1).
  • No .java, pom.xml or .yaml touched, so no Maven gate.

PR #636's body carries the same overstated sentence. I cannot edit it as cleanly from here, so
#639 records it as an open ask instead.

Comment text only. No behaviour change, no test change. Follow-up to PR #636, which I merged at `ea6896f`. That PR's `d7f94ca` fixed defect 7 (a block scalar's value leaking past a masked key line) by masking continuation lines on indentation. The fix is real and is a strict improvement. The paragraph documenting it, however, ended with: > This needs no knowledge of the key's name and so protects a block scalar under any masked key, > present or future. That claims more than the code delivers, and I measured two ways it fails. **The anchor can be missing.** `redact()` is fed `diff -u` output, which prints three lines of context. A block scalar's body often arrives with its key line left out of the hunk. With no key line, `masked` is never set and the body prints in full — and no `<redacted>` appears anywhere, so nothing signals that redaction was even attempted. **The anchor can be dropped mid-block.** A blank line inside a block scalar measures as indent 0, so `indent > masked_indent` is false, `masked` resets, and the rest of the value prints raw. This one prints directly under a `<redacted>` marker. Both reproduced through the real script with `--dry-run`, each with a **positive control run first** proving the secret's lines actually reached the output. Without that control, "the secret never entered the diff" and "it entered and was correctly redacted" are indistinguishable — which is exactly how the previous lead's first attempt at this reproduction came back clean. Full evidence, the reachability check, and the two candidate fix directions are in **#639**. In short: latent, not live — today's `fleetd.yaml` holds 5 block scalars and all 5 sit under non-secret keys, and secrets in this config are referenced by env-var name rather than written inline. ### Why this is a commit and not just a ticket #635 exists because an incomplete redactor that *looks* complete is worse than one that visibly does nothing — the marker stops a reviewer from looking further. A comment that overstates the guarantee is that same defect in prose. A future session reading `redact()` has no other source, and this paragraph is the thing it would trust. ### Test plan - `scripts/test-config-edit.sh` in a clean copy of the edited tree: 16 criteria + 3 extras, exit 0. - `bash -n scripts/config-edit.sh` clean. - Control on the edit itself: the old claim is gone (`grep -c 'present or future'` → 0) and the new text is present (`grep -c 'fleetd #639'` → 1). - No `.java`, `pom.xml` or `.yaml` touched, so no Maven gate. PR #636's body carries the same overstated sentence. I cannot edit it as cleanly from here, so #639 records it as an open ask instead.
ltms added 1 commit 2026-10-01 19:00:46 +02:00
fleetd #639: redact()'s comment claimed more than the code does
CI / shell-tests (pull_request) Failing after 6s
CI / contract (pull_request) Successful in 1m8s
CI / build (pull_request) Failing after 2m11s
faefea14c4
The paragraph added in d7f94ca ended with "This needs no knowledge of the key's
name and so protects a block scalar under any masked key, present or future."
The continuation masking is real and it is an improvement, but that sentence is
too strong: the masking only holds while the masked key's own line is inside the
hunk being printed.

redact() is fed `diff -u` output, which prints three lines of context. A block
scalar's body therefore often arrives with its key line left out. With no key
line, `masked` is never set and the body prints in full, with no "<redacted>"
anywhere. A blank line inside a block scalar loses the anchor the same way: a
blank diff line measures as indent 0, so `indent > masked_indent` is false and
the mask ends early — this time directly under a "<redacted>" marker.

Both were reproduced through the real script with --dry-run, each with a
positive control run first to prove the secret's lines actually reached the
output (without that control, "the secret never entered the diff" and "it
entered and was redacted" are indistinguishable). Filed as fleetd #639, which
also records that this is latent rather than live: today's fleetd.yaml holds 5
block scalars and all 5 sit under non-secret keys.

Comment text only. No change to redact() or to any other function, and no
change to the test suite. scripts/test-config-edit.sh still passes in a clean
copy (16 criteria + 3 extras, exit 0); bash -n clean.

The reason this is worth its own commit: #635 exists because an incomplete
redactor that looks complete is worse than one that visibly does nothing. A
comment that overstates the guarantee is the same defect in prose, and the next
session to read it has no other source.
ltms merged commit 141ae3b04d into main 2026-10-01 19:00:57 +02:00
Sign in to join this conversation.