Repository navigation
Keyring guard hardening, tvly-keyring wrapper and Tavily Research qualification receipt (follow-up to #325) - #329
Merged
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
seathatflowsinourveins
enabled auto-merge (squash)
September 26, 2026 10:23
seathatflowsinourveins
force-pushed
the
claude/keyring-guard-tvly-20260926
branch
from
September 26, 2026 11:08
f93ede8 to
4b238fd
Compare
… receipt
Guard (scripts/hooks/secret_path_guard.py; SHA256SUMS updated):
- Unwrap `kernel_keyring.py exec <name> <ENV_VAR> -- <command>` (any path to
the script, any launcher) and `tvly-keyring <args>`, so every existing rule
applies to the command they start.
- keyring_variable_reference: the started command, code piped into it or a
here-document names the injected variable beyond exec's own argument.
- environment_dump_in_keyring_exec: the started command dumps the environment
it inherits (env, printenv, set, export -p, declare -p, ps e, ${!...},
env or printenv as another program's argument, or whole-environment
access in interpreter, awk or jq code).
- keyring_payload_read: keyctl print, pipe, read and dh_compute (keyctl(1):
request, request2 and prequest2 print a key ID and pass), and payload reads
in inline interpreter code (KEYCTL_READ, keyctl_read(, keyutils.read_key,
kernel_keyring imports, raw keyctl syscalls with operation 11).
- `tvly auth` without --json (prints part of the key) is a
native_token_print; ~/.tavily/config.json is a native store path;
TAVILY_API_KEY is a secret name.
Credential inventory: `tavily` row with the new store kind kernel_keyring
(memory only, same-uid readable, lost at kernel restart). credential_status
reports it `unchecked` and never queries the keyring; set_credential refuses
it.
adoption/tools/tvly-keyring: POSIX sh wrapper that runs the real tvly through
kernel_keyring.py exec, never reads the key, passes arguments verbatim, and
exits 2 when the key is absent. Local integration tests with a fake tvly and
throwaway keyring keys (tests/test_tvly_keyring.py).
Evidence: tavily-research-qualification-20260926 receipt and a sanitized
summary of 33 completed Research (--model pro) reports: native provider
execution through the keyring exec, no quality comparison. The receipt and
the changed hash-listed files still need registration in
manifests/evidence.json.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
…causes Each reviewer finding was reproduced against the committed guard first (check() returned None for all four), then fixed: - Split-quoted exec argument (KK_DEMO_TO"KEN"): the variable check no longer subtracts a count of parsed arguments from raw-text matches. It exempts only the <ENV_VAR> argument's own span in the command after quote removal, so a split argument, an env/eval re-parse or a nested sh -c cannot use up the exemption. Backslash-newlines are joined first (a line continuation could hide the whole exec), and every raw-text rule also reads the unquoted command (sh -c 'echo $GH_TO''KEN'). - Launchers: strip_prefix reads launcher options as getopt does, from each tool's --help (stdbuf -o0, nice -n, timeout -s, sudo -iu, GNU time -f; xargs -i/-l/--max-lines take only glued values, checked with GNU findutils 4.9.0), and strips output redirections before a command. In a keyring exec, every argument naming a shell, interpreter, awk, jq, env, printenv, ps or a variable-printing builtin is read as a command from there on (watch, flock, taskset, find -exec). - keyctl list/rlist print a user key's payload as integers (keyctl(1): "No attempt is made to check that the specified keyring is a keyring"; act_keyctl_list/rlist in keyutils c076dff2). They pass only on @t @p @s @U @us @g, -1..-6 or %:name/%keyring:name; @A, serials and %user: are blocked, inline subprocess use included. - Redirection operands (> >> 2> &> <<< < << <<-) are dropped before the tvly --json/--help and keyctl target checks. The working-tree draft this replaces counted tokens across overlapping segments and blocked the documented exec inside sh -c/bash -lc; those forms are now ALLOWED regression cases. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
… finding) The Claude reviewer found the documented tvly-keyring extract form falsely blocked (environment_dump_in_keyring_exec) when the URL's last path segment is env or printenv. launched_commands now skips URL-shaped words; find -exec env and stdbuf /usr/bin/env inside an exec stay blocked. Guard hash updated in adoption/hooks/claude/SHA256SUMS. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
Re-check of 74508203 (codex exec gpt-6-astra max, read-only): F2 resolved; F1, F3, F4
had variants and one false block. Each reproduced, then fixed:
- "$KK_DEMO_TOKEN"x: unquoted() merged it into another name; the variable rule now
reads the written and the unquoted text (the exec argument exemption accepts quotes).
- keyctl rlist <serial> </dev/null: the serial was read as a descriptor; list and rlist
are blocked whatever the target (kernel_keyring.py status answers presence).
- tvly auth {fd}>--json: bash's named descriptor is now a redirection.
- tvly-keyring search "export" ... --max-results: declare/export flags are short
options only (false block removed).
Regression cases added; docs/secret-storage.md and the guard hash updated.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
…(hot-file commit) Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
seathatflowsinourveins
force-pushed
the
claude/keyring-guard-tvly-20260926
branch
from
September 26, 2026 12:54
4b238fd to
182c702
Compare
seathatflowsinourveins
deleted the
claude/keyring-guard-tvly-20260926
branch
September 26, 2026 13:07
seathatflowsinourveins
pushed a commit
that referenced
this pull request
Sep 26, 2026
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
Collaborator
Author
|
Trading lane, retroactive check (2026-09-26): no problem. This PR changed only 🤖 Generated with Claude Code |
seathatflowsinourveins
added a commit
that referenced
this pull request
Sep 26, 2026
…lly on GPU + MTP) 5x decode at equal F1, now serving (#342) * li26 results: C2 serving profile selected and recorded, llama-cpp host receipts, served model pin Record the preregistered local-inference-latest-20260926 comparison as run on nativestack-5975wx-20260925 (native execution, one run per arm, frozen protocol; speed and memory are this host's under its load at the time): - results/: the frozen analyze.py decision (final, C2 selected, change_serving_profile; a rerun reproduces it byte for byte), the acquisition summary and every window summary and per-arm window record, all copied unchanged, plus results.md with the per-arm table, bootstrap bounds, why M failed criterion (1), B and X not run, the retained failed attempts and the dated operations record. - experiment.json is now observed: adopt_within_scope for the C2 profile, qualified by li26-c2-w3; the acquisition's first refusal (state_dir_must_be_owner_only), C2's admission refusal in window 2 and M's failed criterion stay in failures_and_skips. - README: a dated results section below the unchanged frozen text. - llama-cpp install and use host receipts (catalog_revision 2e77144) with qualified models Qwen3.8-27B UD-Q4_K_M 4ca7207 (pass, C2 profile) and MiMo-V2.6-Distill-Qwen-9B Q8_0 81baddc (measured, fail); the b11057 catalog pin is not rebound. - evidence/receipts/local-inference-c2-serving-switch-20260926.json: the production switch, its post-switch checks and the rollback. - manifests/stack.json: the generation model entry now names the served unsloth/Qwen3.8-27B-GGUF UD-Q4_K_M file (revision 4ca7207, sha256 322e194f...) and the C2 serving profile flags. - The component evidence matrix is regenerated (llama.cpp alternative: receipts_recorded); the li26 test now checks the recorded state and the results' no-document-text boundary. manifests/evidence.json is not changed here: the coordinator registers the 19 files and the switch receipt. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]> * li26 repair: re-record the llama-cpp use receipt with read-only commands Review (gpt-6-astra, medium) found that the use receipt's sixth command made a directory with mktemp inside the checkout, wrote decision.json there through analyze.py --out and deleted it with rm -rf, contradicting the receipt's statement that its commands changed no file. Confirmed against the receipt and scripts/host_receipts.py record, which runs every --cmd in the checkout. - The use receipt is re-recorded at catalog_revision 2e77144 (published on origin/main). Its first five commands are unchanged. The sixth loads the frozen analyze.py under python3 -B, calls its analyze() on the retained private state and hashes the decision serialized as main() writes it, all in memory. It printed the same two lines as the first recording: final, C2 selected, change_serving_profile, and SHA-256 672be297..., the committed results/decision.json. A before/after mtime scan of the checkout, the private state directory, the systemd user configuration and the model and runtime directories showed no change beyond the receipt file and the manifests/evidence.json registration that host_receipts.py record itself writes (reverted; the coordinator registers). The receipt, results.md and the switch receipt say that it replaces the unpublished first recording (11:26:21Z). - Dependent hashes: the switch receipt's post-switch readback now names both receipts with their observation times (install 11:26:02Z, use 12:41:06Z) and the new nvidia-smi reading, and its provenance carries the new use receipt and results.md hashes. experiment.json's results.md artifact hash follows results.md. - test_host_receipt_commands_only_read fails on the first recording (command 6: mktemp) and passes on this one. It also ties the sixth command's printed hash to the committed decision.json. The install receipt keeps its recording: neither of its commands creates, writes or removes a path, and the new test checks both receipts. Checks, with the registration applied temporarily and then reverted: validate.py passed; validate_convergence.py --all-recorded 23 valid; host_receipts.py validate 165 passed; component_matrix.py --check checked; uv run --no-project --with jsonschema --with pyyaml python -m unittest over 10 modules, 652 tests OK. manifests/evidence.json is not changed. Re-register experiment.json, results/results.md, the use receipt, the switch receipt (its receipts[] claim and limitations too) and the li26 test file. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]> * li26 receipts: independent reviews; install receipt re-recorded with python3 -B A separate headless Claude session reviewed both llama-cpp host receipts: - use receipt: independent_session agree. - install receipt (first generation): needs_changes. verify-arms ran without -B (a possible bytecode write, against its read-only limitation) and limitations[1] overstated the recorded source of the MiMo pin. Kept unchanged with that review. - install receipt -2 (supersedes it): re-recorded from a clean checkout at published main 3822178 with python3 -B and the corrected limitations; independent re-review: agree. The bytecode caches present predate the recording. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]> * Register the li26 results, receipts and stack correction (hot-file commit) Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]> * Re-register after rebasing onto #329 (hot-file commit) Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]> --------- Co-authored-by: Scout <scout@local> Co-authored-by: Claude Opus 5.5 (1M context) <[email protected]>
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
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.
Follow-up to #325. On 2026-09-26 the user decided that the Tavily key lives only in the kernel user keyring, never in a file, and that workflows use it seamlessly. This PR hardens the guard around that key, adds a wrapper for seamless use, and records what the Research endpoint actually returned.
Changes
scripts/hooks/secret_path_guard.py(the hash inadoption/hooks/claude/SHA256SUMSis updated):kernel_keyring.py exec <name> <VAR> -- <cmd>andtvly-keyring <args>in any position or launcher, and applies every existing rule to the wrapped command.keyctl print|pipe|read|dh_compute|list|rlist, and inlineKEYCTL_READor raw syscall reads.TAVILY_API_KEYto the secret names, blockstvly authwithout--json(0.1.8 prints part of the key there), and treats~/.tavily/config.jsonas a native store.adoption/tools/tvly-keyring: a POSIX sh wrapper that runs the realtvlywith the key injected into that child only. It never reads the key, exits 2 when the key is absent, and refuses recursion. It is documented inadoption/tools/README.mdandrecipes/tavily.md.adoption/credential-inventory.json: atavilyrow with store kindkernel_keyring(memory only).scripts/credential_status.pyreports kernel-keyring rows asuncheckedand never queries the keyring.docs/secret-storage.mdgets a table row and a "Guard coverage (2026-09-26)" section.tavily-research-qualification-20260926(native CLI E2E), linked fromtavily-cliinmanifests/stack.json:--model proreports, allcompleted;Review record (bounded loop)
/env, fixed in 74508203; a hash quoted wrong in the handoff)stdbuf -o0before an interpreter,keyctl rliston a key,tvly auth > --json), each reproduced and then fixed in 5cc15864"$VAR"xmerged by quote removal, a key serial read as a descriptor,{fd}>--json. Plus one false block (a queryexportfollowed by--max-results). Each was reproduced and fixed in 431946fb with regression casesTests:
tests.test_secret_path_guard,test_kernel_keyring,test_tvly_keyring,test_credential_status,test_credential_toolsandtest_install_claude_profileran 133 tests, OK (2 skipped).validate.pypassed.Known pass-throughs, recorded in
EXPECTED_PASS_THROUGHand indocs/secret-storage.md:script -qc);The guard is defense in depth, not a sandbox.
Evidence classes
tvlythat reports only presence and length. One real E2E,tvly-keyring auth --json, printed only its authenticated statusSOTA sources
commands/auth.pyfor the key print; theresearch run --model proCLI), tavily-ai/tavily-cli.act_keyctl_listandact_keyctl_rlist).{varname}descriptor form).add_key(2)andkeyctl(2)(x86_64 syscalls 248 and 250), whichscripts/kernel_keyring.pywraps.After merge (coordinator)
python3 tools/adoption/install_claude_profile.py --only guardon this host, so the host copy matches the repo.tvly-keyringandkernel_keyring.pyinto~/.local/share/codex-ecosystem/bin, followingadoption/tools/README.md.🤖 Generated with Claude Code