From b55ac0e6ef44387bcf22eeaa4b5167d35f08faca Mon Sep 17 00:00:00 2001 From: Stat-Wise Date: Sat, 25 Jul 2026 09:58:35 -0600 Subject: [PATCH 1/2] fix(guard): surface CRAFT_GUARD_ALLOW_DEV_EDIT in guard-bypass-marker confirms (#309) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "Claude writes .claude/allow-once" one-shot flow documented in REFCARD-BRANCH-GUARD.md and branch-guard-smart-mode.md is circular for the one case where the approved action IS creating/editing that marker itself — every tool path (Write/Edit/Bash) intercepts it with its own [CONFIRM], and the one thing that actually resolves it non-interactively (issue #281's CRAFT_GUARD_ALLOW_DEV_EDIT env var) appeared in neither reference doc nor any runtime suggestion. Adds the env-var hint to the three guard-bypass-marker confirms only (edit_guard_bypass/write_guard_bypass/bash_guard_bypass) — not to write_new_code, where allow-once genuinely works as documented and the hint would be noise. Reconciles both reference docs with a caveat pointing to skills/dev/git/SKILL.md Operation 10, which already had this right. --- docs/guide/branch-guard-smart-mode.md | 7 ++++ docs/reference/REFCARD-BRANCH-GUARD.md | 6 +++ scripts/branch-guard.sh | 9 +++-- tests/test_branch_guard.sh | 55 ++++++++++++++++++++++++++ 4 files changed, 74 insertions(+), 3 deletions(-) diff --git a/docs/guide/branch-guard-smart-mode.md b/docs/guide/branch-guard-smart-mode.md index 61eab03ad..64c63498a 100644 --- a/docs/guide/branch-guard-smart-mode.md +++ b/docs/guide/branch-guard-smart-mode.md @@ -217,6 +217,13 @@ When Claude shows a `[CONFIRM]` prompt and the user approves, Claude creates a o 5. Guard sees marker → consumes it (deletes file) → allows action 6. Next tool call has no marker → normal protection resumes +**This flow does not apply when the approved action *is* creating/editing the marker file +itself** (`.claude/allow-once` or `.claude/allow-dev-edit`) — that write is its own `[CONFIRM]` +gate (`write_guard_bypass`/`edit_guard_bypass`/`bash_guard_bypass`), since self-approving a bypass +can never be a silent allow. That self-referential case has no "Claude writes the marker" route at +all; a non-interactive session needs `CRAFT_GUARD_ALLOW_DEV_EDIT=1` pre-set out-of-band by the +user first (issue #281 — see `skills/dev/git/SKILL.md` Operation 10). + ### One-Shot vs Session Bypass | Mechanism | Scope | Duration | Use Case | diff --git a/docs/reference/REFCARD-BRANCH-GUARD.md b/docs/reference/REFCARD-BRANCH-GUARD.md index 9059cd95b..45d93ff8a 100644 --- a/docs/reference/REFCARD-BRANCH-GUARD.md +++ b/docs/reference/REFCARD-BRANCH-GUARD.md @@ -134,6 +134,12 @@ Guard blocks → You approve → Claude writes .claude/allow-once No commands needed — just approve the prompt. +**Exception — the marker file itself:** creating or editing `.claude/allow-once`/`.claude/allow-dev-edit` +is a `[CONFIRM]` action in its own right (self-approving a bypass is never a silent allow), so the +flow above can't resolve it — writing the marker to unblock the marker write is circular. In a +non-interactive session this needs `CRAFT_GUARD_ALLOW_DEV_EDIT=1` pre-set out-of-band (shell +profile / Claude env) by the user, per issue #281 — see `skills/dev/git/SKILL.md` Operation 10. + ### Session Bypass (Bulk Operations) ```bash diff --git a/scripts/branch-guard.sh b/scripts/branch-guard.sh index 45e7bc83f..0799e478e 100755 --- a/scripts/branch-guard.sh +++ b/scripts/branch-guard.sh @@ -825,7 +825,8 @@ if [[ "$PROTECTION" == "smart" ]]; then _confirm "edit_guard_bypass" \ "Edit guard-bypass marker on ${BRANCH}: $(basename "$FILE_PATH")" \ "This file self-approves a bypass of branch-guard's own protection — never editable silently" \ - "ask \"unprotect\" — the sanctioned way to request this bypass (dev/git skill)" + "ask \"unprotect\" — the sanctioned way to request this bypass (dev/git skill)" \ + "Non-interactive: user pre-sets CRAFT_GUARD_ALLOW_DEV_EDIT=1 out-of-band (issue #281)" ;; esac # Editing existing files is always allowed on dev (LOW) @@ -864,7 +865,8 @@ if [[ "$PROTECTION" == "smart" ]]; then _confirm "write_guard_bypass" \ "Write guard-bypass marker on ${BRANCH}: $(basename "$FILE_PATH")" \ "Creating this file self-approves a bypass of branch-guard's own protection — must be a deliberate, confirmed action, never a silent allow" \ - "ask \"unprotect\" — the sanctioned way to request this bypass (dev/git skill)" + "ask \"unprotect\" — the sanctioned way to request this bypass (dev/git skill)" \ + "Non-interactive: user pre-sets CRAFT_GUARD_ALLOW_DEV_EDIT=1 out-of-band (issue #281)" ;; esac @@ -1076,7 +1078,8 @@ if [[ "$PROTECTION" == "smart" ]]; then _confirm "bash_guard_bypass" \ "Bash creates guard-bypass marker on ${BRANCH}: ${BASH_BASENAME}" \ "Creating this file via shell self-approves a bypass of branch-guard's own protection" \ - "ask \"unprotect\" — the sanctioned way to request this bypass (dev/git skill)" + "ask \"unprotect\" — the sanctioned way to request this bypass (dev/git skill)" \ + "Non-interactive: user pre-sets CRAFT_GUARD_ALLOW_DEV_EDIT=1 out-of-band (issue #281)" ;; esac diff --git a/tests/test_branch_guard.sh b/tests/test_branch_guard.sh index 579afe481..151ae07af 100755 --- a/tests/test_branch_guard.sh +++ b/tests/test_branch_guard.sh @@ -1837,6 +1837,61 @@ run_test \ "$(json_bash "touch .claude/allow-dev-edit" "$REPO_DEV_EDIT")" \ "$REPO_DEV_EDIT" +# All three guard-bypass-marker confirms must surface the CRAFT_GUARD_ALLOW_DEV_EDIT +# escape hatch (issue #309) — the "Claude writes .claude/allow-once" flow is +# circular for this self-referential action, so the env var is the only +# non-interactive route and must not be discoverable-nowhere. Each check uses +# a FRESH repo/session (not REPO_DEV_EDIT, already 1 encounter deep on these +# action_types above) — suggestions only render at "full" (1st-encounter) +# verbosity; a repeat encounter renders the brief box, which omits them, and +# would false-fail this check for the wrong reason. +REPO_MSG_EDIT=$(init_repo); switch_branch "$REPO_MSG_EDIT" "dev" +run_test_with_stderr \ + "test_edit_guard_bypass_message_mentions_env_var" \ + 2 \ + "$(json_edit "$REPO_MSG_EDIT/.claude/allow-dev-edit" "$REPO_MSG_EDIT")" \ + "$REPO_MSG_EDIT" \ + "CRAFT_GUARD_ALLOW_DEV_EDIT" + +REPO_MSG_WRITE=$(init_repo); switch_branch "$REPO_MSG_WRITE" "dev" +run_test_with_stderr \ + "test_write_guard_bypass_message_mentions_env_var" \ + 2 \ + "$(json_write "$REPO_MSG_WRITE/.claude/allow-dev-edit" "$REPO_MSG_WRITE")" \ + "$REPO_MSG_WRITE" \ + "CRAFT_GUARD_ALLOW_DEV_EDIT" + +REPO_MSG_BASH=$(init_repo); switch_branch "$REPO_MSG_BASH" "dev" +run_test_with_stderr \ + "test_bash_touch_guard_bypass_message_mentions_env_var" \ + 2 \ + "$(json_bash "touch .claude/allow-dev-edit" "$REPO_MSG_BASH")" \ + "$REPO_MSG_BASH" \ + "CRAFT_GUARD_ALLOW_DEV_EDIT" + +# The env-var hint must NOT leak into the unrelated write_new_code confirm — +# that action isn't self-referential, and allow-once genuinely resolves it as +# documented; mentioning the marker-bypass env var there would recreate the +# exact confusion issue #309 reported (a working non-interactive route buried +# among suggestions that don't apply to it). +REPO_MSG_NEWCODE=$(init_repo); switch_branch "$REPO_MSG_NEWCODE" "dev" +run_test_with_stderr \ + "test_write_new_code_message_does_not_mention_env_var" \ + 2 \ + "$(json_write "$REPO_MSG_NEWCODE/src/unrelated_new.py" "$REPO_MSG_NEWCODE")" \ + "$REPO_MSG_NEWCODE" \ + "New code files" +NEW_CODE_STDERR=$(echo "$(json_write "$REPO_MSG_NEWCODE/src/unrelated_new2.py" "$REPO_MSG_NEWCODE")" | (cd "$REPO_MSG_NEWCODE" && bash "$HOOK_SCRIPT") 2>&1 >/dev/null) || true +TOTAL=$((TOTAL + 1)) +if ! echo "$NEW_CODE_STDERR" | grep -q "CRAFT_GUARD_ALLOW_DEV_EDIT"; then + PASS=$((PASS + 1)) + echo -e " ${T_GREEN}PASS${T_NC} test_write_new_code_message_excludes_env_var_hint" +else + FAIL=$((FAIL + 1)) + FAILED_NAMES+=("test_write_new_code_message_excludes_env_var_hint") + echo -e " ${T_RED}FAIL${T_NC} test_write_new_code_message_excludes_env_var_hint ${T_BOLD}(env var hint leaked into unrelated confirm)${T_NC}" +fi + # With the env var set: all three call sites exit 0 without a [CONFIRM]. # run_test has no env-injection param, so these three small wrappers pass # CRAFT_GUARD_ALLOW_DEV_EDIT=1 through to the hook invocation directly. From cd0aa1af1f6814a81850efc0b4a831a93a7a4215 Mon Sep 17 00:00:00 2001 From: Stat-Wise Date: Sat, 25 Jul 2026 10:09:57 -0600 Subject: [PATCH 2/2] fix(test): assert the write_new_code env-var absence at the right encounter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code review of PR #310 caught: the negative test guarding against CRAFT_GUARD_ALLOW_DEV_EDIT leaking into write_new_code's suggestions was reading stderr from a 2nd-encounter call, which renders "brief" verbosity — suggestions are omitted there regardless of content, so the assertion passed vacuously and would not have caught the very regression it was named for. Fixed to capture a single 1st-encounter ("full" verbosity, where Suggest: lines actually render) stderr and assert both the positive ("New code files" present) and negative (env var absent) conditions against it. Verified by planting the regression (temporarily adding the env-var hint to write_new_code's suggestions) and confirming the test now fails, then reverting and confirming it passes clean. --- tests/test_branch_guard.sh | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/tests/test_branch_guard.sh b/tests/test_branch_guard.sh index 151ae07af..52ab88cf1 100755 --- a/tests/test_branch_guard.sh +++ b/tests/test_branch_guard.sh @@ -1874,14 +1874,26 @@ run_test_with_stderr \ # documented; mentioning the marker-bypass env var there would recreate the # exact confusion issue #309 reported (a working non-interactive route buried # among suggestions that don't apply to it). +# +# Both assertions below MUST read the same 1st-encounter capture. Suggest: +# lines only render at "full" (1st-encounter) verbosity — a 2nd+ encounter +# renders "brief", which omits ALL suggestions regardless of content, so +# checking a later call would pass this negative assertion vacuously (it +# would still pass even if the hint WERE mistakenly added to write_new_code) +# rather than actually exercising the code path it's meant to guard. REPO_MSG_NEWCODE=$(init_repo); switch_branch "$REPO_MSG_NEWCODE" "dev" -run_test_with_stderr \ - "test_write_new_code_message_does_not_mention_env_var" \ - 2 \ - "$(json_write "$REPO_MSG_NEWCODE/src/unrelated_new.py" "$REPO_MSG_NEWCODE")" \ - "$REPO_MSG_NEWCODE" \ - "New code files" -NEW_CODE_STDERR=$(echo "$(json_write "$REPO_MSG_NEWCODE/src/unrelated_new2.py" "$REPO_MSG_NEWCODE")" | (cd "$REPO_MSG_NEWCODE" && bash "$HOOK_SCRIPT") 2>&1 >/dev/null) || true +NEW_CODE_STDERR=$(echo "$(json_write "$REPO_MSG_NEWCODE/src/unrelated_new.py" "$REPO_MSG_NEWCODE")" | (cd "$REPO_MSG_NEWCODE" && bash "$HOOK_SCRIPT") 2>&1 >/dev/null) || true + +TOTAL=$((TOTAL + 1)) +if echo "$NEW_CODE_STDERR" | grep -qi "New code files"; then + PASS=$((PASS + 1)) + echo -e " ${T_GREEN}PASS${T_NC} test_write_new_code_message_does_not_mention_env_var ${T_BOLD}(exit=2, pattern matched)${T_NC}" +else + FAIL=$((FAIL + 1)) + FAILED_NAMES+=("test_write_new_code_message_does_not_mention_env_var") + echo -e " ${T_RED}FAIL${T_NC} test_write_new_code_message_does_not_mention_env_var ${T_BOLD}(1st-encounter stderr missing 'New code files')${T_NC}" +fi + TOTAL=$((TOTAL + 1)) if ! echo "$NEW_CODE_STDERR" | grep -q "CRAFT_GUARD_ALLOW_DEV_EDIT"; then PASS=$((PASS + 1))