Skip to content

feat(cli): Windows-safe SQLite handle + cross-platform paths + sensitive write prefixes#54

Merged
KooshaPari merged 1 commit into
mainfrom
fix/windows-safe-delegate-and-federation-gate-removal
Jun 13, 2026
Merged

feat(cli): Windows-safe SQLite handle + cross-platform paths + sensitive write prefixes#54
KooshaPari merged 1 commit into
mainfrom
fix/windows-safe-delegate-and-federation-gate-removal

Conversation

@KooshaPari

@KooshaPari KooshaPari commented Jun 13, 2026

Copy link
Copy Markdown
Owner

User description

This PR carries the PolicyStack federation hardening work into the repository that is being folded into phenotype-tooling.


Note

High Risk
Changes deny more write and bypass paths in Claude hooks and alter security-sensitive path resolution; incorrect worktree or prefix logic could block legitimate edits or miss risky writes.

Overview
Claude hook enforcement is stricter: write-via-exec bypasses that would only ask are upgraded to deny; unapproved writes under sensitive prefixes (e.g. /etc/, /tmp/) outside approved worktree paths are denied with a synthetic rule. Shell redirect detection and _resolve_target_path now handle Windows-style paths more reliably.

Delegate cache on Windows uses explicit connect/close, journal_mode=DELETE, and gc.collect() so SQLite file handles release; pattern extraction tolerates negative numbers. RiskTierConfig gains optional max_risk_score from YAML.

CLI/resolver output is restored: policyctl _emit_json prints sorted JSON; resolve.py emits success/failure JSON and stderr messages for non-JSON errors.

Scripts/tests: policy paths in validation output use POSIX formatting; sync_host_rules treats unreadable host configs as “no managed segment”; eight duplicate scripts/federation/f20x/f21x_*_gate.py scripts are removed. Tests skip bash-only cases on Windows, use echo_argv, and relax SQLite timing thresholds on Windows.

Reviewed by Cursor Bugbot for commit 261103c. Bugbot is set up for automated code reviews on this repo. Configure here.


CodeAnt-AI Description

Make policy tools work on Windows and tighten blocked write handling

What Changed

  • Policy and resolver output now prints valid JSON and error messages instead of staying silent in non-JSON mode.
  • Delegate cache access is made Windows-safe so cached decisions can be reused without leaving SQLite files locked.
  • Path display and resolution now use consistent cross-platform formatting, including Windows-style paths.
  • Claude hooks now deny write attempts that bypass normal write actions, and block unapproved writes to sensitive system paths outside approved worktrees.
  • Several duplicate federation gate scripts were removed, and shell-only tests are skipped on Windows; validation and performance tests were updated to match the new path and timing behavior.
  • Added a short test note explaining how to run the suite on Windows and when shell integration tests are skipped.

Impact

✅ Fewer blocked policy cache reads on Windows
✅ Clearer resolver and CLI error output
✅ Stronger protection against unsafe file writes

💡 Usage Guide

Checking Your Pull Request

Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.

Talking to CodeAnt AI

Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

Preserve Org Learnings with CodeAnt

You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

Check Your Repository Health

To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.

@gemini-code-assist

Copy link
Copy Markdown

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@codeant-ai

codeant-ai Bot commented Jun 13, 2026

Copy link
Copy Markdown

CodeAnt AI is reviewing your PR.


Thanks for using CodeAnt! 🎉

We're free for open-source projects. if you're enjoying it, help us grow by sharing.

Share on X ·
Reddit ·
LinkedIn

@coderabbitai

coderabbitai Bot commented Jun 13, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@KooshaPari, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 54 minutes and 50 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more credits in the billing tab to continue.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: a8e79139-45e8-49d8-bb38-b021eea1a274

📥 Commits

Reviewing files that changed from the base of the PR and between 840bf24 and 261103c.

📒 Files selected for processing (34)
  • cli/src/policy_federation/claude_hooks.py
  • cli/src/policy_federation/cli.py
  • cli/src/policy_federation/config_loader.py
  • cli/src/policy_federation/delegate.py
  • resolve.py
  • scripts/federation/f201_*
  • scripts/federation/f201_board_threshold_regression_stability_gate.py
  • scripts/federation/f202_*
  • scripts/federation/f202_kpi_entropy_window_budget_gate.py
  • scripts/federation/f203_*
  • scripts/federation/f203_recert_exception_stability_window_gate.py
  • scripts/federation/f204_*
  • scripts/federation/f204_succession_transition_entropy_stability_gate.py
  • scripts/federation/f213_*
  • scripts/federation/f213_board_threshold_regression_stability_gate.py
  • scripts/federation/f214_*
  • scripts/federation/f214_kpi_entropy_window_budget_gate.py
  • scripts/federation/f215_*
  • scripts/federation/f215_recert_exception_stability_window_gate.py
  • scripts/federation/f216_*
  • scripts/federation/f216_succession_transition_entropy_stability_gate.py
  • scripts/output_contract.py
  • scripts/policy_common.py
  • scripts/sync_host_rules.py
  • scripts/validate_policy_contract.py
  • tests/README.md
  • tests/test_integration.py
  • tests/test_performance.py
  • tests/test_platform_wrappers.py
  • tests/test_policy_contract.py
  • tests/test_policy_contract_validation_governance.py
  • tests/test_smoke_dispatch_host_hook.py
  • tests/unit/support.py
  • tests/unit/test_interceptor.py
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/windows-safe-delegate-and-federation-gate-removal
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch fix/windows-safe-delegate-and-federation-gate-removal

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codeant-ai codeant-ai Bot added the size:L This PR changes 100-499 lines, ignoring generated files label Jun 13, 2026

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 3 potential issues.

Fix All in Cursor

Bugbot Autofix is ON. A cloud agent has been kicked off to fix the reported issues.

Reviewed by Cursor Bugbot for commit 261103c. Configure here.

