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

config-edit.sh prints the daemon's verdict line raw — a refusal may quote the value that caused it #638

New Issue
Open
opened 2026-10-01 18:37:01 +02:00 by ltms · 1 comment
ltms commented 2026-10-01 18:37:01 +02:00
Owner
Copy Link
Copy Source

What

scripts/config-edit.sh prints $VERDICT_LINE without redaction in six places: lines 441, 442, 465, 469, 473 and 515 (measured on 096f08c, PR #636).

That string is the daemon's own verdict, read back out of fleetd.out. On the refusal path it reads:

config reload refused — <error message>

If the loader's error message quotes the offending value, a refused edit prints that value to the terminal. Every other path in the script that prints config-derived content goes through redact; this one does not.

Status — not verified, and that is the point of the ticket

I have not measured whether a real refusal quotes the value. It needs a live daemon fed a deliberately bad config, which is a lead-side action, and I am filing this rather than doing it inline so the measurement and the decision are recorded together.

What is measured: the six call sites print the variable with no filter. What is not: whether ConfigRef's refusal text ever contains a config value as opposed to only a key name and a type error.

The plausible case is a type coercion failure. config-edit.sh --set writes every value as a quoted string, so a bad value reaches Jackson as a string on a typed field, and Jackson's InvalidFormatException text conventionally includes the offending input. That is a guess about a library's message format, not a measurement, and it may well be that ConfigRef wraps it and keeps only the key. Do not treat the mechanism above as established.

Why it is not simply "pipe it through redact"

Two things pull against each other, which is why this is a ticket and not a one-line fix.

  • The refusal line is the only thing that explains why an edit was rejected. Masking it can remove exactly the detail the operator needs, turning a useful error into "something was wrong". A redactor that hides the cause of a failure has made the failure harder to fix.
  • redact masks key: value shapes and rewrites URL userinfo. A verdict line is prose, not key: value, so the key-name half of redact would pass it through untouched and report nothing. Piping it through redact unchanged would therefore give the appearance of coverage while changing almost nothing — the same false-coverage shape as defect 7 on #635 (comment 17670).

So the real question is what a useful redacted refusal looks like, not whether to call the existing function.

Suggested direction, to be decided with the measurement

  1. First measure. Feed a live daemon a config with a bad value on a typed field and capture the exact refusal line. Everything below depends on what that line actually contains.
  2. If it does quote values, apply the userinfo rewrite only (sed -E 's#://[^@]*@#://<redacted>@#g', the g is required) to the verdict line. That catches the highest-value case, a URI with embedded credentials, and leaves the diagnostic prose intact.
  3. Consider naming the key without echoing the value, if the line's shape allows it to be split reliably. If it does not, say so and keep the whole line — a guess at parsing the daemon's prose is worse than printing it.
  4. If the measurement shows the line never carries a value, close this with that evidence recorded. A negative result here is a useful outcome, not a wasted ticket.

Acceptance criteria

  1. The captured real refusal line, from a live daemon, pasted into the ticket. This comes first; the rest depends on it.
  2. If a change is made, a test proving a verdict line carrying scheme://user:pass@host has its userinfo masked, with a positive control proving the rest of the line still reaches the output. Without that control the test passes if the line vanishes.
  3. If no change is made, the evidence that none is needed, and the six call sites annotated so the next reader does not re-open this.

Found how

Found during the lead review of PR #636 while surveying every path in config-edit.sh that prints content it did not redact. The survey found two unredacted paths: the --set failure messages echoing the operator's value (fixed in #635 as defect 8, acceptance criterion 16, measured and reproducible) and this one. This one was split out because, unlike the other, it cannot be verified without a live daemon and it carries a design question about how much of a refusal to hide.

## What `scripts/config-edit.sh` prints `$VERDICT_LINE` without redaction in six places: lines 441, 442, 465, 469, 473 and 515 (measured on `096f08c`, PR #636). That string is the daemon's own verdict, read back out of `fleetd.out`. On the refusal path it reads: ``` config reload refused — <error message> ``` If the loader's error message quotes the offending value, a refused edit prints that value to the terminal. Every other path in the script that prints config-derived content goes through `redact`; this one does not. ## Status — not verified, and that is the point of the ticket **I have not measured whether a real refusal quotes the value.** It needs a live daemon fed a deliberately bad config, which is a lead-side action, and I am filing this rather than doing it inline so the measurement and the decision are recorded together. What is measured: the six call sites print the variable with no filter. What is not: whether `ConfigRef`'s refusal text ever contains a config *value* as opposed to only a key name and a type error. The plausible case is a type coercion failure. `config-edit.sh --set` writes every value as a quoted string, so a bad value reaches Jackson as a string on a typed field, and Jackson's `InvalidFormatException` text conventionally includes the offending input. That is a guess about a library's message format, not a measurement, and it may well be that `ConfigRef` wraps it and keeps only the key. **Do not treat the mechanism above as established.** ## Why it is not simply "pipe it through redact" Two things pull against each other, which is why this is a ticket and not a one-line fix. - The refusal line is the only thing that explains *why* an edit was rejected. Masking it can remove exactly the detail the operator needs, turning a useful error into "something was wrong". A redactor that hides the cause of a failure has made the failure harder to fix. - `redact` masks `key: value` shapes and rewrites URL userinfo. A verdict line is prose, not `key: value`, so the key-name half of `redact` would pass it through untouched and report nothing. Piping it through `redact` unchanged would therefore give the appearance of coverage while changing almost nothing — the same false-coverage shape as defect 7 on #635 (comment 17670). So the real question is what a *useful redacted refusal* looks like, not whether to call the existing function. ## Suggested direction, to be decided with the measurement 1. First measure. Feed a live daemon a config with a bad value on a typed field and capture the exact refusal line. Everything below depends on what that line actually contains. 2. If it does quote values, apply the **userinfo rewrite only** (`sed -E 's#://[^@]*@#://<redacted>@#g'`, the `g` is required) to the verdict line. That catches the highest-value case, a URI with embedded credentials, and leaves the diagnostic prose intact. 3. Consider naming the key without echoing the value, if the line's shape allows it to be split reliably. If it does not, say so and keep the whole line — a guess at parsing the daemon's prose is worse than printing it. 4. If the measurement shows the line never carries a value, close this with that evidence recorded. A negative result here is a useful outcome, not a wasted ticket. ## Acceptance criteria 1. The captured real refusal line, from a live daemon, pasted into the ticket. This comes first; the rest depends on it. 2. If a change is made, a test proving a verdict line carrying `scheme://user:pass@host` has its userinfo masked, with a positive control proving the rest of the line still reaches the output. Without that control the test passes if the line vanishes. 3. If no change is made, the evidence that none is needed, and the six call sites annotated so the next reader does not re-open this. ## Found how Found during the lead review of PR #636 while surveying every path in `config-edit.sh` that prints content it did not redact. The survey found two unredacted paths: the `--set` failure messages echoing the operator's value (fixed in #635 as defect 8, acceptance criterion 16, measured and reproducible) and this one. This one was split out because, unlike the other, it cannot be verified without a live daemon and it carries a design question about how much of a refusal to hide.
ltms commented 2026-10-01 19:14:54 +02:00
Author
Owner
Copy Link
Copy Source

Related, lower-severity instance of the same family — checked, and my verdict is no fix needed.

Recording this so it is not rediscovered as a new finding.

The #635 worker ran a report-only audit of every path in config-edit.sh that prints
config-derived content, and reported one beyond the two that matter:

resolve_port() extracts .bind.port via yq and echoes it directly, NOT through redact — low
risk in practice since it's a single fixed numeric field, not a generic content print, but it is
technically a config-derived value printed unredacted. Noted, not fixed.

I checked it on main at 141ae3b rather than taking the report. The claim is accurate.
resolve_port (scripts/config-edit.sh:217-228) reads .bind.port with yq and echoes it; the
value is captured at :554 and :599 and then printed to the terminal at four places —
:556, :558, :601, :603 — none of them through redact.

No fix, for three reasons:

  1. .bind.port is not a secret, and redact would not mask it even if it were routed through:
    the key name port matches none of
    TOKEN|SECRET|PASSWORD|PASSWD|PASSPHRASE|CREDENTIAL|URI|_KEY. Piping it through would be the
    false-coverage mistake that #636 already names twice.
  2. It is one scalar from a single fixed key, not a generic content print. The two generic paths
    (run_edit's change report and dry_run_diff) both do use redact — confirmed independently
    by the worker, the previous lead, and me.
  3. Printing the port is the point of the line. ok daemon is listening on 127.0.0.1:8765 masked
    would tell the operator nothing.

The worker's own hedge was correct, and it was right to report rather than fix it.

Why this belongs on #638 and not its own ticket: #638 is about $VERDICT_LINE reaching the
output unredacted, which is the same shape — a value printed without passing the redactor. The
difference is that #638's value is daemon-derived refusal text whose content is unknown in advance,
so masking it is a real question with a real cost (masking a refusal can hide the one thing that
explains it). .bind.port has no such question. Same family, different answer.

Unrelated to #639, which is about redact() failing on content that does reach it.

**Related, lower-severity instance of the same family — checked, and my verdict is no fix needed.** Recording this so it is not rediscovered as a new finding. The #635 worker ran a report-only audit of every path in `config-edit.sh` that prints config-derived content, and reported one beyond the two that matter: > `resolve_port()` extracts `.bind.port` via yq and echoes it directly, NOT through redact — low > risk in practice since it's a single fixed numeric field, not a generic content print, but it is > technically a config-derived value printed unredacted. Noted, not fixed. **I checked it on `main` at `141ae3b` rather than taking the report.** The claim is accurate. `resolve_port` (`scripts/config-edit.sh:217-228`) reads `.bind.port` with `yq` and echoes it; the value is captured at `:554` and `:599` and then printed to the terminal at **four** places — `:556`, `:558`, `:601`, `:603` — none of them through `redact`. **No fix, for three reasons:** 1. `.bind.port` is not a secret, and `redact` would not mask it even if it were routed through: the key name `port` matches none of `TOKEN|SECRET|PASSWORD|PASSWD|PASSPHRASE|CREDENTIAL|URI|_KEY`. Piping it through would be the false-coverage mistake that #636 already names twice. 2. It is one scalar from a single fixed key, not a generic content print. The two generic paths (`run_edit`'s change report and `dry_run_diff`) both do use `redact` — confirmed independently by the worker, the previous lead, and me. 3. Printing the port is the point of the line. `ok daemon is listening on 127.0.0.1:8765` masked would tell the operator nothing. The worker's own hedge was correct, and it was right to report rather than fix it. **Why this belongs on #638 and not its own ticket:** #638 is about `$VERDICT_LINE` reaching the output unredacted, which is the same shape — a value printed without passing the redactor. The difference is that #638's value is daemon-derived refusal text whose content is unknown in advance, so masking it is a real question with a real cost (masking a refusal can hide the one thing that explains it). `.bind.port` has no such question. Same family, different answer. Unrelated to #639, which is about `redact()` failing on content that *does* reach it.
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#638
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?