feat(bin): add guarded settings inspection tool - #2
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 18963c7afa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| stdin=subprocess.DEVNULL, | ||
| stdout=subprocess.PIPE, | ||
| stderr=subprocess.PIPE, | ||
| start_new_session=True, |
There was a problem hiding this comment.
Forward termination signals to the detached child
When the wrapper receives SIGTERM or SIGHUP while the command is running—for example during a worker timeout or teardown—start_new_session=True prevents the child from receiving that signal, and the wrapper has no handler that terminates the new process group. I reproduced this with a child sleep: terminating fm-secrets.sh left the child alive with the selected secret still in its environment. Track the Popen instance and forward termination to its process group before exiting so commands cannot outlive the guarded runner.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in b66a85c. The wrapper now retains the Popen handle, forwards SIGHUP, SIGINT, and SIGTERM to the detached child process group with os.killpg(), waits for the child, scrubs captured output, and returns the conventional 128+signal status. A behavioral regression test sends SIGTERM around a sleeping child and verifies status 143 and that the child group is gone.
Intent
That's two secrets exposed in under 24 hours, Firstmate. This is becoming a problem. How do we make this stop happening? Is this an AGENTS.md addition or what? I can't have this keep happening...
Asked whether to build a settings tool that becomes the only way workers touch settings (list names, check presence, run a command with only named settings injected and every secret value scrubbed from its output): "Yes"
Context: two real leaks on 2026-09-24. (1) TypeSafe API keys were printed while a worker read the running warner-api/warner-scheduler process environment with a redaction predicate that was too narrow. (2) The ATS production database URL (postgres://user:password@host form) was printed while a worker dumped ~/warner-analytics/.env through a sed redaction filter that missed a value line with leading whitespace. Workers and those services all run as the same OS user, so agents can read both .env files and /proc//environ. A written no-secrets rule in the standing worker instructions already existed and did not stop either leak.
What Changed
fm-secrets.shand its Python implementation for listing setting names, checking file or systemd service setting presence, and running commands with named settings injected and output scrubbed.Risk Assessment
✅ Low: The final wildcard-order repair is minimal and the broader settings boundary has no additional source-verifiable defect in the reviewed call paths.
Testing
Live CLI checks demonstrated names/booleans only, selected-environment isolation, URL and short-secret redaction, exit-status preservation, cross-stream redaction, terminal isolation, conservative real-service presence, and emitted worker guidance. Focused behavior coverage passed for the stopped-service fallback; a controlled real stopped-systemd-unit proof was not permitted. This is CLI/Markdown behavior, so rendered UI evidence does not apply.
Evidence: Live fm-secrets CLI transcript
Source: Live fm-secrets CLI transcript
Evidence: Live terminal-isolation and service transcript
Source: Live terminal-isolation and service transcript
Evidence: Generated worker settings rule
Source: Generated worker settings rule
Evidence: Generated scout brief
Source: Generated scout brief
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 1 issue found → auto-fixed ✅
bin/fm-secrets.py:367- WildcardEnvironmentFiles=entries use unsortedglob.glob()results, while later assignments determine the final value. With10.env: TOKEN=keep,20.env: TOKEN=remove, andUnsetEnvironment=TOKEN=remove, a20,10expansion returnsTOKEN=yesalthough systemd's ordered expansion leaves it absent. Sort each wildcard expansion while preserving directive order.🔧 Fix applied.
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
bin/fm-secrets.sh names,has, andrunagainst process-substitution settings fixtures; captured redacted live transcriptbin/fm-secrets.sh runfrom a real pty, attempting/dev/ttyoutputbin/fm-secrets.sh has --service dbus.service PATH DOES_NOT_EXISTFM_HOME=~/.no-mistakes/evidence/01M3CQDG0EGPW464G4FNT1VMJT/fm-brief-live-home bin/fm-brief.sh live-settings-worker alpha --scoutbash tests/fm-secrets.test.shgit diff 5b83e6aa3aec7b019218adcc0cfa5f9adf586e98 b66a85caa693b47cca0ad3e88ee7d406c82c2706 | jev find "a change unrelated to the worker settings inspection, selected-environment command runner, secret output redaction, systemd presence lookup, or generated worker settings rule"✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.