Repository navigation
Key runner: id-based injector that masks the keys it injects (credential_run.py), public_variables inventory field, docs as available - #497
Merged
Conversation
seathatflowsinourveins
added
the
lane:foundation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
seathatflowsinourveins
force-pushed
the
claude/key-injector-20260929
branch
from
September 29, 2026 07:49
3275b47 to
5a01baa
Compare
…d variable The key runner of the next commit (tools/credentials/credential_run.py) masks every variable it injects except the names an entry lists in the new optional field public_variables (D1 PR-4 contract, amendment 4). scripts/credential_status.py now accepts the field, requires it to name distinct optional_variables of its own entry (a required variable is never public), and requires an env-file entry that declares optional variables to classify them explicitly ([] masks them all). Rows without optional variables carry no field. Classification: APCA_API_BASE_URL is public on alpaca-paper and alpaca-paper-2 (a base URL, not a secret). EDGAR_IDENTITY (sec-contact, private contact data) and GF_SECURITY_ADMIN_USER (grafana-admin, engine-held and never injected) stay masked. Failed first: test_public_variables_classify_every_optional_stored_variable errored with KeyError 'public_variables' before the inventory change, and the schema ignored unknown keys, so none of its five planted mutations (a string, a required name, another entry's name, a duplicate, a missing classification) produced an inventory error. Checked with tests.test_credential_status and tests.test_credential_tools: all pass. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… (D1 PR-4, phase 1) python3 tools/credentials/credential_run.py <inventory-id> [--only NAME]... -- <command> [args...] python3 tools/credentials/credential_run.py <inventory-id> --check Stdlib only, Python 3.9 grammar. It re-executes under -I -S before anything is read. It resolves the entry through credential_status (inventory_errors must be empty; STORE_ROOT plus expand_template), refuses engine-held, native, interactive and CI ids with a hint naming the holder, and reads the 0600 file through a no-follow handle on the 0700 store directory outside every Git worktree (no symlink, FIFO or hard link, at most 64 KiB). The parser is measure.py's load_env_file grammar held to set_credential.py's value grammar (BARE_VALUE unquoted, encode() quoted), so a hand-edited $, backtick or backslash is refused, never expanded. Masked values must be at least 6 bytes (Buildkite LengthMin). The command gets the caller's environment minus every inventory and must_not_be_set name, plus this id's declared variables (narrowed by --only). It runs in a new session with RLIMIT_CORE 0 and inherited stdin, and SIGINT, SIGTERM and SIGHUP are forwarded to its process group. Its exit code passes through (128+N for signal N). Both pipes are relayed as bytes through one masker each. Needles are the raw value, base64 and base64url stable interiors at alignments 0, 1 and 2, percent forms (quote and quote_plus, upper- and lower-case hex), the JSON escape and hex; they are deduplicated and matched longest first, with overlaps merged. A suffix that may begin a needle is held and never cut through a complete match. A held tail of 3 bytes or less is written after 100 ms idle or at the end; 4 bytes or more is never written and becomes [REDACTED-PARTIAL:<NAME>] at EOF, on kill or after the 2 s drain that stops a descendant holding a pipe from hanging the runner. Messages carry ids, names, line numbers and reason codes only; a catch-all prints the exception type and internal_error. Sources are in the module docstring, each re-read at a pinned commit on 2026-09-29: dotenvx@278101db redactOutput.js, actions/runner@15231bed ValueEncoders.cs and SecretMasker.cs, buildkite/agent@3345ee60 redact.go and replacer.go, dmno-dev/varlock@1b880652 redact-stream.ts, and Generalized-Labs/ironrun@b611c7ce encodings.go (v0.4.0). Failed first: tests/test_credential_run.py was written before the tool and failed to import it (exit 1). The first run of the tool then failed 7 of 30 tests. Four waited for the previous commit's public_variables. Two caught a too-broad test check. One, test_flushes_an_unterminated_tail_at_eof, showed that printing no held tail at EOF turns the last "d" of any output into a marker, because base64 of every Tavily key ("tvly-") starts with "d". Hence the 3-byte rule at EOF, which is a deviation from amendment 4's literal text: a held tail is replaced at EOF only from 4 bytes, the idle timer's own threshold. SHORT_TAIL_MAX flips it back, and the mutation below pins today's choice. A later straddling-match case showed that cutting through a complete match prints its last bytes raw, so the tool now holds such a match with the tail. Mutation check (scratch script, one intended test per mutant, tool restored by sha256): 32 of 32 mutants caught by assertion failures, including base64 or hex needles removed, no held tail, idle flush of any tail, held tail printed at EOF, no continuation after an idle flush, no overlap merge, no safe boundary, UTF-8 decoding of output, inventory names kept, --only ignored, public names masked, no minimum length, no writer grammar, undeclared names accepted, mode, hard link and worktree checks removed, engine ids injected, the unknown id echoed, no -S, no re-exec, stdin not inherited, no signal forwarding, raw signal codes, same session, core dumps allowed, an unbounded drain, the exception message printed and --check printing values. Checked: tests.test_credential_run (31 tests, including with ResourceWarning as an error), tests.test_credential_status and tests.test_credential_tools all pass. These are local integration checks with synthetic values in temporary stores, not upstream acceptance. The guard model, the oracle's injector phase and the docs follow; the runner does not ship without the guard model. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ly, macOS, decisions) - docs/secret-storage.md: new section "Using a key (default path, 2026-09-29)", with an explicit using-a-key anchor for AGENTS.md. It covers the runner form and --check, what is refused, the one user step to add a key (a new provider also gets its inventory entry and SECRET_NAMES line in the agent's worktree), the honest limits (only injected whole values and the listed encodings; not fragments or transformed values, not files, logs or the network; stdout and stderr masked separately; the 4-byte held tail rule; environ visibility; the guard covers Claude's Bash tool only), and units, launchd, Codex and MCP stdio servers. The five environment-only consumers now use the runner, and the set -a subshell is marked deprecated but still documented. The keyring "Use it" paragraph now names the runner as the default and exec as the per-boot spare. - AGENTS.md: the design's credentials sentence (docs_and_default_path). The rest of the bullet stays. - recipes/tavily.md: every tvly command goes through credential_run.py tavily -- tvly <args>. The keyring exec and tvly-keyring lines stay as a per-boot spare that works only until the next restart, and macOS uses the stored file instead of secret run. - adoption/platforms/macos-arm64.md: "Keys under launchd". ProgramArguments go through the runner and EnvironmentVariables hold PATH only; the store is ~/.config, Tavily is a file, and the boot receipt is a later change. The ps capital-E guard fix is required before Mac acceptance. Not run on a Mac. - docs/decisions/2026-09-29-key-management.md: one appended section, "Part 4: the id-based key runner". It holds the selection, the references at their pins, and the alternatives with pins and evidence class (mise and agentself measured and rejected with the deciding measurements, dotenvx, ironrun, varlock and the design's list). It also records the one deviation (the held tail at EOF), the evidence class and the overturn comparisons. - docs/decisions/2026-09-22-broker-credential-handling.md: the macOS note said ps eww. It is now ps -E, per Apple's ps(1) (apple-oss-distributions/adv_cmds@6bed8737 ps/ps.1 L123-128: -E shows the environment and -e means -A; L602-609: the environment -e is legacy mode only). Failed first: tests.test_adoption_docs_consistency failed on the first draft (test_every_relative_link_resolves: the macOS page linked secret-storage.md#using-a-key, which is an <a id> alias, not a heading). All markdown links now use heading slugs computed with the test's own github_slug, and link_errors() reports none for the six changed pages. Checked: the brief's acceptance command (tests.test_credential_run, test_credential_status, test_credential_tools, test_install_claude_profile, test_adoption_docs_consistency and test_secret_path_guard) ran 219 tests, OK with 4 skipped. The guard's documented-keyring-commands test still finds every keyring line. scripts/validate.py reports only SHA-256 and byte-count mismatches of pinned files, which the coordinator re-pins in the branch's last commit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… masked names come from the schema module Review finding 6 (2026-09-29, read-only GPT-6 review at effort max of e437a361). A required variable listed again in optional_variables and in public_variables produced no schema error and dropped out of the runner's masked names, so the runner would have injected it unmasked. scripts/credential_status.py now rejects an entry whose variables and optional_variables share a name, and a name repeated within variables, optional_variables or pointer_variables (a public_variables name outside optional_variables and a repeat in public_variables were already rejected). It also owns public_names() and masked_names(): a public name is only an optional variable that is not also required. The runner's masked_names() and public_names() delegate to them. Failed first: test_variable_lists_reject_overlap_and_repeats_and_keep_ required_masked reported 0 errors for each of six planted entries (overlap, the review's overlap plus public, and repeats in three lists) and found no cs.masked_names; the runner test found a required name absent from masked_names(). Checked with tests.test_credential_status, tests.test_credential_run and tests.test_credential_tools: all pass. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…r; a closed stdin reads end of file Review finding 2 (2026-09-29, read-only GPT-6 review at effort max of e437a361). RLIMIT_CORE 0 stops a core file, not a collector. The kernel sets the limit aside for a core_pattern that begins with a pipe (a program such as systemd-coredump or apport) and, since 6.16, for one that begins with "@" (a core socket): torvalds/linux@v6.16 fs/coredump.c L795-820, L242-243 and L919. systemd-coredump then declines to store the core but still journals the crashing process's environment, COREDUMP_ENVIRON (systemd/systemd@v257 src/coredump/coredump.c L472-479 and L1458-1459), which holds the injected key. The runner now reads /proc/sys/kernel/core_pattern before it reads the store or starts anything, and refuses with core_pattern_pipe or core_pattern_socket (exit 1, the message names that file and never what it holds). A file that cannot be read is refused as core_pattern_unreadable; a file that does not exist (macOS, no /proc) skips the check. There is no override flag. A setrlimit(RLIMIT_CORE, 0) that fails is refused as core_limit_not_set instead of ignored. The "@" refusal goes beyond the review's wording, on the kernel source above. Out of scope: a debugger, ptrace or /proc reads by the same uid. Review finding 7. A stdin that was closed at start is reopened on /dev/null by os.open, whose descriptor is close-on-exec, so the command's exec closed it again and its read failed with EBADF instead of reaching end of file. Each reopened standard descriptor is now marked inheritable, including when the new number equals the target. Failed first: the core-pattern and limit tests had nothing to call (check_core_pattern and CORE_PATTERN_FILE did not exist), and the closed-stdin test showed the command printing errno 9 instead of eof. Tests run on a host whose real core_pattern pipes (common on CI runners) through a test-only launcher that points CORE_PATTERN_FILE at a temporary file; the two tests of the real re-execution skip there, and a test checks the real tool against this host's own pattern. Checked with tests.test_credential_run: 40 tests pass, also with CREDENTIAL_RUN_TEST_LAUNCHER=1 (2 skipped). Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…coders Review finding 3 (2026-09-29, read-only GPT-6 review at effort max of e437a361). The masker registered only the percent form written with no safe characters, and Python's json.dumps form, which for a value in the writer's grammar equals the value. The real encoders write more: urllib.parse.quote() keeps "/" by default (safe="/") while escaping "+", "=" ":" and "@"; quote_plus() writes a space as "+"; PHP's json_encode writes "/" as "\/"; Go's encoding/json writes < > & as < > & (PHP's JSON_HEX_TAG writes < >). encoded_forms() now registers the percent forms of quote() and quote_plus() with safe="" and safe="/", each in upper- and lower-case hex, and the JSON string form with and without "\/" and with no, lower-case and upper-case HTML escapes. Forms are deduplicated by needles_for(), so a value without those characters collapses to the few it needs. Forms that stay unmasked, as the documentation lists them: nested encodings, base64 wrapped across lines, other escapers (for example Gson's = for "=" or another safe set) and fragments; a test pins that list. Failed first: EncodedFormTests, over the output of urllib.parse.quote, quote_plus, json.dumps and the two hand-written variants for a bare value (+ / = : @) and a quoted one (space, < > & '), failed on 16 encodings; the runner printed for example tvly-<hex>/%2B%3D%3A%40<hex> and tvly-<hex>\/+=:@<hex> unmasked. Checked with tests.test_credential_run: 43 tests pass. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…wever long a run of matches is Review finding 4 (2026-09-29, read-only GPT-6 review at effort max of e437a361). Overlapping matches moved the hold-back boundary back to the start of the merged run, so a run such as six identical characters was never written, stayed in memory whole and was scanned again at every chunk: 31 chunks of 4,096 matching bytes emitted nothing and retained 126,976 bytes. Masker.feed() now writes everything up to the start of the tail that could still begin a needle, or up to the end of a complete match that runs into that tail, and keeps only the tail: at most the longest needle minus one byte. The bytes of a match that stay in the tail count as written, so a later match that starts in them is masked from there on, and a tail that never completes is not printed. A long run of repeated matches may therefore come out as several markers, and a match that B's partial tail overlaps is written whole at once (as one marker before, when B failed to complete); the existing straddle, overlap and idle-flush tests pass unchanged. close() now weighs the held tail itself (SHORT_TAIL_MAX), not the tail plus a match it overlaps, which also stops a complete key at the end of the output whose last byte could begin another form (a hex "d") from being replaced by a partial marker. Failed first: BoundedHoldbackTests retained 4,096 bytes after the first 4,096-byte chunk of the review's value (bound 5) and wrote nothing, wrote nothing for the straddling match, and exceeded the bound in 76 random and pathological rounds. Now: the same input keeps at most 5 bytes and writes at every chunk; 250 random outputs that are not pathological equal the whole-buffer masker byte for byte; 80 periodic runs leave the same unmasked bytes as it does. A scratch fuzz over 40,000 dense-overlap streams in four chunkings (not committed) found no difference in the unmasked bytes and no retention above the bound. Checked with tests.test_credential_run: 49 tests pass. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…er cannot hold the runner Review finding 5 (2026-09-29, read-only GPT-6 review at effort max of e437a361). Output was written with blocking os.write calls, so a consumer that stopped reading held the runner in write: it stayed alive 2.7 s after SIGTERM and exited 143 only when the consumer resumed, and the drain deadline meant nothing. The relay now writes through a Sink per output descriptor. A pipe or socket is made non-blocking for the relay and put back afterwards; what the consumer does not take waits in a queue of at most QUEUE_LIMIT (256 KiB) per stream, watched for writability by the same select loop, and the relay stops reading the command's pipe while the queue is full, so the command waits as in any pipeline. Once a shutdown signal has arrived, nothing waits for the consumer: the queue is dropped, later output is written only if it is taken at once, and reading goes on so the command can exit. Once the command has exited, the consumer has DRAIN_SECONDS to take what is queued, then the rest is dropped. Dropping is safe: only masked bytes are ever queued. A terminal, a regular file and /dev/null are written directly, since they cannot wait on a reader that never comes, and a descriptor that shares its inode with the command's stdin is never flipped, because its flags are the command's too (an interactive terminal); EPIPE ends that stream as before. A runner killed by SIGKILL leaves a flipped shared descriptor non-blocking. Failed first: with a flooding command and a consumer that never reads, the runner was still alive 20 s after SIGTERM, and with a command that wrote 150 kB to each stream and exited 7 it was still alive 20 s after that exit (both subprocess.TimeoutExpired); the Sink tests had no Sink. Now the first exits 143 within 10 s of the signal (measured: about 0.1 s), the second exits 7 about DRAIN_SECONDS after the command (measured: 2.1 s in all), a slow consumer still gets 700,000 bytes on each stream intact, a closed reader still ends the command, stdout and stderr on one pipe (2>&1) shut down together, and a terminal shared with stdin stays blocking. Checked with tests.test_credential_run: 59 tests pass, also with CREDENTIAL_RUN_TEST_LAUNCHER=1 (2 skipped). Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…n signal Follow-up to the non-blocking output change (review finding 5). The mutation run showed two rules that no test pinned: a mutant that kept the queue after a shutdown signal still exited within the drain deadline that follows a command's exit, and a mutant without the read-side backpressure passed every test. The SIGTERM test now requires the runner to exit in less than DRAIN_SECONDS - 0.5 after the signal (measured: about 0.1 s), because after a shutdown signal nothing waits for the consumer. A new test has a command that counts the 64 KiB chunks it has written to a consumer that never reads: with the queue bound and the pipes it stops after a handful, where a runner that kept reading would let the count run into the thousands; the test asks for 1 to 24. Both mutants are caught now. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…mmand guard models it Review finding 1 (2026-09-29, read-only GPT-6 review at effort max of e437a361). AGENTS.md and the docs called the runner the default path, but the command guard does not model it yet. A check of this checkout's guard returned native_token_print for "tvly auth" and None for "python3 tools/credentials/credential_run.py tavily -- tvly auth", and a plain "tvly auth" prints a key's first eight and last four characters, which masking cannot catch. Making the runner the default first would reopen shapes the guard refuses today. The guard model is phase 2 of the same pull request series, by another builder, after the guard tightening lands. AGENTS.md is restored to the base sentence. recipes/tavily.md is main's text again (the interim rotation step of #490 and the keyring exec commands are the default) with the runner added as an option: its commands, --check and --json, and a dated sentence that the guard does not read its command yet (phase 2) and that a "tvly auth" behind it would print part of the key. docs/secret-storage.md "Using a key" (now titled "available") says the runner is available and becomes the default after the guard model; the Alpaca consumer and keyring exec passages it had replaced say subshell and exec again, with the runner as an alternative. The section also lists the new core_pattern refusal, the encoder forms, the bounded hold-back, the non-blocking output with its 256 KiB queue and 2 s drain, the forms that stay unmasked (nested encodings, wrapped base64, other escapers, fragments) and the residual that a same-uid debugger, ptrace or /proc read is out of scope. The decision record's Part 4 gets a "Not yet" line for the guard model and the default-path flip, the repair round with its pins (torvalds/linux v6.16 fs/coredump.c, systemd v257 src/coredump/coredump.c), new evidence counts and an overturn condition; the macOS page's runner paragraph says "may" and notes that the core-pattern check is skipped there. The module docstring of credential_run.py carries the same status. Checked with tests.test_credential_run, tests.test_credential_status, tests.test_credential_tools, tests.test_install_claude_profile, tests.test_adoption_docs_consistency and tests.test_secret_path_guard. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…forward all fatal signals, ask for SIGTERM when the runner dies (second review, item 1) The second review found that the command could outlive the runner: it runs in its own session, only SIGINT, SIGTERM and SIGHUP were forwarded, a SIGQUIT, SIGUSR1, SIGUSR2 or SIGALRM ended the runner alone, and descendants still running after the 2 s drain were left running with the key in their environment. - run_command ends the command's process group on every way out (a finally): SIGTERM, TERM_GRACE_SECONDS (2 s), SIGKILL, then reap. It returns at once when no member is left, so a normal run pays nothing. Same order as bazelbuild/bazel@d2545923 src/main/tools/process-tools.cc KillEverything L94-110. - SIGQUIT joins the shutdown signals; SIGUSR1, SIGUSR2 and SIGALRM are forwarded too, without the shutdown flag, because a command may handle them and keep running (dd prints progress on SIGUSR1) and the flag would drop its output for the rest of the run. The exit code is the command's, 128+N when it died of N. - On Linux the command asks the kernel for SIGTERM when the runner dies (PR_SET_PDEATHSIG through ctypes, set in the forked command before exec, skipped where unavailable or on failure, never failing the start). It fires when the thread that forked the command ends; run_command's signal.signal calls already confine it to the main thread. A runner that ended before the call is noticed by comparing the parent pid. Failed first (the five new or changed tests on the previous head, 11 failures): SIGQUIT, SIGUSR1, SIGUSR2 and SIGALRM ended the runner alone (exit -3, -10, -12, -14 instead of the command's code or 128+N); a descendant outlived the runner; a failure inside the relay left the command running (returncode None instead of -15); a SIGKILLed runner sent its command nothing. Limit that remains, stated in the documentation with the next commits: the parent-death signal reaches the command only (the kernel clears it for the command's children, man7.org PR_SET_PDEATHSIG(2const)); a descendant that leaves the process group (setsid, setpgid) is out of reach of the group kill; a SIGKILL of the runner before it forked, and a same-user debugger, are out of scope. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…when the runner is killed (second review, item 1, the SIGKILL case) PR_SET_PDEATHSIG (the previous commit) reaches the command only: the kernel clears it for the command's own children (man7.org PR_SET_PDEATHSIG(2const)). Measured on the previous head with the new test: a SIGKILLed runner ended the command, and a descendant it had started went on and wrote its marker file. The brief's own test (the grandchild dies after a SIGKILL of the runner) cannot pass with the parent-death signal alone, so the runner starts a watchdog. - The watchdog is a second interpreter (-I -S, an empty environment, its own session) started right after the command. It holds a pipe whose write end only the runner has (made after the command, not inheritable) and the number of the command's process group. When the runner dies by any signal the kernel closes the pipe, the watchdog reads end of file and ends the group: SIGTERM, TERM_GRACE_SECONDS, SIGKILL. A runner that ends in order writes one byte first, waits for the watchdog to exit and leaves none behind; the watchdog then touches nothing (exit 0; it exits 1 after acting on end of file). - It ignores SIGINT, SIGTERM, SIGHUP, SIGQUIT, SIGUSR1, SIGUSR2 and SIGALRM, and lives in its own session, so a group-wide kill of the runner (timeout -k) does not reach it. CPython's multiprocessing resource tracker is the same shape: a helper that waits for the end of a pipe and ignores SIGINT and SIGTERM (python/cpython@v3.13.15 Lib/multiprocessing/resource_tracker.py L8, L246-267, L425-429). - It cannot fail the run: a watchdog that cannot start leaves the parent-death signal and the group kill at the end of run_command. WATCHDOG is a module constant that the test launcher can switch off, to see the parent-death signal alone; there is no environment switch. Tests: the tree test starts the command, a descendant and one that ignores SIGTERM, each writing a marker after a delay; the runner is SIGKILLed and none of the three may write it (the SIGKILL comes after the 2 s grace). The watchdog is also tested on its own: end of file ends the group, a byte first leaves it alone, a member that ignores SIGTERM is killed after the grace, and the seven signals do not stop it. A runner that ends in order tells its watchdog (exit 0) and leaves none. Limits, unchanged from the previous commit and to be stated in the documentation: a SIGKILL of the runner before the watchdog started (a few milliseconds after the command's fork; the parent-death signal still reaches the command), a descendant that leaves the process group with setsid or setpgid, a set-user-ID command (the kernel clears the parent-death signal and none can be signalled), and a same-user debugger. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…served-name rule and the relay's stdin sharing (second review, items 2 and 3) Item 2. The runner removed the variables, optional_variables and must_not_be_set names of every entry from the command's environment, but not the pointer_variables of the other entries. A command run under one id could therefore load another entry's store file through a pointer that the documented shell profile sets (ENV_FILE, SEC_CONTACT_ENV, PIT_ALPACA_ENV_PATH, PAPER_ENV_FILE_2, ...). child_environment now takes the selected entry and removes every entry's pointer variable except the ones that entry itself declares (alpaca-paper keeps PAPER_ENV_FILE, ENV_FILE and PIT_ALPACA_ENV_PATH, the paper units' own pointers; tavily keeps none). Failed first: the changed test_strips_other_inventory_names_from_the_child and a new inventory-driven test (for each of the seven injectable ids, all pointers of the inventory set in the caller's environment; the command must see exactly its own entry's) failed on the previous head, 8 failures. Item 3. Two rules had no pinning test. Both were checked with a mutant against a copy of the previous head's test module: the mutant survived. - reserved_variable: a planted inventory entry that declares LD_*, DYLD_* or PYTHON* in variables or optional_variables is refused before any store is read (in-process main with a planted inventory, and a read_store that fails if it is reached), and a name that merely starts alike (LDAP_BIND_KEY, MY_PYTHON_KEY) is not refused. Mutants: the check removed; only LD_; required variables only. All three survived the previous suite and are caught now. - the relay's stdin identity: one socketpair end is fds 0, 1 and 2 of the runner, the command looks at os.get_blocking(0) after its output has passed through the relay, and it must still be blocking. The existing pty test cannot fail on this (a terminal is never flipped). The mutant that builds the sinks without the stdin identity survived the previous suite and is caught now. - also new: a command that handles SIGUSR1 and keeps writing is delivered every byte although the consumer is slow, which pins that only the shutdown signals start the runner's no-wait rule (a mutant that treats every forwarded signal alike is caught). Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…tests are renamed, a neutral fixture name for CodeQL (second review, items 5 and 6) Item 5, tests. The decision record said "a test pins the list" of unmasked forms, but the test printed five forms and never looked at the runbook. - print_forms reads the "What masking does not cover" list of docs/secret-storage.md (whitespace normalised) and asserts, for every form it prints, that the list names it; it then checks the runner prints the form as it is. Negative control: with the phrase "colour codes" removed from the runbook the test fails. - New forms, each named in the runbook now: fold-wrapped, xxd and hexdump -C columns, wrapped table cells, colour codes, hex with separators, a value with a single quote requoted the way set -x, printf %q and shlex.quote write it, a value interleaved with the bytes of a second writer (a second process on the same pipe), and a value written by the command to an inherited read-write stdin (a socket end stands in for a terminal), which never passes the relay. - The tests named after "every real encoder" are renamed (test_the_percent_and_json_encoder_forms_...): the PHP and Go JSON forms are string-replacement models of what those encoders write, and only the urllib.parse and json.dumps forms are executed; the helper's docstring says so. - credential_run.py: the comment on the JSON escape forms had lost its escape examples (a tool pipeline decodes a backslash followed by u and four hex digits); they are restored. No code change. Item 6. The pull request's code scanning reported py/clear-text-storage-sensitive-data (high) at line 141, the path.write_bytes(...) in the plant helper. The SARIF data flow of the analysis (1856995192, gh api with Accept: application/sarif+json) starts at the variable named "secret" in RunnerCase.alpaca(), which holds a fake() value: a name heuristic on a synthetic fixture. The variable is renamed to a neutral name, with a comment saying why; no suppression comment is added. CodeQL was not run locally, so CI is the check. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…e key runner (items 4 and 5) docs/secret-storage.md, "Using a key" only: - says which signals the runner forwards (SIGINT, SIGTERM, SIGHUP, SIGQUIT, SIGUSR1, SIGUSR2, SIGALRM) instead of "signals pass through"; - says that the other entries' pointer variables are removed from the command's environment and the entry's own stay; - restores the JSON escape examples (Go's and PHP's forms) that a tool pipeline had turned into the characters they stand for; - adds "The command's lifetime": the group kill at the end and on every way out, the parent-death signal, the watchdog; - adds to the limits list, once each and dated 2026-09-29: the non-blocking flag on the open file description (with the planned fix); that "cannot hold the runner" holds only for the pipes and sockets the runner can flip; the new unmasked forms and the rule that a value is masked only where a whole form appears unbroken in one stream; and the command's lifetime limits (a SIGKILL before the watchdog started, a descendant that leaves the process group, a set-user-ID command, Linux only, ptrace). docs/decisions/2026-09-29-key-management.md, Part 4 only: - the second repair round with its six items, the sources it read (man7.org PR_SET_PDEATHSIG(2const), python/cpython@v3.13.15 resource_tracker.py, bazelbuild/bazel@d2545923 process-tools.cc, krallin/tini@924c4bd6 tini.c, torvalds/linux@v6.16 fs/coredump.c), the limits, and the CodeQL finding with its data flow; - an unverified lead, not a claim: at RLIMIT_CORE exactly 1 the kernel aborts a piped core dump (fs/coredump.c L801-819), so the runner might set (1, 1) and work on a host with a piped core pattern; not tested against a real collector; - claims the review showed unsupported are corrected: "a test pins the list" (a test now reads the list), "the same unmasked bytes" (the committed check compares outputs with the markers removed), the timing figures (the tests assert under 1.5 s and 1.5 to 10 s; 0.1 s and 2.1 s were single manual measurements), the Go escape example that had lost its backslash sequence; - a second deviation from the build contract: public names skip the 6-byte minimum length, being unmasked; - the mise spike's note that __MISE_DIFF can be closed with unset in a task, which does not change the rejection (a masked run passes no stdin); - the evidence class updated to the test and mutant counts of this round. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
A docstring line and the WATCHDOG comment were 121 and 122 columns; the file had none before this round. No code change: the final mutation pass runs on this commit. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
… zombie holds the number, and put the runner's handlers back before the parent-death signal is armed (third review, findings 1 and 2) Finding 1 (high). The runner's final killpg and the watchdog kept only the number of the command's process group and acted after the command had been reaped. Once nothing holds a number the kernel may give it to a stranger, and a signal to it reaches the stranger's group. - Command wraps the command and is the one place that reaps it. The runner learns that the command exited without reaping it (os.waitid with WEXITED|WNOHANG|WNOWAIT in the relay, WEXITED|WNOWAIT when it waits), signals the group while the command is still a zombie, whose pid and so the group's number nobody can take (Linux kernel/pid.c __change_pid; XNU kern_fork.c skips a pid that is a process, group or session id), then tells the watchdog to stand down, and reaps last. signal_group() refuses once the command is reaped, forward() uses it, and the forwarded signals are blocked while the command is reaped, so no handler runs between the reap and its record. The watchdog therefore acts only when the runner died without telling it. - kill(-pgid, 0) cannot tell that a group is empty while the command's zombie is in it (a zombie stays a member until reaped, on Linux and in XNU's reap_child_locked), so the live members are counted from /proc (live_members): a command that leaves nothing but its own zombie is not signalled at all, and the wait after SIGTERM ends when the count is 0. - A default SIGCHLD is set before the command starts: an inherited ignored one makes the kernel reap the command at once (no zombie, no status). - PINNED (os.waitid with WNOWAIT and a /proc) is False on macOS: os.waitid is not compiled for Apple before CPython 3.13 (posixmodule.c HAVE_WAITID && !defined(__APPLE__) in v3.9.25 to v3.12.0), and macOS has no /proc. There the exit is seen by reaping, the group is never signalled after that, and only a failure inside the runner (the command unreaped) ends it: SIGTERM, the grace, SIGKILL. A recorded loss, in the docs. The brief took os.waitid for available on macOS; it is from 3.13. - What remains, in the docs: if the runner dies before it stands the watchdog down and init reaps the command before the watchdog acts, a recycled number is theoretically possible on a host with a small pid space. Failed first (the previous head with only PINNED declared): the recorded events showed killpg on a reaped command in the runner's final cleanup and in the probes after it ([killpg 15 reaped], [killpg 0 reaped] ...), the same for a command that left nothing behind, and a signal that reached the runner after the reap was forwarded. Tests: the recording launcher shows every killpg with the command running or a zombie and the stand-down before the reap (a lingering descendant that ignores SIGTERM and holds no pipe: TERM, KILL, both on the zombie); a command that leaves nothing is not signalled; Command and live_members on real processes; the fallback (PINNED False) never signals after the reap, and a failure in the relay still ends the group; a signal after the reap is not forwarded; SIGCHLD ignored at start still gives the command's status; a descendant that obeys SIGTERM ends the wait at once and is never sent SIGKILL. The recorder also sees the stand-down byte itself (a write of one "."), which precedes the reap. The whole module also passes with PINNED forced off in this process and every runner, as a macOS without os.waitid and /proc would run it (92 tests, 11 skipped: 9 that need the pinned cleanup and the 2 of the real re-execution). Finding 2 (medium). The command's pre-exec code ran with a copy of the runner's handlers; forward() only records a signal, so a parent-death SIGTERM that arrived between the parent-pid check and the exec was queued in the copy and the command went on to exec and run (the reviewer's probe: pause after the check, kill the runner, resume: exit 42). The hook now puts every signal the runner handles back to SIG_DFL and clears the inherited signal mask before it arms PR_SET_PDEATHSIG, and keeps the parent-pid check after arming. pre_exec_pause(stage) is a module function that does nothing; the test launcher replaces it (RUNNER_TEST_SETUP is read by the launcher, never by the tool) to hold the forked command at "before_arming" or "after_check". Failed first (with the seam in place and the reset absent): the command outlived its killed runner, and all seven handlers were the runner's and SIGUSR1 stayed blocked. New tests: killed while held after the check the command is gone and never runs; killed while held before arming it ends by the parent-pid check and never runs (a pin of a rule that had none); started with a signal blocked it sees default handlers and an empty mask. Mutants, one per new rule (16, all caught). The mutation script now clears the cached bytecode around each mutant: a same-size mutant written in the same second as an earlier compile was served the earlier code once (a survivor that a rerun caught). Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…ile signalled, pre-exec handlers) docs/secret-storage.md, "Using a key" only: - "The command's lifetime" says that the runner signals what is left of the group while the command is still a zombie that holds the group's number, then stands the watchdog down and reaps last, and that the forked command arms PR_SET_PDEATHSIG only after its copy of the runner's handlers is put back to the defaults; - the lifetime limit (dated 2026-09-29, once): what remains if the runner dies before it stands the watchdog down and init reaps the command first (a recycled number on a host with a small pid space, macOS stops at 99,999; the watchdog's prompt reaction bounds the window); that the pinned cleanup is Linux-only (no os.waitid on macOS before Python 3.13, no /proc there) and what the runner does elsewhere (sees the exit by reaping, signals nothing after it, so a descendant of a command that already exited is left running). docs/decisions/2026-09-29-key-management.md, Part 4 only: - the third repair round with its two findings, the failing-first evidence, the platform correction (the brief took os.waitid for available on macOS; it is from CPython 3.13) and the sources read: man7.org wait(2) WNOWAIT, torvalds/linux@v6.16 kernel/pid.c __change_pid L349-369, xnu-12377.121.6 kern_fork.c L972-974, kern_exit.c L2969 and proc_internal.h L767, and CPython posixmodule.c and os.rst for os.waitid; - the selection bullet and the limits list brought up to date; - the evidence class: the test and mutant counts of this round. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…source does not support; record the watchdog's measured reaction - end_group's docstring said that bazelbuild/bazel's KillEverything signals "after waitpid has reaped its child". Read at d2545923: the wrapper calls KillEverything from its signal handlers (process-wrapper-legacy.cc L172-180) while it waits, and process-tools.cc L94-110 gives the order (SIGTERM to the group, a timeout, SIGKILL to the group), which is all the docstring may claim. The clause is removed; no code changes. - The runbook and Part 4 said the watchdog reacts "within milliseconds". Measured once, 25 runs on an idle host, from the pipe's closing to the death by SIGTERM of a sleeping member of its group: median 1.3 ms, largest 1.4 ms. The figure is now stated where the claim is, with its scope. Also checked on this host (not a repository test): a zombie group leader that is the only member keeps killpg(pgid, 0) succeeding until it is reaped, and ESRCH after, which is why the group's live members are counted from /proc. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…commit, last) Hashes of the seven changed pinned files on top of main's manifest at b2767df (docs/lanes.md protocol: main's copy, register each file, component_matrix and new_host_grand_list --write; validate.py passes: 69 components, 8080 hashed files, 171 receipts). No receipt entry: this PR records no native run; the first real use is the later canary proof. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
seathatflowsinourveins
force-pushed
the
claude/key-injector-20260929
branch
from
September 29, 2026 12:25
5a01baa to
46100a4
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Scope
tools/credentials/credential_run.py, a small stdlib runner that reads one inventory entry's private 0600 store file and puts ONLY that entry's declared variables into the environment of ONE child command (never argv, never a temp file), runs the child in a new session with core dumps refused, and masks the values in the child's stdout and stderr (raw, base64 and base64url at three alignments, percent-encoded with both safe sets, JSON with and without\/and HTML escapes, hex). It also adds an optionalpublic_variablesinventory field (injected but never masked, such as a base URL) with its schema checks, and documents the tool as AVAILABLE with its limits.b2767df2lane:foundationtools/credentials/credential_run.py(new),tests/test_credential_run.py(new),adoption/credential-inventory.json,scripts/credential_status.py,tests/test_credential_status.py,docs/secret-storage.md("Using a key" section),recipes/tavily.md(the runner as an option),adoption/platforms/macos-arm64.md(a drafted Mac paragraph),docs/decisions/2026-09-29-key-management.md(Part 4),docs/decisions/2026-09-22-broker-credential-handling.md(the macOSpsnote now says-E), and the last commit'smanifests/evidence.json. Not touched:AGENTS.md(the default-path sentence waits for the guard model), the guard, any credential file, any unit.credential_run.py <id> -- printenvis allowed by the guard and protected only by masking, and atvly authbehind the runner would print part of the key (masking cannot catch a partial);AGENTS.mdand the recipe therefore keep the keyring chain as the default.SOTA sources
278101db(redactOutput and held-back partial-match tail), actions/runner15231bed(SecretMasker.cs,ValueEncoders.cs: overlapping-match merge, base64 alignments, percent and JSON escapes), buildkite/agent3345ee60(redactminimum length 6, replacer), varlock1b880652(redact-stream), ironrunb611c7ce(internal/redact/encodings.go).2184db810a7e2c29475abdba53e5af910e96859a: masks only undermise run, passes no stdin to a masked run, drops the rest of stdout after a non-UTF-8 line, injects a decodable copy of the environment as__MISE_DIFF, expands$in values, echoes the store path and raw line on a malformed line, no id concept, no minimum length) and agentself v0.2.4 (commit418e7d790849808a97819baa53594633645f114b: exits 0 when the child exits 7, returns output only after the child exits inside one JSON object, imports a store file as one whole value, no release asset), plus dotenvx v2.31.1 (runexecutes$(...)in a value; banner prints the store path). Details and pins are in the decision record.fs/coredump.cat v6.16 (a|core pattern sets the size limit aside) and systemd v257src/coredump/coredump.c(collects and journals the process environment asCOREDUMP_ENVIRONeven when its zero-limit check skips saving the core); the runner refuses a|or@core pattern, an unreadable pattern file and a failedsetrlimit.-Iand-Ssemantics (the runner re-executes itself under-I -S:-Ialone still runs.pthcode from site-packages),subprocess/selectnon-blocking pipe handling,urllib.parse.quote/quote_plus/jsonencoder outputs (the tests use the real encoders); Appleadv_cmdsps.1(-E, cited for the corrected note).Evidence-class table
local_integration(failing-first tests)python3 -m unittest tests.test_credential_run(92 tests)local_integrationlocal_integrationLocal commands run
Decision record
docs/decisions/2026-09-29-key-management.md, "Part 4": the selection (build from cited reference designs after measuring mise and agentself), the alternatives with pins and the deciding measurements, the deviations (the 4-byte end-of-output threshold;-I -S; a stricter parser than the reference), what masking does and does not prove, and what is not yet done (the guard model and the default-path flip).Cross-family review: one read-only GPT-6 round (
gpt-6-astra, effort max) on the phase-1 diff returned seven findings, all fixed in one repair round with failing-first tests: (high) the docs and AGENTS.md declared the runner the default before the guard reads it, re-openingtvly authshapes that are refused today: AGENTS.md is restored, the docs say available; (high)RLIMIT_CORE=0does not stop a piped collector from journalling the child's environment: a|or@core pattern, an unreadable pattern and a failedsetrlimitare refused; (medium)quote()with its default safe set,quote_plusand the\/and HTML-escaped JSON forms escaped masking: registered and tested with the real encoders; (medium) repeated overlapping matches kept the whole stream buffered: completed matches are emitted, retained state is bounded; (medium) blocking output writes defeated the shutdown deadline: non-blocking queued writes with deadlines; (medium) a required variable could be declared public: the schema rejects overlap and duplicates; (low) a reopened stdin was closed again at exec: inheritable.Second and third rounds: an independent Opus review of the runner found no path that prints a whole masked value and six process-level items, and a targeted GPT-6 review of the resulting watchdog and group-kill code found two more; all are fixed with failing-first tests: the command could outlive a killed runner (a process-group kill on every way out, a watchdog process that ends the group when the runner is SIGKILLed, and PR_SET_PDEATHSIG for the command), other entries' pointer variables reached the command (stripped), two rules had no pinning test (added), the docs limits and several unsupported claims (corrected), a group number could be reused after the reap (the group is signalled only while the command's unreaped zombie holds its number; on macOS, where os.waitid is not available before CPython 3.13, the runner never signals after the reap and says so), and the pre-exec handler could swallow the parent-death signal (handlers reset before it is armed). Known limits stated in the docs: on macOS the leftover descendants of an already-exited command are not ended (untested there), the non-blocking flag is set on an inherited open file description (fix planned before any default-path flip), and the runner module's tests take about 90 s.
Host evidence
Not applicable: no file under
evidence/hosts/changes; nothing is installed on the host by this PR. The tool has not touched the real store or a real key.Checklist
permissions: contents: read: no workflow changes.🤖 Generated with Claude Code