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..52ab88cf1 100755 --- a/tests/test_branch_guard.sh +++ b/tests/test_branch_guard.sh @@ -1837,6 +1837,73 @@ 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). +# +# 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" +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)) + 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.