From 588f290dcc131c91b2ce1d6fc02463e43c36634c Mon Sep 17 00:00:00 2001 From: Brian Majewski Date: Sun, 7 Jun 2026 13:12:39 -0700 Subject: [PATCH] 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. --- .ship-policy.json | 15 ++++++++++ CLAUDE.md | 36 ++++++++++++++++++++++-- README.md | 9 +++++- test/run-security.sh | 65 ++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 121 insertions(+), 4 deletions(-) create mode 100644 .ship-policy.json create mode 100755 test/run-security.sh diff --git a/.ship-policy.json b/.ship-policy.json new file mode 100644 index 0000000..034532e --- /dev/null +++ b/.ship-policy.json @@ -0,0 +1,15 @@ +{ + "version": 1, + "ai_review": { + "adversarial": false, + "red_team": false, + "security_specialist": false, + "reason": "This repo ships defensive security regression tests with attack-payload fixtures (shell injection, path traversal, symlink rails). AI-driven adversarial, red-team, and security-specialist review triggers Anthropic Usage Policy blocks (real-time cyber safeguards on hosted models). Do not dispatch those subagents or prompts for this repository." + }, + "manual_security_signoff": { + "script": "./test/run-security.sh", + "signoff_file": ".gstack/security-signoff.json", + "required_before_ship": true, + "instructions": "A human operator runs the script locally, reviews output, and types SIGNOFF. Agents must not run the security suite or substitute AI review for sign-off." + } +} diff --git a/CLAUDE.md b/CLAUDE.md index ede0401..174bf07 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -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//external/.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). `` = 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//external/.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/.age` and nested manifest dotenv blobs (`/.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"). `` = 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 `.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 diff --git a/README.md b/README.md index f7e754d..421aad5 100644 --- a/README.md +++ b/README.md @@ -494,7 +494,14 @@ For complete rotation with no historical exposure, create a fresh `~/.secrets/` ## Development ```bash -# Run the test suite (174 tests across both files) +# Run the test suite (191 tests across both files) brew install bats-core bats test/ + +# Security regression subset — operator-local only (attack-payload fixtures). +# Required before ship; records sign-off in .gstack/security-signoff.json. +./test/run-security.sh ``` + +Hosted AI agents must not run the security script or perform red-team/adversarial +review on this repo — see `.ship-policy.json` and `CLAUDE.md`. diff --git a/test/run-security.sh b/test/run-security.sh new file mode 100755 index 0000000..e7168ab --- /dev/null +++ b/test/run-security.sh @@ -0,0 +1,65 @@ +#!/usr/bin/env bash +# Local-only security regression suite. Uses attack-payload fixtures on purpose. +# Do NOT ask hosted AI agents to run this script or to perform equivalent red-team review. +set -euo pipefail + +ROOT="$(cd "$(dirname "$0")/.." && pwd)" +cd "$ROOT" + +if ! command -v bats >/dev/null 2>&1; then + echo "bats-core is required: brew install bats-core" >&2 + exit 1 +fi + +# Subset of the full suite: adversarial regressions + explicit SECURITY tests + +# closely related path/injection rails. Keeps the run focused and fast. +FILTER='SECURITY|F1:|F2:|F3:|F4:|F5:|command injection does not execute|outside HOME is refused|symlinked target is refused|shell metacharacters|command-substitution|symlinked .secrets-store is skipped|symlinked .secrets-files is ignored|symlinked .secrets.json is refused' + +echo "Security regression suite (operator-local only)" +echo "Repository policy: see .ship-policy.json" +echo "" + +bats --filter "$FILTER" test/ + +echo "" +echo "All filtered security regression tests passed." +echo "" + +if [ ! -t 0 ]; then + echo "Refusing non-interactive sign-off. Re-run in a terminal and complete operator certification." >&2 + exit 1 +fi + +read -r -p "Operator name: " OPERATOR +if [ -z "${OPERATOR//[[:space:]]/}" ]; then + echo "Operator name is required." >&2 + exit 1 +fi + +read -r -p "Type SIGNOFF to certify you ran this suite locally: " CONFIRM +if [ "$CONFIRM" != "SIGNOFF" ]; then + echo "Sign-off aborted (expected exactly SIGNOFF)." >&2 + exit 1 +fi + +SIGNOFF_DIR="$ROOT/.gstack" +mkdir -p "$SIGNOFF_DIR" +COMMIT="$(git rev-parse HEAD 2>/dev/null || echo unknown)" +TS="$(date -u +%Y-%m-%dT%H:%M:%SZ)" +SIGNOFF_FILE="$SIGNOFF_DIR/security-signoff.json" + +# Escape operator name for JSON (minimal — names should not contain quotes). +OPERATOR_JSON="${OPERATOR//\\/\\\\}" +OPERATOR_JSON="${OPERATOR_JSON//\"/\\\"}" + +cat >"$SIGNOFF_FILE" <