fix(roots): expand leading tilde before cwd join in resolveAllowedPath

[rome-xi]

Expanding leading-tilde paths (~/...) before making them absolute ensures resolveAllowedPath resolves against the user's home directory rather than joining as a literal tilde segment under cwd. This fixes open_workspace calls using home-relative paths (e.g. from ChatGPT).

Fixes #369

Summary by CodeRabbit

• Bug Fixes
  • Paths beginning with ~ now resolve from the home directory before access is checked, rather than being treated as paths beneath the working directory.
  • Relative paths continue to resolve against the working directory. Paths outside the allowed locations, including those reached through parent-directory traversal, are rejected.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 2a51c2b8-fda5-4cf9-93f8-29247b7492f4

📥 Commits

Reviewing files that changed from the base of the PR and between 531d3f9 and 2fe233c.

📒 Files selected for processing (2)

• src/roots.test.ts
• src/roots.ts

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.

---

📝 Walkthrough

Walkthrough

resolveAllowedPath now expands leading ~, ~/, and ~\ before path resolution and allowed-root checks. Tests cover home paths, differing working directories, ordinary relative paths, and parent-directory traversal.

Changes

Allowed-Root Path Resolution

Layer / File(s)	Summary
Home expansion and containment checks src/roots.ts, src/roots.test.ts	resolveAllowedPath expands leading-tilde paths before resolution. Tests cover paths within and outside allowed roots, relative paths, and parent-directory traversal.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: waishnav

Merge Risk: ⚪ Minimal · up to 2fe23

Home-relative workspace paths should now resolve against the user’s home directory while remaining subject to allowed-root checks. No actionable merge risk is established.

Architecture Summary

Architecture risk: 🔵 Low · up to 2fe23

The change affects 1 system.

Changed systems: src

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

• observed — src (service) was modified; 2 changed files map to changed impact.

Before / after behavior

• observed — Modified behavior in src/roots.test.ts: Added assertions that home-relative paths resolve beneath the home directory and are accepted when the expanded path is within the allowed root, including when the working directory differs.
• observed — Modified behavior in src/roots.test.ts: Added rejection assertions for home-relative paths outside the allowed root and replaced the prior expectation that "~/file.txt" resolves beneath "/workspace" with an expectation that it is rejected as outside that root.
• observed — Modified behavior in src/roots.test.ts: Added assertions that relative paths resolve against the working directory when within the allowed root, while a path using .. to escape that root is rejected.
• observed — Modified behavior in src/roots.ts: resolveAllowedPath now expands ~, ~/, and ~\ before resolution. Expanded absolute paths are resolved directly, while relative paths are resolved against cwd; previously all inputs were resolved against cwd without home expansion.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: expanding a leading tilde before resolving the path against the current working directory.
Linked Issues check	✅ Passed	Issue #369 requires leading-tilde expansion before absolute normalization and allowed-root checks. src/roots.ts expands ~, ~/..., and ~\\... before resolveAllowedPath chooses the absolute or …
Out of Scope Changes check	✅ Passed	The reviewed changes are limited to src/roots.ts and src/roots.test.ts. They implement and test the path-resolution and containment behavior required by issue #369. No unrelated production behavio…

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

🧪 Generate unit tests (beta)

• Create a new PR

🛠️ Fix failing CI checks 💡

• Commit to this branch
• Create a new PR

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{A rabbit checks the paths at night
And turns a tilde homeward right
The allowed roots stand firm and clear
No wandering parent slips past here
Then carrots mark the tests complete}

_{Comment @coderabbitai help to get the list of available commands.}

[greptile-apps]

Confidence Score: 4/5

[Medium risk] Changes path resolution logic for tilde expansion.

The confirmed test reliability issue is non-blocking; the change is safe to merge.

Findings

1.  Rejection test depends on HOME ▶

Summary

The PR expands leading-tilde paths before checking them against allowed roots and adds path-resolution tests. One new rejection test fails when the test runner’s home directory is /workspace or beneath it, even though path resolution is correct.

_{Reviews (1) · Last reviewed commit: "fix(roots): expand leading tilde before ..."}

[greptile-apps]

Rejection test depends on HOME

If the test runner’s home directory is /workspace or beneath it, ~/file.txt is inside the allowed root. The helper correctly accepts it, but this assertion fails. Use an allowed root that excludes the file, such as join(home, "personal"). This is a non-blocking test reliability issue.

Suggested change

	() => resolveAllowedPath("~/file.txt", "/workspace", ["/workspace"]),
	() => resolveAllowedPath("~/file.txt", "/workspace", [join(home, "personal")]),

Artifacts

Command used to run and capture the HOME boundary checks

• This executed shell command ran the actual test and focused reproduction under three HOME values, capturing each command and exit code.

Focused executable source for the allowed-root check

• This executed TypeScript source calls the changed helper with the original and proposed roots, showing their outcomes.

Actual root test passes with HOME outside /workspace

• Running `src/roots.test.ts` with `HOME=/home/user` exited 0, establishing the passing comparison.

Actual root test fails with HOME=/workspace

• Running the same test with `HOME=/workspace` exited 1 with a missing-expected-exception error at line 43, confirming the finding.

Focused path check with HOME outside /workspace

• The direct helper call rejected `~/file.txt` under the original `/workspace` root when HOME was `/home/user`.

Focused path check with HOME=/workspace

• The direct helper call accepted `~/file.txt` under `/workspace` but rejected it under `join(home, 'personal')`, confirming the proposed root avoids this failure.

Focused path check with HOME below /workspace

• With `HOME=/workspace/child`, the original root accepted `~/file.txt` while the proposed root rejected it.

Captured command and executed reproduction source

• A command captured the shell runner and TypeScript reproduction source used for the checks, making the execution inspectable.

_{Ran code and verified through T-Rex}

[greptile-apps]

Comments Outside Diff

These findings sit on lines the diff does not cover, so they could not be posted inline. Each one leaves this list once its file changes.

•  Allowed-root assertion depends on HOME being outside /workspace ▶

  • Bug
    • With HOME set to /workspace or /workspace/child, the test expects an exception for a path that is actually within /workspace.
  • Cause
    • resolveAllowedPath expands ~/file.txt against HOME before checking whether the resulting absolute path is inside the allowed root.
  • Fix
    • Use join(home, 'personal') as the allowed root for this rejection assertion; ~/file.txt is outside that sibling directory for the HOME values tested.