Repository navigation
fix: confine directory exclusions around symlinks (VULN-565) - #208
Conversation
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Validate and remove exclusion matches lib/copy.sh, tests/copy_safety.bats, tests/cmd_create_integration.bats, docs/configuration.md |
The exclusion logic validates roots and expands patterns component by component without descending through symlinks. Tests cover external targets, unsafe paths, leaf symlinks, spaces, shell-option state, and worktree creation. The documentation describes the exclusion behavior. |
Priority:
Estimated code review effort: 3 (Moderate) | ~20 minutes
Change: Bug fix · Severity of issue fixed: High
Suggested reviewers: helizaga
Merge Risk: 
6eb4b
-
The changed exclusion handling has no identified merge-blocking issue; normal checks can proceed.
Security architecture risk: 
6eb4b
-
The change restricts symlink-assisted deletion without adding privileges or expanding access. Remaining risk is low: cleanup still assumes the destination is not maliciously changed concurrently, and interrupted deletion is not transactional.
Retained concerns
No architecture-level concerns identified.Security review details
-
Security Blast Radius
- inferred — The relevant authority is the invoking user's filesystem access. Repository-controlled configuration and symlinks previously allowed exclusion deletion to reach removable files outside the worktree. The PR narrows that deterministic path; it does not introduce a service identity or privilege elevation.
Trust Boundaries and Controls
- observed — Enforcement remains at the destructive consumer rather than relying on configuration trust. Root checks reject unsafe relative paths, symlink components, and explicit .git components. Matching also rejects dot, parent, and .git names produced by glob expansion. Regression assertions cover external sentinels, committed configuration and symlinks, dangling leaf links, and glob-spelled traversal.
Resilience and Maintainability Implications
- inferred — Validation and later pathname-based operations are not atomic, so concurrent directory replacement can invalidate checks. Failures or interruption can leave partial deletion, and success logging indicates at least one successful removal rather than complete exclusion. These limitations predate the PR; no new caller, authority, or concurrent exposure was identified that materially worsens them.
-
Pre-merge checks: 
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely identifies the main change: confining directory exclusions around symlinks. It also includes the relevant vulnerability identifier. Docstring Coverage Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within survi… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Docstring Coverage
-
Explanation
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 3 functions across 3 files. (1 skipped: 1 unsupported.)
-
Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
I thump beside the worktree gate,
No symlink sends me past its place.
The leaf-link hops; its target stays,
Safe globs now clear the copied maze,
And carrot crumbs mark bounded ways.
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
A directory exclusion from
.gtrconfigcould expand through a preserved symlink and delete files outside the worktree duringgit gtr new.Expand exclusion globs one component at a time, refusing to descend through symlinks, parent aliases, or Git metadata. Check every component of the exclusion root before entering it. A matched leaf symlink is unlinked without touching its target, and shell options remain local to the exclusion pass.
Regression tests use disposable repositories and sentinel files, including worktree creation with committed configuration and a committed symlink. Copy settings continue to work without requiring trust approval.
Validation: focused BATS tests passed; ShellCheck and generated-completion checks passed; Bash 3.2 CLI create/list/go/remove smoke test passed. Full BATS suite passed (564 passed, 3 skipped; using isolated temporary Git configuration).
Tracking: VULN-565.
Summary by CodeRabbit
Bug Fixes
Documentation