fix(resolve-file-uri): explain project boundary restriction in rejection warning (fixes #3554)#3679
Open
MoerAI wants to merge 1 commit intocode-yeongyu:devfrom
Open
Conversation
…ion warning (fixes code-yeongyu#3554) Root cause: when a file:// prompt URI resolves outside the project root, resolvePromptAppend returns the warning '[WARNING: Path rejected: $URI]' with no indication of WHY the path was rejected. Issue code-yeongyu#3554 reports that this is confusing because the docs explicitly advertise support for absolute, home-relative, and cross-project file:// paths, yet the code intentionally restricts file:// prompt resolution to the project boundary (commit 9865978, security hardening). Fix: extend the warning message so it now includes the resolved project root and an explicit hint that file:// prompts must reside within the project boundary. The security restriction itself is preserved unchanged. Verification: added a regression test that asserts the rejection warning matches /outside project root/i. Test fails before the fix, passes after. Full resolve-file-uri.test.ts suite: 11 pass / 0 fail. typecheck clean.
Contributor
Author
|
I have read the CLA Document and I hereby sign the CLA |
There was a problem hiding this comment.
No issues found across 2 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Auto-approved: Low-risk diagnostic message improvement with a corresponding regression test; maintains existing security boundaries while clarifying behavior for users.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Make the
[WARNING: Path rejected: ...]placeholder explain why the path was rejected. The current message just echoes the URI and gives no hint thatfile://prompt loading is intentionally confined to the project boundary, which leaves users (correctly) confused given that the docs advertise broaderfile://path support.Root Cause
resolvePromptAppend()insrc/agents/builtin-agents/resolve-file-uri.tscallsisWithinProject(filePath, projectRoot)and, on rejection, returns[WARNING: Path rejected: ${promptAppend}]. There is no signal that:configDir ?? process.cwd(), anddocs/reference/configuration.mdanddocs/guide/agent-model-matching.md) describe absolute, relative, and~-homefile://paths.The boundary check itself was added intentionally for security (commit
98659783c0def8ecf60c4f6108c119a5f6435f73, "fix(security): confine file resolution to project roots"). This PR keeps that restriction; it only improves the diagnostic so users can self-correct without reading the source.Changes
src/agents/builtin-agents/resolve-file-uri.tsprojectRootand an explicit note thatfile://prompts must reside within the project boundary.src/agents/builtin-agents/resolve-file-uri.test.ts/outside project root/i.Reproduction (before fix)
With the regression test added but the source change reverted, the new test fails:
Verification (after fix)
Test
src/agents/builtin-agents/resolve-file-uri.test.ts(new caserejection warning explains the project boundary restriction (issue #3554)).rejects absolute file URI outside configDir,rejects traversal file URI that escapes configDir,rejects symlink file URI that escapes configDir,resolves home directory URI path) still match on the[WARNING: Path rejected:prefix and continue to pass.resolve-file-uri.test.ts: 11 pass / 0 fail.bun run typecheck: clean.Fixes #3554
Codesmith can help with this PR, just tag
@codesmithor enable autofix. Settings.Summary by cubic
Improves the
file://prompt rejection message to explain the project boundary restriction and show the resolved project root. Fixes #3554.resolvePromptAppend()warning to includeprojectRootand a note thatfile://prompts must stay within the project boundary.Written for commit 111b796. Summary will update on new commits. Review in cubic