chore: security review policy + operator runner, doc updates (EGB-677 stage 1)

Add .ship-policy.json (opts out AI adversarial/red-team/security-specialist
review; requires local operator sign-off) and test/run-security.sh (the
operator-local security regression subset). Document the policy in CLAUDE.md
and README, fix stale test counts (manifest.bats 41->58, total 174->191), and
update the storage-recursion note to reflect rekey/list now walking the full
project tree.
This commit is contained in:
Brian Majewski 2026-06-07 13:12:39 -07:00
parent c6ea724ddb
commit 588f290dcc
4 changed files with 121 additions and 4 deletions

View file

@ -17,6 +17,7 @@ cd ~/my-project && ./secrets pull # Pull + decrypt .env* files
```bash
brew install bats-core
bats test/ # runs secrets.bats + manifest.bats
./test/run-security.sh # security regression subset + operator sign-off (see below)
```
**bash 3.2 assertion gotcha:** bats runs under system bash 3.2, where a
@ -24,6 +25,35 @@ failing `[[ ]]` mid-test does NOT fail the test (the ERR trap skips `[[`
compound commands). Every standalone `[[ ... ]]` assertion MUST end with
`|| false`. Single-bracket `[ ]` assertions are unaffected.
## Security review policy (mandatory for agents)
This repo contains **defensive** security regression tests with deliberate
attack-payload fixtures (shell injection strings, symlink escape cases, path
rails). Hosted models (including Opus) block AI-driven "think like an attacker"
/ red-team / security-specialist review of that material under Anthropic Usage
Policy (real-time cyber safeguards).
**Read `.ship-policy.json` before any `/ship`, `/review`, or `/autoplan` work.**
When that file is present (it always is in this repo):
1. **Never** dispatch red-team subagents, security specialist subagents, or
adversarial review prompts ("think like an attacker", "find security holes",
"chaos engineer", exploit hunting) against this codebase.
2. **Never** run `./test/run-security.sh` or filter bats to SECURITY/adversarial
tests on behalf of the user — that is operator-local work only.
3. **Before ship/PR:** ask the human operator to run `./test/run-security.sh`
locally and complete the SIGNOFF prompt. Record lives at
`.gstack/security-signoff.json` (gitignored). Proceed with ship only after
they confirm sign-off for the current `HEAD` (or note in the PR that sign-off
is pending).
4. Normal `./ship` steps may continue (tests via `bats test/`, eng review,
changelog, PR) — only the AI security/red-team/adversarial passes are opted out.
Gstack `/ship` reads the same policy via `gstack-ship-policy` (sources
`.ship-policy.json` at repo root): Step 9 blocks when manual sign-off is missing,
skips security specialist + red team, and Step 11 skips adversarial review.
## Architecture
Single bash script (`secrets`) with subcommands: init, push, pull, list, rm, rekey.
@ -44,7 +74,7 @@ secrets # CLI script (~2000 lines bash)
hooks/pre-commit # Pre-commit hook template
test/
secrets.bats # bats-core test suite (133 tests)
manifest.bats # EGB-677 .secrets.json manifest tests (41 tests)
manifest.bats # EGB-677 .secrets.json manifest tests (58 tests)
test_helper.bash # Shared setup/teardown
README.md # User-facing documentation
CLAUDE.md # This file
@ -74,7 +104,7 @@ The active store directory is picked by `resolve_store()` using these rules, hig
Key design decisions (all driven by /autoplan review):
- **Wire-in is at command scope** (`cmd_push`/`cmd_pull`), via `push_external_files` / `pull_external_files`, **not** inside `push_dir_to_project` / `pull_project_to_dir` (those loop per-workspace and `pull_project_to_dir` uses stdout as a data channel).
- **Storage:** blobs live in `$SECRETS_DIR/<project>/external/<slug>.gradle-properties.age`. The `external/` subdir keeps them out of the existing non-recursive `*.age` / `.*.age` globs (pull, list, rekey), so the old dotenv path can never decrypt a blob into cwd. `cmd_rekey` and `cmd_list` recurse into `external/` explicitly (rekey MUST, or the blob is orphaned after rotation = data loss). `<slug>` = manifest path token with non-`[A-Za-z0-9._-]` chars → `_`, plus a `cksum` suffix of the original path so paths that clean to the same string (`a/b` vs `a_b`) don't collide. Machine-independent (derived from the committed manifest token, not the expanded path).
- **Storage:** blobs live in `$SECRETS_DIR/<project>/external/<slug>.gradle-properties.age`. The `external/` subdir keeps them out of the legacy non-recursive `*.age` / `.*.age` globs the dotenv `pull` path uses, so a dotenv pull can never decrypt an external blob into cwd. `cmd_rekey` and `cmd_list` instead walk the **entire** project tree (`find -type f`), so they cover both `external/<slug>.age` and nested manifest dotenv blobs (`<project>/<relpath>.age`) — rekey MUST recurse, or any nested/external blob is orphaned under the old key after rotation = data loss (EGB-677 regression test: "rekey re-encrypts a nested manifest dotenv blob"). `<slug>` = manifest path token with non-`[A-Za-z0-9._-]` chars → `_`, plus a `cksum` suffix of the original path so paths that clean to the same string (`a/b` vs `a_b`) don't collide. Machine-independent (derived from the committed manifest token, not the expanded path).
- **Merge is pure bash, no `sed`/regex** (`merge_gradle_keys`): exact-string key comparison (avoids `beaconClerkPk` vs `beaconClerkPkTest` substring bug), value treated as opaque literal (survives `& \ /` in values). Updates a managed key in place at its first occurrence, collapses duplicates, appends new keys, preserves unrelated lines/comments/order. Continuation lines (trailing odd backslashes, tracked by `_trailing_bs_odd`) are never matched as keys. Atomic write: temp in the same dir → `chmod` to match (or `600` on create) → `mv`. Backs up to `<target>.secrets-bak` before each merge.
- **Properties separator parsing** (`_props_get`): key ends at the first `=`, `:`, or whitespace (after lstrip); handles `key=value`, `key = value`, `key:value`, `key value`; last definition wins.
- **Security:** the write target comes from a committed file, so `_validate_external_target_path` locks it down — basename must be `gradle.properties`, must resolve inside `$HOME` (deepest-existing-ancestor resolved, symlink target/parent refused, `..` rejected). This blocks a malicious manifest from appending decrypted keys to `~/.gitconfig`/`~/.bashrc`. `_parse_secrets_files_manifest` rejects shell metacharacters/control chars in path and keys (path allows `[A-Za-z0-9/._~-]` only; keys allow `[A-Za-z0-9._-]` + space), mirrors the `.secrets-store` posture (no shell expansion, symlinked manifest skipped).
@ -119,7 +149,7 @@ Key routing rules:
- QA/testing site behavior → invoke /qa or /qa-only
- Code review/diff check → invoke /review
- Visual polish → invoke /design-review
- Ship/deploy/PR → invoke /ship or /land-and-deploy
- Ship/deploy/PR → invoke /ship or /land-and-deploy (after reading `.ship-policy.json`; no AI adversarial/red-team/security-specialist review in this repo)
- Save progress → invoke /context-save
- Resume context → invoke /context-restore
- Author a backlog-ready spec/issue → invoke /spec