5 Commits

Author	SHA1	Message	Date
Dai Ha	faefea14c4	fleetd #639: redact()'s comment claimed more than the code does ... 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.	2026-10-01 19:00:29 +02:00
Dai Ha	d7f94cafa2	fleetd #635 follow-up: redact() masks block-scalar continuations + passphrase; --set failures stop echoing the value (defects 7 and 8, criteria 15a/15b/16) ... Defect 7 (comment 17670): redact() only masked a line that itself started with a secret-looking key, so a YAML block scalar's value leaked on the lines that followed the key while the key line right above it printed a reassuring "<redacted>". Fixed by tracking the masked key's own indentation and masking every following line indented deeper than it, stopping once indentation returns to the key's level or shallower; the diff's leading +/-/space marker is stripped before indentation is measured, per the comment's own pitfall. "passphrase" is now also in the key-name backstop. Defect 8 (comment 17673): apply_set_pairs echoed the operator's full "path=value" input, unredacted, in both of its yq-failure die messages — a failing --set with a secret-looking value printed that value right back. Fixed to print only the path; deliberately not routed through redact, which would pass a non-"key: value"-shaped string straight through. Adds acceptance criteria 15a (block-scalar continuation), 15b (passphrase key), and 16 (failing --set never echoes its value) to scripts/test-config-edit.sh, each with a positive control proving the relevant line really was in the printed output before asserting the secret is absent. All three confirmed RED against the pre-fix code and GREEN after, in isolation, before being folded into the full suite (16 criteria + 3 extras, exit 0). Also updates PR #636's description per comment 17671: the redact() sentence now names the continuation-masking rule and says plainly that the key-name list is a backstop, never a complete list.	2026-10-01 18:47:00 +02:00
Dai Ha	096f08c866	fleetd #635 follow-up: fix the stale --restore not-found message (defect 6, criterion 14) ... The --restore "no backup found" message still printed the old beside-the-config glob (${CONFIG}.bak.*) even though newest_backup had already moved to searching the managed .config-backups/ directory. The message was left behind when the search moved — the search itself was already correct (ticket comment 17664). Fix is reporting-only: the message now names the directory actually searched (via backup_dir_for), and separately says that a backup written the old way, directly beside the config, is not searched any more, with the one-line cp to recover one by hand. No search fallback was added — reading backups from outside the managed directory stays unsupported, as instructed. Acceptance criterion 14 proves both directions: the not-found message names the real directory (confirmed red on the pre-fix code, green after), and a restore with a real backup present in .config-backups/ still succeeds (confirmed this catches an "always not-found" regression that direction 1 alone would miss). All 14 criteria plus 3 extras pass in scripts/test-config-edit.sh.	2026-10-01 18:25:08 +02:00
Dai Ha	4eb720029c	fleetd #635 follow-up: refuse empty --set values, gitignore backups, preserve file mode ... Five fixes against PR #636, all verified by the lead's own review and reproduced here: 1. --set .a.b= (a forgotten value) is now refused outright instead of silently nulling the field — a null numeric config value falls back to its default rather than erroring, which widens capacity silently instead of failing loudly. A deliberate clear gets its own spelling, --set .a.b=null, which writes a literal YAML null via yq, never through strenv(). (criteria 9, 10) 2. Backups move from beside fleetd.yaml to a dedicated fleetd/.config-backups/ directory, gitignored at the repo root (so it also covers scripts/test-config-edit.sh's own throwaway fixtures) and in fleetd/.gitignore, plus a fleetd.yaml.bak.* glob backstop for any stray backup written the old way. A backup of a file that must never be committed inherits that requirement. (criterion 11) 3. The live config's file mode now survives both an edit and a restore. mv from a mktemp candidate used to carry mktemp's 0600 onto the live path forever, and cp onto an existing file keeps the destination's mode, so a restore did not undo it either. (criterion 12) 4. A global CAND + single EXIT/INT/TERM trap prevents an uninstalled .config-edit.XXXXXX candidate from leaking if the script is interrupted mid-run. No acceptance criterion is gated on this — a reproducible leak could not be made to happen on demand — but it is cheap and obviously right. 5. Acceptance criterion 7's redaction check gained a positive control: it now asserts the output actually CONTAINS the redaction marker and the changed key, not only that it lacks the secret. The prior two assertions were negative-only and passed just as happily when the diff was never printed at all — confirmed by reproducing the lead's own mutation (deleting the redacted diff print on the edit path) and watching it survive the old test and get caught by the new one. (criterion 13) All 13 acceptance criteria plus 3 extras pass in scripts/test-config-edit.sh. Criteria 9, 10, 11, 12 and 13 were each proven non-vacuous: criteria 9/10 by mutating the test's own expected value and watching it fail by name, then reverting; criteria 11/12/13 by reverting or mutating the corresponding fix in config-edit.sh and watching the matching criterion fail by name, then restoring the fix and re-confirming a clean pass.	2026-10-01 18:07:12 +02:00
Dai Ha	0db6d31dc2	fleetd #635: add scripts/config-edit.sh, the one auditable way to edit fleetd.yaml ... Backs up, builds a candidate off the live file, parse-checks it with yq before install, installs atomically, then reads the daemon's own ConfigRef reload verdict back out of fleetd.out (marked from before the edit, so a stale line can never be mistaken for this edit's result). Four exit codes: 0 clean, 3 needs a restart, 4 refused (backup restored), 5 cannot tell (nothing restored, printed --restore command). Every diff is redacted. scripts/test-config-edit.sh drives it end to end against fixtures in a throwaway temp dir, with no daemon involved.	2026-10-01 17:29:04 +02:00