Skip to content

Rectify: Write Guard Path Resolution Immunity - #3555

Merged
Trecek merged 6 commits into
developfrom
write-guard-extract-redirect-targets-drops-relative-paths-ha/3549
Jun 1, 2026
Merged

Rectify: Write Guard Path Resolution Immunity#3555
Trecek merged 6 commits into
developfrom
write-guard-extract-redirect-targets-drops-relative-paths-ha/3549

Conversation

@Trecek

@Trecek Trecek commented Jun 1, 2026

Copy link
Copy Markdown
Collaborator

Summary

The write guard has three independent path extraction pathways for Bash commands: segment-verb extraction (sed, tee, mv...), redirect extraction (>, >>, 2>...), and interpreter extraction (python3 -c "open(...)"). Each pathway independently implements CWD-relative path resolution — or fails to. The redirect pathway silently drops relative paths, and a vestigial has_non_gh gate bypasses redirect checking entirely for gh-only commands.

The architectural weakness is decentralized path resolution — each extraction function must independently implement the same if path.startswith("/") ... elif cwd: os.path.join(cwd, path) pattern, and new or modified extraction functions routinely miss it. The write guard has been rectified 10+ times in 90 days; each fix addresses one bypass while leaving adjacent ones open.

The immunity plan introduces a centralized resolve_write_target() function that all extraction pathways must use, eliminates the vestigial has_non_gh gate, and adds a cross-product test matrix that catches missing resolution by design.

Closes #3549

Implementation Plan

Plan file: /home/talon/projects/autoskillit-runs/remediation-20260601-132207-687449/.autoskillit/temp/rectify/rectify_write_guard_path_resolution_immunity_2026-06-01_140500.md

🤖 Generated with Claude Code via AutoSkillit

Token Usage Summary

Step Model count uncached output cache_read peak_ctx turns cache_write time
rectify* opus[1m] 1 80 38.3k 2.5M 146.7k 61 130.0k 19m 1s
review_approach* opus[1m] 1 7.8k 6.0k 196.5k 49.9k 12 37.1k 5m 39s
dry_walkthrough* opus 1 30 7.1k 776.7k 89.9k 29 73.3k 4m 54s
implement* opus[1m] 1 68 12.3k 2.1M 86.7k 65 70.1k 4m 47s
audit_impl* opus[1m] 1 33 10.5k 179.9k 41.0k 15 36.2k 6m 28s
prepare_pr* MiniMax-M3 1 45.5k 3.2k 165.5k 48.9k 16 0 1m 23s
compose_pr* MiniMax-M3 1 106.4k 1.7k 118.6k 39.0k 13 0 58s
review_pr* opus[1m] 1 41 38.3k 812.7k 91.6k 40 76.0k 9m 3s
resolve_review* opus[1m] 1 42 14.0k 1.5M 74.5k 52 58.5k 10m 15s
Total 159.9k 131.3k 8.4M 146.7k 481.2k 1h 2m

* Step used a non-Anthropic provider; caching behavior may differ.

Token Efficiency

Step LoC Changed cache_read/LoC cache_write/LoC output/LoC
rectify 0
review_approach 0
dry_walkthrough 0
implement 292 7285.4 240.1 42.1
audit_impl 0
prepare_pr 0
compose_pr 0
review_pr 0
resolve_review 5 301561.2 11693.4 2801.0
Total 297 28128.0 1620.1 442.2

Model Usage Breakdown

Model steps uncached output cache_read cache_write time
opus[1m] 6 8.0k 119.4k 7.3M 407.9k 55m 17s
opus 1 30 7.1k 776.7k 73.3k 4m 54s
MiniMax-M3 2 151.9k 4.8k 284.1k 0 2m 21s

Trecek and others added 6 commits June 1, 2026 14:35
Add TestResolveWriteTarget, TestExtractRedirectTargetsCwd,
TestRedirectRelativePathResolution, TestGhCommandRedirectChecking,
TestWriteGuardCrossProductMatrix, and arch test for resolve_write_target
usage enforcement. All tests expected to fail until implementation.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add resolve_write_target() to _command_classification.py as the single
path resolution function for all extraction pathways. Update
extract_redirect_targets() to accept cwd parameter and use
resolve_write_target() instead of inline startswith("/") checks.

Remove vestigial has_non_gh gate in _extract_bash_write_targets() so
redirect checking runs unconditionally — the token-based parser (PR
#3300) correctly identifies redirect targets without confusing
positional arguments.

Refactor all 6 inline resolution sites in _extract_segment_targets()
to use resolve_write_target(), eliminating duplicated path resolution
logic.

Fixes #3549

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…th calls

The arch test for extract_redirect_targets was checking for any
"startswith" string in the AST dump, but the function legitimately uses
tok.startswith("(") for subshell depth tracking. Narrow the check to
specifically detect .startswith("/") calls which indicate inline path
resolution that should use resolve_write_target() instead.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…o exact return type

The function returns [] (not None) when a redirect is detected but the
relative path cannot be resolved without CWD. Pin assertion to == []
for a stronger test contract.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…solution

Prevents file descriptor leak by using with-statement instead of bare
open().read().

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When no CWD is set, the redirect path cannot be resolved, so
found_any_write is never set and the function returns None (not []).
Pin to assert result is None for a stronger test contract.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@Trecek
Trecek force-pushed the write-guard-extract-redirect-targets-drops-relative-paths-ha/3549 branch from 8fd44fb to 11b1aed Compare June 1, 2026 21:35
@Trecek
Trecek added this pull request to the merge queue Jun 1, 2026
Merged via the queue into develop with commit 266f2f5 Jun 1, 2026
3 checks passed
@Trecek
Trecek deleted the write-guard-extract-redirect-targets-drops-relative-paths-ha/3549 branch June 1, 2026 21:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant