From e63a4e9945bc68bb29c563a8b0b69e448b522d59 Mon Sep 17 00:00:00 2001 From: AIOSAI Date: Tue, 2 Jun 2026 20:24:21 -0700 Subject: [PATCH] feat(#630): retire blanket rm deny + aipass doctor migration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 2 of #630. The rm_gate hook + drone rm now own destructive-delete protection (cross-provider, path-aware, teaching), so the Claude-only blanket deny is redundant AND harmful (it short-circuits before the hook, suppressing the teaching message). - setup.sh: removed Bash(rm -rf*) + Bash(rm -r *) from git_deny (new installs) - bootstrap.py: removed Bash(rm -rf *) shipped via aipass init (project settings) - doctor_wire.reconcile_stale_deny(): aipass doctor WARNs on stale rules; --fix removes them (idempotent, preserves all else) — migration for existing installs (the 'aipass update should be trusted' goal) - .aipass/project_hooks.json template: added rm_gate (new projects get it) Tests: 8 reconcile + 432 aipass total, seedgo 99%. CHANGELOG W23. --- .aipass/project_hooks.json | 5 + CHANGELOG.md | 11 ++ setup.sh | 2 - .../aipass/apps/handlers/init/bootstrap.py | 1 - src/aipass/aipass/apps/modules/doctor.py | 28 ++-- src/aipass/aipass/apps/modules/doctor_wire.py | 50 +++++++ src/aipass/aipass/tests/test_doctor.py | 130 ++++++++++++++++++ 7 files changed, 211 insertions(+), 16 deletions(-) diff --git a/.aipass/project_hooks.json b/.aipass/project_hooks.json index 4059135c..2521ade5 100644 --- a/.aipass/project_hooks.json +++ b/.aipass/project_hooks.json @@ -40,6 +40,11 @@ "enabled": true, "handler": "aipass.hooks.apps.handlers.security.git_gate.handle", "matcher": "Bash|Edit|MultiEdit|Write|NotebookEdit" + }, + "rm_gate": { + "enabled": true, + "handler": "aipass.hooks.apps.handlers.security.rm_gate.handle", + "matcher": "Bash" } }, diff --git a/CHANGELOG.md b/CHANGELOG.md index a9d9c011..74fc4574 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,17 @@ and this project uses [Calendar Versioning](https://calver.org/) in the format (unparseable targets are blocked, not allowed), and skips `drone rm` itself. This makes the safe-delete path discoverable at the moment of friction. (#630) +### Changed + +- **Retired the blanket `rm` deny from provider settings** — `setup.sh` and + `aipass init` no longer ship `Bash(rm -rf*)` / `Bash(rm -r *)` deny rules + (they were mis-filed among git rules, blocked all `/tmp` cleanup, and gave a + bare "permission denied" with no guidance). The `rm_gate` hook + `drone rm` + now own this — cross-provider, path-aware, and they teach. `aipass doctor` + detects the stale rules on existing installs and `aipass doctor --fix` removes + them (idempotent, preserves all other rules). Claude Code still natively + circuit-breaks `rm -rf /` and `rm -rf ~`. (#630) + ### Fixed - **A release merge can no longer destroy the `dev` branch** — `drone @git merge` diff --git a/setup.sh b/setup.sh index 3f1e9c18..abc55d35 100755 --- a/setup.sh +++ b/setup.sh @@ -606,7 +606,6 @@ git_deny = [ "Bash(git push -f *)", "Bash(git rebase*)", "Bash(git clean*)", - "Bash(rm -rf*)", "Bash(git reset*)", "Bash(git merge*)", "Bash(git config*)", @@ -617,7 +616,6 @@ git_deny = [ "Bash(git branch -D*)", "Bash(git stash drop*)", "Bash(git stash clear*)", - "Bash(rm -r *)", "Bash(git checkout -b*)", "Bash(git switch -c*)", "Bash(git switch --create*)", diff --git a/src/aipass/aipass/apps/handlers/init/bootstrap.py b/src/aipass/aipass/apps/handlers/init/bootstrap.py index 948fbfa1..034ad7ec 100644 --- a/src/aipass/aipass/apps/handlers/init/bootstrap.py +++ b/src/aipass/aipass/apps/handlers/init/bootstrap.py @@ -213,7 +213,6 @@ def _claude_settings(aipass_home: str | None = None) -> str: data["permissions"] = { "deny": [ - "Bash(rm -rf *)", "Bash(git push --force*)", "Bash(git reset --hard*)", "EnterPlanMode", diff --git a/src/aipass/aipass/apps/modules/doctor.py b/src/aipass/aipass/apps/modules/doctor.py index 01ec8dbe..ac8d0943 100644 --- a/src/aipass/aipass/apps/modules/doctor.py +++ b/src/aipass/aipass/apps/modules/doctor.py @@ -38,6 +38,7 @@ from aipass.aipass.apps.modules.doctor_fix import ( from aipass.aipass.apps.modules.doctor_wire import ( _auto_wire_provider, prompt_auto_wire, + reconcile_stale_deny, ) from aipass.aipass.apps.handlers.system_detect.system_detector import ( detect_cpu, @@ -68,9 +69,6 @@ class CheckResult(NamedTuple): remediation: str -# --- Identity helpers --- - - def _find_registry() -> Path | None: """Walk up from CWD first (user's project), then branch root.""" cwd = Path.cwd() @@ -87,9 +85,6 @@ def _find_registry() -> Path | None: return None -# --- Check groups --- - - def _check_system() -> List[CheckResult]: """Run System group checks.""" results: List[CheckResult] = [] @@ -147,11 +142,10 @@ def _check_identity() -> List[CheckResult]: """Run Identity group checks.""" results: List[CheckResult] = [] - # Project root — derived from registry location - reg = _find_registry() - project_root = str(reg.parent) if reg else "" - if project_root: - results.append(CheckResult("AIPASS_HOME", GLYPH_PASS, project_root, "")) + # Project root + registry — single lookup + reg_path = _find_registry() + if reg_path: + results.append(CheckResult("AIPASS_HOME", GLYPH_PASS, str(reg_path.parent), "")) else: home = os.environ.get("AIPASS_HOME", "") if home: @@ -166,8 +160,6 @@ def _check_identity() -> List[CheckResult]: ) ) - # Registry present - reg_path = _find_registry() if reg_path is None: results.append(CheckResult("registry", GLYPH_FAIL, "not found", "Run 'aipass init' to create registry")) return results @@ -451,6 +443,10 @@ def _check_services(verbose: bool = False) -> List[CheckResult]: manifest_checks = _check_provider_manifest() results.extend(manifest_checks) + # stale rm deny rules — detect only (fix runs in run_doctor when --fix) + for tup in reconcile_stale_deny(fix=False): + results.append(CheckResult(*tup)) + return results @@ -588,6 +584,12 @@ def run_doctor(verbose: bool = False, interactive: bool = False, fix: bool = Fal r for r in groups.get("Services", []) if r.label not in ("hooks", "env vars", "permissions") ] + manifest_results + if fix: + stale_results = [CheckResult(*tup) for tup in reconcile_stale_deny(fix=True)] + if stale_results: + services = groups.get("Services", []) + groups["Services"] = [r for r in services if r.label != "rm deny migration"] + stale_results + pass_count = 0 warn_count = 0 error_count = 0 diff --git a/src/aipass/aipass/apps/modules/doctor_wire.py b/src/aipass/aipass/apps/modules/doctor_wire.py index 03aaadad..ca639f5a 100644 --- a/src/aipass/aipass/apps/modules/doctor_wire.py +++ b/src/aipass/aipass/apps/modules/doctor_wire.py @@ -51,6 +51,56 @@ ENV_DESCRIPTIONS: Dict[str, str] = { # ============================================================================= +# STALE DENY RULE MIGRATION +# ============================================================================= + +_STALE_RM_DENY_RULES = frozenset({"Bash(rm -rf*)", "Bash(rm -r *)"}) + + +def reconcile_stale_deny(fix: bool = False) -> list: + """Detect and optionally remove stale rm deny rules from provider settings. + + Returns list of (label, glyph, detail, remediation) tuples matching + doctor.CheckResult shape — imported as tuples to avoid circular import. + """ + from aipass.aipass.apps.handlers.ui.progress import GLYPH_PASS, GLYPH_WARN + + results: list = [] + settings_path = Path.home() / ".claude" / "settings.json" + if not settings_path.exists(): + return results + + data = json_handler.load_path(settings_path) + if data is None: + return results + + deny = data.get("permissions", {}).get("deny", []) + stale = [r for r in deny if r in _STALE_RM_DENY_RULES] + + if not stale: + results.append(("rm deny migration", GLYPH_PASS, "no stale rules", "")) + return results + + if fix: + deny_cleaned = [r for r in deny if r not in _STALE_RM_DENY_RULES] + data.setdefault("permissions", {})["deny"] = deny_cleaned + json_handler.save_path(settings_path, data) + removed = ", ".join(stale) + results.append(("rm deny migration", GLYPH_PASS, f"removed: {removed}", "")) + logger.info("[doctor] removed stale deny rules: %s", stale) + else: + found = ", ".join(stale) + results.append( + ( + "rm deny migration", + GLYPH_WARN, + f"stale rules: {found}", + "Run aipass doctor --fix to remove (rm_gate + drone rm replace these)", + ) + ) + + return results + # ============================================================================= # AUTO-WIRE diff --git a/src/aipass/aipass/tests/test_doctor.py b/src/aipass/aipass/tests/test_doctor.py index b7057f85..c59a8bf2 100644 --- a/src/aipass/aipass/tests/test_doctor.py +++ b/src/aipass/aipass/tests/test_doctor.py @@ -612,3 +612,133 @@ class TestHooksJsonCheck: assert len(hooks_results) == 1 assert hooks_results[0].glyph == GLYPH_WARN assert "init update" in hooks_results[0].remediation + + +# ============================================================================= +# TestReconcileStaleDeny +# ============================================================================= + + +class TestReconcileStaleDeny: + """Tests for stale rm deny rule migration (DPLAN-0192 Phase 2).""" + + def test_no_settings_file_returns_empty(self, tmp_path) -> None: + """Missing settings.json returns no results.""" + from aipass.aipass.apps.modules.doctor_wire import reconcile_stale_deny + + with patch("aipass.aipass.apps.modules.doctor_wire.Path.home", return_value=tmp_path): + results = reconcile_stale_deny(fix=False) + assert results == [] + + def test_no_stale_rules_returns_pass(self, tmp_path) -> None: + """Settings with no stale rm rules returns PASS.""" + from aipass.aipass.apps.modules.doctor_wire import reconcile_stale_deny + + settings = tmp_path / ".claude" / "settings.json" + settings.parent.mkdir(parents=True) + settings.write_text( + json.dumps({"permissions": {"deny": ["Bash(git push --force*)", "Bash(git reset --hard*)"]}}), + encoding="utf-8", + ) + with patch("aipass.aipass.apps.modules.doctor_wire.Path.home", return_value=tmp_path): + results = reconcile_stale_deny(fix=False) + assert len(results) == 1 + assert results[0][1] == GLYPH_PASS + assert "no stale" in results[0][2] + + def test_stale_rules_detected_without_fix(self, tmp_path) -> None: + """Stale rm rules present returns WARN when fix=False.""" + from aipass.aipass.apps.modules.doctor_wire import reconcile_stale_deny + + settings = tmp_path / ".claude" / "settings.json" + settings.parent.mkdir(parents=True) + settings.write_text( + json.dumps({"permissions": {"deny": ["Bash(rm -rf*)", "Bash(git push --force*)", "Bash(rm -r *)"]}}), + encoding="utf-8", + ) + with patch("aipass.aipass.apps.modules.doctor_wire.Path.home", return_value=tmp_path): + results = reconcile_stale_deny(fix=False) + assert len(results) == 1 + assert results[0][1] == GLYPH_WARN + assert "rm -rf" in results[0][2] + assert "rm -r " in results[0][2] + + def test_fix_removes_stale_rules(self, tmp_path) -> None: + """fix=True removes stale rules and preserves others.""" + from aipass.aipass.apps.modules.doctor_wire import reconcile_stale_deny + + settings = tmp_path / ".claude" / "settings.json" + settings.parent.mkdir(parents=True) + original = { + "permissions": {"deny": ["Bash(rm -rf*)", "Bash(git push --force*)", "Bash(rm -r *)"]}, + "env": {"AIPASS_HOME": "/test"}, + } + settings.write_text(json.dumps(original), encoding="utf-8") + with patch("aipass.aipass.apps.modules.doctor_wire.Path.home", return_value=tmp_path): + results = reconcile_stale_deny(fix=True) + assert len(results) == 1 + assert results[0][1] == GLYPH_PASS + assert "removed" in results[0][2] + updated = json.loads(settings.read_text(encoding="utf-8")) + assert "Bash(rm -rf*)" not in updated["permissions"]["deny"] + assert "Bash(rm -r *)" not in updated["permissions"]["deny"] + assert "Bash(git push --force*)" in updated["permissions"]["deny"] + assert updated["env"]["AIPASS_HOME"] == "/test" + + def test_fix_single_stale_rule(self, tmp_path) -> None: + """fix=True works when only one of two stale rules is present.""" + from aipass.aipass.apps.modules.doctor_wire import reconcile_stale_deny + + settings = tmp_path / ".claude" / "settings.json" + settings.parent.mkdir(parents=True) + settings.write_text( + json.dumps({"permissions": {"deny": ["Bash(rm -rf*)", "Bash(git reset --hard*)"]}}), + encoding="utf-8", + ) + with patch("aipass.aipass.apps.modules.doctor_wire.Path.home", return_value=tmp_path): + results = reconcile_stale_deny(fix=True) + assert len(results) == 1 + assert results[0][1] == GLYPH_PASS + updated = json.loads(settings.read_text(encoding="utf-8")) + assert updated["permissions"]["deny"] == ["Bash(git reset --hard*)"] + + def test_fix_idempotent(self, tmp_path) -> None: + """Running fix twice is safe — second run returns PASS with no stale rules.""" + from aipass.aipass.apps.modules.doctor_wire import reconcile_stale_deny + + settings = tmp_path / ".claude" / "settings.json" + settings.parent.mkdir(parents=True) + settings.write_text( + json.dumps({"permissions": {"deny": ["Bash(rm -rf*)", "Bash(rm -r *)"]}}), + encoding="utf-8", + ) + with patch("aipass.aipass.apps.modules.doctor_wire.Path.home", return_value=tmp_path): + reconcile_stale_deny(fix=True) + results = reconcile_stale_deny(fix=True) + assert len(results) == 1 + assert results[0][1] == GLYPH_PASS + assert "no stale" in results[0][2] + + def test_empty_deny_list_returns_pass(self, tmp_path) -> None: + """Empty deny list returns PASS.""" + from aipass.aipass.apps.modules.doctor_wire import reconcile_stale_deny + + settings = tmp_path / ".claude" / "settings.json" + settings.parent.mkdir(parents=True) + settings.write_text(json.dumps({"permissions": {"deny": []}}), encoding="utf-8") + with patch("aipass.aipass.apps.modules.doctor_wire.Path.home", return_value=tmp_path): + results = reconcile_stale_deny(fix=False) + assert len(results) == 1 + assert results[0][1] == GLYPH_PASS + + def test_no_permissions_key_returns_pass(self, tmp_path) -> None: + """Settings without permissions key returns PASS.""" + from aipass.aipass.apps.modules.doctor_wire import reconcile_stale_deny + + settings = tmp_path / ".claude" / "settings.json" + settings.parent.mkdir(parents=True) + settings.write_text(json.dumps({"env": {"FOO": "bar"}}), encoding="utf-8") + with patch("aipass.aipass.apps.modules.doctor_wire.Path.home", return_value=tmp_path): + results = reconcile_stale_deny(fix=False) + assert len(results) == 1 + assert results[0][1] == GLYPH_PASS