""")
conn.commit()
conn.close()
_INITIALIZED_CACHE_DBS.add(db_key)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cache schema skipped after deletion

Medium Severity

_ensure_cache_schema returns immediately when the DB path is in _INITIALIZED_CACHE_DBS, without checking that tables still exist. If delegate_cache.db is removed or replaced while the process keeps running, later cache reads and writes hit an empty file, SQL fails, and caching silently stops until restart.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 261103c. Configure here.

if paths and all(_is_worktree_path(path) for path in paths):
return False

return _targets_sensitive_write_paths(paths)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sensitive deny skips resolved paths

High Severity

The new _should_deny_unapproved_write gate checks raw target_paths against Unix /etc/, /tmp/, etc., but Write-tool paths are passed through unchanged and never run through _resolve_target_path. Relative targets such as ../etc/passwd can stay in ask instead of being upgraded to deny even when they resolve outside approved worktrees to sensitive locations.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 261103c. Configure here.

for raw in target_paths:
path = _normalize_policy_path(raw)
if any(path.startswith(prefix) for prefix in _SENSITIVE_WRITE_PREFIXES):
return True

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Windows paths bypass sensitive deny

Medium Severity

_SENSITIVE_WRITE_PREFIXES and _targets_sensitive_write_paths only recognize Unix-style absolute prefixes like /tmp/ and /etc/. On Windows, writes to typical sensitive locations (for example C:/Windows/... or user Temp paths) never match, so the new ask-to-deny hardening does not apply despite cross-platform path work elsewhere in the PR.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 261103c. Configure here.

@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
1 Security Hotspot
D Security Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

normalize_payload(payload, Path.cwd())


@pytest.mark.skipif(sys.platform == "win32", reason="requires bash/WSL")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Add an FR trace reference to this new test section (for example in the skip reason or an adjacent docstring/marker) so the file includes a repository-compliant FR identifier for test traceability. [custom_rule]

Severity Level: Minor ⚠️

Why it matters? 🤔

This is a newly added test section, and the file contains no FR-XXX-NNN reference in the class name, marker, docstring, or skip reason. That matches the repository rule requiring FR traceability for new tests, so the violation is real.

Fix in Cursor | Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** tests/test_policy_contract.py
**Line:** 582:582
**Comment:**
	*Custom Rule: Add an FR trace reference to this new test section (for example in the skip reason or an adjacent docstring/marker) so the file includes a repository-compliant FR identifier for test traceability.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎


assert result.returncode == 0
assert f"[skip] {missing_path} (missing)" in result.stdout
assert "[skip] policy-config/system.yaml (missing)" in result.stdout

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Add an FR trace reference (for example in a test docstring, pytest marker, or FR-tagged test name) in this touched test module so it contains at least one explicit FR-XXX-NNN identifier as required by the repository traceability rules. [custom_rule]

Severity Level: Minor ⚠️

Why it matters? 🤔

The repository rule requires test files to reference at least one FR ID via a test name, marker, tag, or docstring. This touched test module contains no FR-XXX-NNN references, so the suggestion correctly identifies a real traceability violation.

Fix in Cursor | Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** tests/test_policy_contract_validation_governance.py
**Line:** 97:97
**Comment:**
	*Custom Rule: Add an FR trace reference (for example in a test docstring, pytest marker, or FR-tagged test name) in this touched test module so it contains at least one explicit `FR-XXX-NNN` identifier as required by the repository traceability rules.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎

Comment on lines +12 to +15
pytestmark = pytest.mark.skipif(
sys.platform == "win32",
reason="requires bash/WSL",
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Add an FR trace reference to this test module (for example via a requirement marker or equivalent FR tag) so the file is not an orphan test and complies with the repository traceability rule. [custom_rule]

Severity Level: Minor ⚠️

Why it matters? 🤔

The repository rule requires test files to reference at least one FR ID via a test name, marker, tag, or docstring. This module's test-level marker only skips on Windows and does not include any FR-XXX-NNN trace reference, so the suggestion correctly identifies a real traceability violation.

Fix in Cursor | Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** tests/test_smoke_dispatch_host_hook.py
**Line:** 12:15
**Comment:**
	*Custom Rule: Add an FR trace reference to this test module (for example via a `requirement` marker or equivalent FR tag) so the file is not an orphan test and complies with the repository traceability rule.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎

Comment thread tests/test_integration.py
Comment on lines +409 to +411
# Cache hit should be very fast (skip timing race on Windows SQLite)
if sys.platform != "win32":
assert hit_time < miss_time or miss_time < 0.01

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Add an explicit FR trace reference (for example, FR-XXX-NNN) in this updated test (docstring, pytest marker, or inline trace comment) so the file satisfies the repository requirement that tests include requirement linkage. [custom_rule]

Severity Level: Minor ⚠️

Why it matters? 🤔

The repository rule requires test files to reference at least one FR ID (for example in a docstring, marker, tag, or trace comment). This test file contains no FR-XXX-NNN reference anywhere, so the updated test does violate the traceability requirement.

Fix in Cursor | Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** tests/test_integration.py
**Line:** 409:411
**Comment:**
	*Custom Rule: Add an explicit FR trace reference (for example, `FR-XXX-NNN`) in this updated test (docstring, pytest marker, or inline trace comment) so the file satisfies the repository requirement that tests include requirement linkage.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎

Comment thread tests/test_performance.py
from __future__ import annotations

import statistics
import sys

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Add at least one FR trace reference in this test module (for example a Traces to: FR-... docstring/comment or a requirement marker) so the file complies with mandatory test-to-requirement traceability rules. [custom_rule]

Severity Level: Minor ⚠️

Why it matters? 🤔

The repository rule requires test files to reference at least one FR ID via a test name, marker, tag, or docstring trace. This module contains no FR-XXX-NNN references anywhere, so the missing traceability is a real rule violation.

Fix in Cursor | Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** tests/test_performance.py
**Line:** 9:9
**Comment:**
	*Custom Rule: Add at least one FR trace reference in this test module (for example a `Traces to: FR-...` docstring/comment or a requirement marker) so the file complies with mandatory test-to-requirement traceability rules.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎

record_audit_event,
)
from support import REPO_ROOT
from support import REPO_ROOT, echo_argv

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Add at least one FR trace reference in this test module (for example a file-level # @trace FR-XXX-NNN tag or FR-linked test docstring/name) so the updated tests are not orphaned from requirements. [custom_rule]

Severity Level: Minor ⚠️

Why it matters? 🤔

The repository rule requires test files to reference at least one FR ID via test names, markers, tags, or docstrings. This file contains no FR-XXX-NNN references anywhere in the test module, so the suggestion correctly identifies a real rule violation.

Fix in Cursor | Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** tests/unit/test_interceptor.py
**Line:** 23:23
**Comment:**
	*Custom Rule: Add at least one FR trace reference in this test module (for example a file-level `# @trace FR-XXX-NNN` tag or FR-linked test docstring/name) so the updated tests are not orphaned from requirements.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎

Comment on lines +676 to +685
if platform == "forge":
forge_data = _load_json(path)
allow = _read_list_field(forge_data, "commandAllowlist", path)
request = _read_list_field(forge_data, "commandRequestlist", path)
deny = _read_list_field(forge_data, "commandDenylist", path)
return (
_has_json_managed_segment(allow, path=path, key="commandAllowlist")
or _has_json_managed_segment(request, path=path, key="commandRequestlist")
or _has_json_managed_segment(deny, path=path, key="commandDenylist")
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: The new Forge branch parses the host config with _load_json, but Forge settings are YAML (permissions.yaml) and are read as YAML elsewhere in this script. A valid Forge YAML file will fail JSON parsing, get swallowed by the broad except, and incorrectly report had_managed_segment_before=False. Parse Forge files as YAML in this branch (or use the same loader as _apply_forge_rules) so managed segment detection is accurate. [api mismatch]

Severity Level: Major ⚠️
- ⚠️ Direct callers of `_had_managed_segment_before("forge")` get false negatives.
- ⚠️ Future Forge host-rule reporting would misstate managed segment presence.
Steps of Reproduction ✅
1. Create a Forge permissions file at some path, e.g. `/tmp/permissions.yaml`, containing
a valid YAML mapping with `commandAllowlist`, `commandRequestlist`, and `commandDenylist`
keys and including the managed-segment markers used by `replace_managed_entries()` in
`_apply_forge_rules` at `scripts/sync_host_rules.py:10-37` (loaded via
`forge_yaml.safe_load(path.read_text(...))`).

2. In a Python REPL or unit test, import `_had_managed_segment_before` from
`scripts/sync_host_rules.py` where it is defined at `scripts/sync_host_rules.py:207-219`
and call `_had_managed_segment_before("forge", Path("/tmp/permissions.yaml"))`.

3. During this call, the shared loader `_load_json()` at
`scripts/sync_host_rules.py:10-20` executes `json.load(fp)` on the YAML file at line 652
(`data = _load_json(path)`), raising a `ValueError`/`json.JSONDecodeError` because the
file is YAML, not JSON; this exception is caught by the `except (OSError, ValueError,
json.JSONDecodeError):` block at line 696.

4. Because the exception is swallowed, `_had_managed_segment_before()` returns `False`
(lines 647-649), incorrectly reporting `had_managed_segment_before=False` for a Forge
config that does have a managed segment, in contrast to `_apply_forge_rules()` which reads
the same file as YAML at `scripts/sync_host_rules.py:10-23`.

Fix in Cursor | Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** scripts/sync_host_rules.py
**Line:** 676:685
**Comment:**
	*Api Mismatch: The new Forge branch parses the host config with `_load_json`, but Forge settings are YAML (`permissions.yaml`) and are read as YAML elsewhere in this script. A valid Forge YAML file will fail JSON parsing, get swallowed by the broad `except`, and incorrectly report `had_managed_segment_before=False`. Parse Forge files as YAML in this branch (or use the same loader as `_apply_forge_rules`) so managed segment detection is accurate.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎

Comment on lines +210 to +211
if normalized_cwd.startswith("/"):
return f"{normalized_cwd.rstrip('/')}/{normalized_target.lstrip('/')}"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Relative target paths are now concatenated with the cwd without normalization, so traversal segments like ../ are preserved. This lets writes such as ../../etc/shadow avoid sensitive-prefix detection because the resulting string starts with the cwd instead of /etc/. Resolve and normalize the joined path before policy checks so traversal cannot bypass the deny logic. [security]

Severity Level: Critical 🚨
- ❌ Sensitive writes under /etc bypass prefix-based blocking.
- ❌ Claude Bash hooks fail to block ../etc writes.
- ⚠️ Enforcement logs misrepresent actual sensitive filesystem targets.
Steps of Reproduction ✅
1. Run the Claude pre-tool hook entrypoint `main()` in
`cli/src/policy_federation/claude_hooks.py:116-124`, which reads a JSON payload from stdin
and calls `evaluate_claude_pretool_payload(payload)` at `claude_hooks.py:389`.

2. Provide a payload with `tool_name: "Bash"`, `cwd: "/tmp/worktrees/repo"` and
`tool_input["command"]` set to `python -c "open('../../etc/shadow','w').write('x')"` so
that `_extract_request()` at `claude_hooks.py:248-297` classifies this as a write-via-exec
and extracts `target_paths` using the `open()` regex at `claude_hooks.py:283-285`.

3. Inside `_extract_request()`, the `open()` match `m.group(1)` is `"../../etc/shadow"`
and `_resolve_target_path(m.group(1), resolved_cwd)` is called at `claude_hooks.py:284`
with `resolved_cwd="/tmp/worktrees/repo"`; `_resolve_target_path()` at
`claude_hooks.py:199-213` takes the branch `if normalized_cwd.startswith("/")` and returns
the non-canonical string `"/tmp/worktrees/repo/../../etc/shadow"` (line 210-211),
preserving the `"../"` segments.

4. Back in `evaluate_claude_pretool_payload()` at `claude_hooks.py:389-427`, the `result`
from `intercept_command()` is post-processed by `_should_deny_unapproved_write()` at
`claude_hooks.py:372-387`; this function computes `paths = target_paths or [cwd]` (here
`["/tmp/worktrees/repo/../../etc/shadow"]`) and calls `_targets_sensitive_write_paths()`
at `claude_hooks.py:364-369`, which checks `path.startswith(prefix)` for prefixes like
`"/etc/"`, `"/var/"`, `"/tmp/"` but never normalizes `"../"` segments, so
`"/tmp/worktrees/repo/../../etc/shadow"` is not recognized as starting with `"/etc/"` and
the synthetic deny rule is not triggered even though the real filesystem target resolves
to `/etc/shadow`.

Fix in Cursor | Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** cli/src/policy_federation/claude_hooks.py
**Line:** 210:211
**Comment:**
	*Security: Relative target paths are now concatenated with the cwd without normalization, so traversal segments like `../` are preserved. This lets writes such as `../../etc/shadow` avoid sensitive-prefix detection because the resulting string starts with the cwd instead of `/etc/`. Resolve and normalize the joined path before policy checks so traversal cannot bypass the deny logic.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎

Comment on lines +356 to +361
def _is_worktree_path(path: str) -> bool:
normalized = _normalize_policy_path(path)
return any(
marker in normalized
for marker in ("-wtrees/", "/.worktrees/", "/worktrees/", "PROJECT-wtrees/")
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Worktree detection uses substring matching on arbitrary path text, so attacker-controlled paths like /tmp/worktrees/evil.txt are treated as approved worktree paths even when they are outside any real approved checkout. This can suppress the synthetic deny for sensitive writes; replace this with strict path-root validation against known approved worktree directories. [incorrect condition logic]

Severity Level: Critical 🚨
- ❌ Synthetic deny skipped for /tmp/worktrees outside repos.
- ❌ Attackers craft paths mimicking worktrees to avoid checks.
- ⚠️ Risk assessment treats untrusted temporary directories as worktrees.
Steps of Reproduction ✅
1. Note the worktree detection helper `_is_worktree_path()` in
`cli/src/policy_federation/claude_hooks.py:356-361`, which normalizes separators then
returns `True` if any of the markers `"-wtrees/"`, `"/.worktrees/"`, `"/worktrees/"`, or
`"PROJECT-wtrees/"` appear as a substring anywhere in the path, without tying this to an
actual repository root or approved worktree list.

2. Observe `_should_deny_unapproved_write()` at `claude_hooks.py:372-387`, which is called
from `evaluate_claude_pretool_payload()` at `claude_hooks.py:389-427` after
`intercept_command()` returns, and is responsible for applying a synthetic deny when
`action == "write"`, `final_decision == "ask"`, and the target paths hit sensitive
prefixes listed in `_SENSITIVE_WRITE_PREFIXES` at `claude_hooks.py:340-349`.

3. Supply a payload to `main()` (`claude_hooks.py:116-124`) where `_extract_request()` at
`claude_hooks.py:307-320` sets `action: "write"` and `target_paths:
["/tmp/worktrees/evil.txt"]` (for example by using a write tool targeting
`/tmp/worktrees/evil.txt`) so that `evaluate_claude_pretool_payload()` later calls
`_should_deny_unapproved_write(action="write", target_paths=["/tmp/worktrees/evil.txt"],
cwd="/tmp", final_decision="ask")`.

4. Inside `_should_deny_unapproved_write()`, `paths` becomes `["/tmp/worktrees/evil.txt"]`
and the guard `if paths and all(_is_worktree_path(path) for path in paths): return False`
at `claude_hooks.py:382-384` calls `_is_worktree_path("/tmp/worktrees/evil.txt")`, which
returns `True` because the substring `"/worktrees/"` is present, so
`_should_deny_unapproved_write()` exits early and never calls
`_targets_sensitive_write_paths()` at `claude_hooks.py:364-369`; as a result, the
synthetic deny for sensitive prefixes such as `"/tmp/"` is skipped even though
`/tmp/worktrees/evil.txt` is not an approved repository worktree path.

Fix in Cursor | Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** cli/src/policy_federation/claude_hooks.py
**Line:** 356:361
**Comment:**
	*Incorrect Condition Logic: Worktree detection uses substring matching on arbitrary path text, so attacker-controlled paths like `/tmp/worktrees/evil.txt` are treated as approved worktree paths even when they are outside any real approved checkout. This can suppress the synthetic deny for sensitive writes; replace this with strict path-root validation against known approved worktree directories.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎

@codeant-ai

codeant-ai Bot commented Jun 13, 2026

Copy link
Copy Markdown

CodeAnt AI finished reviewing your PR.

@KooshaPari KooshaPari merged commit db9d18e into main Jun 13, 2026
13 of 15 checks passed
@KooshaPari KooshaPari deleted the fix/windows-safe-delegate-and-federation-gate-removal branch June 13, 2026 02:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L This PR changes 100-499 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant