Repository navigation
feat(minimal): store, list and remove secrets with min secret (T26) - #2522
norrietaylor merged 18 commits into
Conversation
Each item is a generic password under dev.minimal.secret whose access control 8000 trusts only the proxy binary (BEP-051). Hosts other than macOS refuse until a native store lands.
set reads the value from the terminal or standard input and prints only the identifier. list prints store, rule, consent and proxy access, and no value (BEP-050, BEP-052, BEP-053).
|
Important Review skippedBot user detected. To trigger a single review, invoke the
 |
|
Approved: every review thread resolved at @norrietaylor resolved the last thread, which releases the approval this review had withheld. Nothing to do. |
There was a problem hiding this comment.
arch-review: CONDITIONS · 2 conditions · the full ruling
The command grammar, the value sources, the list row and rm conform to the min secret reference in architecture.md line 513. The store provider is in bep, as Gatehouse §6.10 line 844 says. The real Keychain path never ran. So nobody showed that bep reads the item with no prompt, and BEP-051 depends on that (line 513). Do the macOS run in C1 and record it, or fix bep::store::host::set if bep prompts. File the resolver-drift issue in C2 and add it to the PR body as a residual.
Ruled at a0bd91c against gominimal/arch@109541a48c31 by claude-opus-5-5.
An update in place with add-generic-password -U keeps the old item's access control. So set now removes the old item, adds the new one without -U, and reads the item back. The read-back decides the result, so a warning on stderr after a good write no longer fails the command.
There was a problem hiding this comment.
arch-review: CONDITIONS · 3 conditions · the full ruling
The access control can name the wrong proxy. min looks for bep beside min, but minvmd starts the bep beside itself, and min starts minvmd from PATH. Line 513 of architecture.md says the ACL grants the BEP process identity a prompt-free read. C1 makes min use the same anchor as minvmd. The Keychain path never ran on a Mac, so the BEP-051 proof is open. C2 adds that test and the recorded run. C3 makes the duplicate resolver an owned issue.
Ruled at 8ae45ff against gominimal/arch@109541a48c31 by claude-opus-5-5.
|
Round 1 of 10 on the runtime's review findings, pushed as
Verify Review threads the coder judged:
Notes from the agent: Revise round 1. The changed host::set code is in the |
|
/revise Round 2 of #2522, on the arch ruling at 8ae45ff and the review findings that stay open. No round is in flight.
One or two commits, Conventional Commits subjects, body lines at most 100 characters. Generated by Claude Code |
min secret set named `bep` beside the `min` program in the item's access control. The running minvmd starts `bep` beside its own executable, and autospawn starts the first `minvmd` on PATH. So the access control now names `bep` beside that `minvmd`, or the MINVMD_BEP_BIN path when it is set and not empty. With no `minvmd` on PATH, set refuses and names both remedies.
set on a held id deleted the old keychain item before it added the new one, so a refused add destroyed a working secret. The new value now goes first into an item under dev.minimal.secret.staging and is read back. Only then is the old item deleted. If the last add still fails, the error says that the old value is removed. The proxy path doc now names the minvmd anchor. Adds the macOS ignored test host_store_set_list_rm_round_trip for a real keychain.
There was a problem hiding this comment.
arch-review: CONDITIONS · 2 conditions · the full ruling
The verb, the refusals and the list row conform to architecture.md line 513. The compare fixes the proxy anchor and the replace order. Two items stay open. C1: the macOS Keychain path never ran, and line 513 makes the keychain ACL the only proof that the proxy reads without a prompt. Run the ignored round-trip test with a copy of bep as the proxy, and paste the output under ## Proof. C2: the proxy-path rule has a second copy in minvmd and no issue owns it. File that issue, and name it in both doc comments and in the PR body.
Ruled at 7d0e4b2 against gominimal/arch@109541a48c31 by claude-opus-5-5.
|
Round 2 of 10 on a /revise note, pushed as
Verify Review threads the coder judged:
Notes from the agent: Commits f03dc98 (min side: proxy anchor) and 7d0e4b2 (bep side: staged replace, proxy_binary_path doc, macOS round-trip test). Directive item 1: resolve_proxy in crates/minimal/src/cmd/secret.rs uses MINVMD_BEP_BIN when set and not empty, else |
|
/revise Round 3 of #2522, on the arch ruling at 7d0e4b2 and the review findings that stay open. No round is in flight. The epic owner filed #2527 for the proxy-binary resolver's single owner (arch C2), and will run the macOS Keychain leg (arch C1) on the head this round produces.
One or two commits, Conventional Commits subjects, body lines at most 100 characters. Generated by Claude Code |
resolve_proxy picks the program that the keychain item trusts. An empty or relative PATH entry let a `minvmd` in the current directory anchor it, and a relative MINVMD_BEP_BIN went into the access control as is. Now find_on_path skips every entry that is not absolute, a relative MINVMD_BEP_BIN is refused, and a proxy that exists comes back canonical. run_set refuses a relative path. The doc comment names issue 2527, the one owner for the rule.
/usr/bin/true is Apple-signed, so it cannot show the access control that the proxy gets. The macOS round trip now copies the built `bep` into a temporary directory and names that copy. proxy_binary_path names issue 2527.
There was a problem hiding this comment.
arch-review: PASS_WITH_CONDITIONS · 1 condition · the full ruling
The compare closes the conditions on 7d0e4b26. The trusted proxy path is now absolute and canonical. The Mac round trip trusts a copy of bep, and issue 2527 owns the duplicated path rule. The code meets architecture.md line 513 for set, list and rm. Nobody ran the BEP-051 access control on a real Keychain. C1 runs the ignored round trip on a Mac and records the output in the PR body. No code change is necessary.
Ruled at 5d0e7d0 against gominimal/arch@109541a48c31 by claude-opus-5-5.
|
Round 3 of 10 on a /revise note, pushed as
Verify Review threads the coder judged:
Notes from the agent: Round 3. Commit 1971099 (fix(minimal)) hardens resolve_proxy and find_on_path and names issue 2527 in the resolve_proxy doc comment. Commit 5d0e7d0 (test(bep)) makes host_store_set_list_rm_round_trip name a copy of the built bep in a temp dir, and names issue 2527 in the proxy_binary_path doc comment. The test takes option_env!("CARGO_BIN_EXE_bep") and falls back to the bep beside target//deps. On a Mac, build bep first (cargo build -p bep), then run: cargo nextest run -p bep host_store_set_list_rm_round_trip --run-ignored only. I type-checked that test on Linux by removing its macOS cfg for one clippy run, then put the cfg back. It did not run here, because the box is Linux. For the PR body, which the runner writes: add a ## Residuals section with #2527 (one owner for the proxy-binary rule, arch C2), and state that the BEP-051 leg is not_run until the owner pastes the Mac output under ## Proof. I do not know the number of the BEP-033 task issue that proves the prompt-free read, so the person who edits the body must add it. A relative MINVMD_BEP_BIN has a new refusal: "min secret set did not read the value: MINVMD_BEP_BIN is the relative path . Set MINVMD_BEP_BIN to the absolute path of the |
|
/revise Round 4 of #2522, on the three review findings left open at 5d0e7d0. No round is in flight. Arch C1 (the real-Keychain run) is the epic owner's and runs on the head this round produces; do not change
One or two commits, Conventional Commits subjects, body lines at most 100 characters. Generated by Claude Code |
…-and-remove-secrets-with-min-secret-un
add_line checks the id with valid_id, so no caller of keychain_add_line can put an option such as -A on the line that security -i parses. set_item reports a staging cleanup that the keychain refuses. A replace that succeeded returns StagingLeft, which says the new value is stored and names the security delete-generic-password command that removes the copy. SecretStore gains check_available, so set can learn before it reads a value that the host has no store.
run_set asks the store whether this host can use it before it prompts or reads standard input, so nobody types a secret that is then refused. The new test secret_set_checks_the_store_before_it_reads keeps that order. check_id uses the store's InvalidId refusal, so the text has one owner. A StagingLeft error prints without "did not store", because the value is stored.
check_proxy_binary now walks every directory from the proxy's parent up to the root. Each one must be owned by the current user or root, and must not be writable by group or others. A sticky directory above the parent, such as /tmp, can be writable. add_checked now deletes the new item when the read-back after a successful add fails, so a refused set leaves no unchecked item behind.
There was a problem hiding this comment.
arch-review: PASS_WITH_CONDITIONS · 1 condition · the full ruling
The new commit makes the proxy-only access in architecture.md line 513 stronger. The proxy check now covers every directory up to the root. A failed read-back now deletes the new item, so the set fails closed. One condition from the last ruling stays open. C1: run the macOS Keychain proof, or file it as an issue and link it. Residual #2527 stays open for the duplicated proxy resolver.
Ruled at c8bcc05 against gominimal/arch@109541a48c31 by claude-opus-5-5.
|
Round 6 of 10 on a /revise note, pushed as
Verify Review threads the coder judged:
Notes from the agent: Round 6 is one commit, c8bcc05. The ancestor walk keeps the parent directory strict. A directory above the parent can be writable when its sticky bit is set, as /tmp is. In a sticky directory, only the owner of an entry can rename or remove it, and the entry below that directory also must pass the owner rule. Without this exemption, every proxy under a temporary directory fails the check. This box does not run as root, so the test for a parent owned by another user pins the rule function directly. That test also runs the full directory walk with chown, but only when it runs as root. Also ran |
|
/revise Round 7 of #2522, on the two review findings opened at c8bcc05. No round is in flight. Arch C1 (the real-Keychain run, threads 4369434893 and 4369550479) is the epic owner's; leave those threads open and do not change
One commit, Conventional Commits subject, body lines at most 100 characters. Generated by Claude Code |
…-and-remove-secrets-with-min-secret-un
min secret list no longer fails when no minvmd is on PATH. It shows the items, prints proxy access as unknown, and adds one line that says why. min secret set with --no-input and a terminal on standard input refuses at once, and names piped standard input as the way to give the value. Each refusal of a name or a store now names the command that printed it.
There was a problem hiding this comment.
arch-review: PASS_WITH_CONDITIONS · 1 condition · the full ruling
The compare conforms. With --no-input, min secret set now refuses at once on a terminal, as principle 3 (line 329) requires. min secret list now shows proxy access as unknown when it cannot resolve the proxy path, and it never shows a false yes. That agrees with line 513. One item stays open from the baseline: nobody ran the real Keychain leg on a Mac. C1 tells you how to close it. The resolver duplication stays filed as #2527.
Ruled at 48471a3 against gominimal/arch@109541a48c31 by claude-opus-5-5.
|
Round 7 of 10 on a /revise note, pushed as
Verify Review threads the coder judged:
Notes from the agent: One commit, 48471a3. resolve_proxy errors no longer name a command. |
|
/revise Round 8 of #2522, on the three review findings opened at 48471a3. No round is in flight. Arch C1 (the real-Keychain run, threads 4369434893, 4369550479 and 4369645885) is the epic owner's; leave those threads open and do not change
One commit, Conventional Commits subject, body lines at most 100 characters. Generated by Claude Code |
min secret set checks the native store before it resolves the proxy path. A host with no store now gets the store refusal first. The minvmd found on PATH resolves as the daemon's current_exe() does: links resolved on Linux, the path as is on macOS. parse_dump merges the copies of one identifier from several keychains. The merged item has proxy access when any copy grants it.
There was a problem hiding this comment.
arch-review: CONDITIONS · 1 condition · the full ruling
Two of the three fixes conform: the store check now runs first, and the symlinked minvmd resolves to the bep that the daemon starts. The duplicate merge in parse_dump_service fails open. An identifier is readable when any keychain copy grants the proxy access. Line 513 says readable means the item's ACL carries the entry for the proxy. The proxy reads one copy, and a copy without the entry prompts. add_checked also uses this merge, so an older copy in the system Keychain can confirm a set that failed. Make the change in C1: merge the copies with AND.
Ruled at e670990 against gominimal/arch@109541a48c31 by claude-opus-5-5.
|
Round 8 of 10 on a /revise note, pushed as
Verify Review threads the coder judged:
Notes from the agent: Round 8. One commit, e670990. (1) cmd_secret_set calls the new set_proxy, which runs check_available before resolve_proxy. secret_set_checks_the_store_before_it_reads pins the order. (2) Finding 4369647106: I did not canonicalize on every platform. On Linux, current_exe() resolves links, so exe_path canonicalizes the minvmd found on PATH. On macOS, Rust's current_exe() is _NSGetExecutablePath, and that function gives the path that started the program. That path can be the link itself. If min canonicalized there, the access control would name a bep that the daemon does not start, and macOS is the one host with a store. So exe_path keeps the macOS path as is. The macOS behaviour comes from the documented contract of current_exe(). Nobody ran it on a Mac. The link test in secret_set_refuses_when_proxy_binary_absent expects each platform's result. (3) parse_dump_service merges the copies of an identifier with an OR of proxy_access. secret_item_acl_admits_proxy_only now has claude in two keychains, in both orders. Item 4 of the note asks me to reply on and resolve threads. My instructions do not let me resolve threads, so the verdicts below are the replies. Arch C1 is left open, as the note says. host_store_set_list_rm_round_trip is unchanged and was not run (it needs a real Keychain). I did not run just clippy-strict. |
|
/revise Round 9 of #2522, on the arch ruling at e670990 and the two review findings opened there. No round is in flight. Arch C1's real-Keychain run (threads 4369434893, 4369550479 and 4369645885) is the epic owner's; leave those threads open and do not change
One commit, Conventional Commits subject, body lines at most 100 characters. Generated by Claude Code |
…-and-remove-secrets-with-min-secret-un
parse_dump now merges duplicate identifiers with AND. An identifier is readable only when every copy's access control carries the proxy's entry, so the read-back in add_checked no longer accepts a new copy because an older one grants access. min secret set reads standard input as bytes into a Zeroizing buffer of 64 KiB and a line break. A value that is not UTF-8 is stored as is. A longer value is refused with a message that names the limit.
There was a problem hiding this comment.
minimal-review — approval withheld
Verdict COMMENT — APPROVE withheld: 8 unresolved threads — minimal-arch[bot] crates/bep/src/store.rs:1184, minimal-arch[bot] crates/bep/src/store.rs:432, minimal-arch[bot] crates/bep/src/store.rs:1184, minimal-arch[bot] crates/bep/src/store.rs, minimal-arch[bot] crates/bep/src/store.rs:583, …. No findings.
Panel: 3 of 3 personas ran (correctness, security, supply-chain).
There was a problem hiding this comment.
arch-review: PASS_WITH_CONDITIONS · 1 condition · the full ruling
The fix commit meets both conditions from the last ruling. min secret list now reports a prompt-free read only when every copy of the item carries the proxy's entry, as architecture.md line 513 says. min secret set now reads stdin as bytes, up to a limit. One condition is still open from the earlier rulings. Nobody ran the macOS Keychain path on a Mac, and line 513 makes the Keychain the native store there. Do C1: run that path on a Mac and record the result, or open an issue for the run and list it under Residuals.
Ruled at eb953d7 against gominimal/arch@109541a48c31 by claude-opus-5-5.
|
Round 9 of 10 on a /revise note, pushed as
Verify Review threads the coder judged:
Notes from the agent: Round 9, commit eb953d7. Item 1: parse_dump_service now merges duplicate identifiers with AND, and secret_item_acl_admits_proxy_only now expects proxy_access: false for claude in both orders. Item 2: no code change. The directive and my instructions ask me to leave thread 4369746890 open, so it stays open. The note on the thread gives the reason. Item 3: standard input is now read as bytes into a Zeroizing buffer of MAX_VALUE_BYTES (64 KiB) plus a line break. The buffer is allocated at full size before the read, so it never reallocates. A full buffer followed by one more byte is refused. Two new tests pin this: secret_set_stores_binary_stdin_value and secret_set_refuses_value_over_size_limit. The terminal path now moves the typed String into bytes without a copy. Directive item 4 asks me to resolve the fixed threads. My instructions forbid it, so I resolved no thread. I also did not change host_store_set_list_rm_round_trip, and I did not run the real-Keychain test, as the note asks. No Mac is available here. just clippy-strict was not run. |
There was a problem hiding this comment.
minimal-review — approved
Verdict APPROVE — every review thread is resolved; @norrietaylor closed the last one. The review of eb953d7e6be8 withheld this approval on the threads open then. Posted as a comment: the App cannot approve its own pull request; the check and the label carry it.
There was a problem hiding this comment.
Babysitter approval at eb953d7: every merge gate passes at this head (arch's conditions are the macOS legs, filed as residual #2562 by the epic owner's decision; zero open threads; every lane aggregator green). Squash title feat(minimal): store, list and remove secrets with min secret (T26) and the squashed message pass commitlint. Queued for squash merge.
Generated by Claude Code
T26 — Store, list and remove secrets with min secret under proxy-only access
slice: S3
kind: task
covers: BEP-050, BEP-051, BEP-052, BEP-053
size: L
est: 380 code, 480 tests
executor: agent
difficulty: needs care
model: strongest
diagnostics:
min secret set <id> <value>is refused with "min secret set reads the value from the terminal or standard input, never from an argument or an environment flag." on stderr, pinned by secret_set_refuses_value_argument_and_env_flag;min secret listshows store, authorities, injection form, consent rule and whether the proxy's access entry is present, never a valueobservability: one info log line per set and remove naming the store and the identifier, never the value
layers:
- cli:
- crates/minimal/src/cli.rs:
min secret set | list | rm [--store <s>]- crates/minimal/src/cmd/mod.rs: dispatch and module line
- crates/minimal/src/cmd/secret.rs (new):
set,list,rmper BEP-050, BEP-052 and BEP-053test: cargo nextest run -p minimal secret_set_stores_in_keychain_and_prints_id_only
test: cargo nextest run -p minimal secret_set_refuses_value_argument_and_env_flag
test: cargo nextest run -p minimal secret_set_without_tty_or_stdin_fails_immediately
test: cargo nextest run -p minimal secret_set_refuses_gatehouse_store
test: cargo nextest run -p minimal secret_list_shows_metadata_without_values
test: cargo nextest run -p minimal secret_rm_removes_item
- store:
- crates/bep/src/store.rs (new): Keychain store items with the BEP-051 access control
- crates/bep/src/lib.rs: module line
test: cargo nextest run -p minimal secret_item_acl_admits_proxy_only
test: cargo nextest run -p minimal secret_set_refuses_when_proxy_binary_absent
produces:
- crates/bep/src/store.rs
consumes: none
proof:
min secret set claudeprints only the identifier, andmin secret listshows its metadata and no value.depends on:
- blocked by #2280
Proof
min secret set claudereads the value from the terminal (no echo) or from piped standard input, stores it in the macOS Keychain under service dev.minimal.secret with the bep proxy as the only trusted application, and prints onlyclaude.min secret listprints, for each secret, its store, the upstream and injection form of its [secret-store-rules] rule, the consent action ornone, and whether the proxy reads it without a prompt. It prints no value. On this Linux box the proof comes from unit tests against an in-memory store. Nobody ran the real Keychain path.Notes from the agent: The macOS Keychain backend in bep::store (
security -iwith add-generic-password -T -X , delete-generic-password, and a parser forsecurity dump-keychain -athat reads each item's access-control entries) is not run on a Mac. Only its command line and the dump parser are tested, from the minimal tests on Linux. Before the proof is final, runmin secret setandmin secret liston a Mac. Check that dump-keychain -a asks for no access, and that its output format matches parse_dump. On hosts other than macOS, HostStore refuses every operation: the spec says the Linux native store waits on the store roster. The proxy path is MINVMD_BEP_BIN orbepbesidemin. That mirrors resolve_bep_binary in crates/minvmd/src/cmd/run.rs, which is outside this task's layers, so the two copies can drift. A value given as an argument, or through the hidden --env/--from-env flags, is refused with the exact text the task pins. That text states the accepted sources but names no command to run. I kept it as written. Identifiers are limited to letters, digits, '.', '_' and '-' (at most 128 characters, not starting with '-'), so they need no quoting on the security command line.min secret rmof an absent name exits non-zero. A follow-on task needs this: the proxy's per-request read (BEP-033) must read service dev.minimal.secret with account . BEP-079 rule flags (--upstrea…[truncated]Tests
cargo nextest run -p minimal secret_set_stores_in_keychain_and_prints_id_onlycargo nextest run -p minimal secret_set_refuses_value_argument_and_env_flagcargo nextest run -p minimal secret_set_without_tty_or_stdin_fails_immediatelycargo nextest run -p minimal secret_set_refuses_gatehouse_storecargo nextest run -p minimal secret_list_shows_metadata_without_valuescargo nextest run -p minimal secret_rm_removes_itemcargo nextest run -p minimal secret_item_acl_admits_proxy_onlycargo nextest run -p minimal secret_set_refuses_when_proxy_binary_absentVerify
min task run agent-verifyexited 101 under rustc 1.98.1, which the repository does not pin (1.97.0 in rust-toolchain.toml); the PR is not a draft on that alone — CI, which honours the pin, is the gate.Residuals
min secret(bep::store::proxy_binary_path,resolve_proxy) andminvmd(resolve_bep_binary), so the Keychain ACL and the started proxy cannot drift.mainon the manual macOS lane of T82 — Run the Box Egress Proxy legs on the macOS lane #2500 (epic owner's decision, 2026-10-25; arch's conditions on the legs).Closes #2287
Generated by Claude Code