Repository navigation
Conversation
 [Medium impact] Adds file-inclusion logic to instruction file processing. Not safe to merge until imports are prevented from disclosing private local text and from exhausting memory or stalling instruction loading. The fence-parsing and sibling-import issues are non-blocking. Findings
Fix with agent prompt### Issue 1
crates/jcode-base/src/prompt.rs:1002-1007
If a user opens an untrusted project whose AGENTS.md imports a readable private file outside the checkout, this code reads the file without a project-boundary check and places its contents in system-prompt instructions. This can disclose local private text to the model provider and must be fixed before merging.
**How this was verified:** An absolute import placed the contents of a synthetic outside-project file in the loaded system-prompt instructions.
### Issue 2
crates/jcode-base/src/prompt.rs:undefined-1007
If an `@path` line names a nonregular source such as `/dev/zero`, this code attempts to read it to completion without a file-type or byte limit. Under a memory limit, that read reached an out-of-memory error; a large regular file was also expanded without a size check. Import loading can therefore consume excessive memory or stall, and needs a bound before merging.
### Issue 3
crates/jcode-base/src/prompt.rs:995-996
A three-backtick line inside a four-backtick Markdown fence does not close the fence, but this code treats it as a closing marker. It expands an `@path` example inside the fence and leaves a later import outside it unexpanded. This non-blocking error changes which instructions are loaded.
### Issue 4
crates/jcode-base/src/prompt.rs:1012-1013
When two imported files each import `shared.md`, this code leaves the first expansion in `seen`. The second branch retains `@shared.md` instead of including its contents. This non-blocking error leaves expected instructions missing at that location.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.SummaryAdds standalone
The previously reported private-file disclosure and unbounded file reads remain unresolved and block merging. The fence-parsing and sibling-import issues also remain, but are non-blocking. Reviews (16) · Last reviewed commit: "feat(prompt): expand @path imports in AG..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| .and_then(|path| { | ||
| let canonical = std::fs::canonicalize(&path).ok()?; | ||
| if seen.contains(&canonical) { | ||
| return None; | ||
| } | ||
| let text = std::fs::read_to_string(&canonical).ok()?; |
There was a problem hiding this comment.
If a user opens an untrusted project whose AGENTS.md imports a readable private file outside the checkout, this code reads the file without a project-boundary check and places its contents in system-prompt instructions. This can disclose local private text to the model provider and must be fixed before merging.
How this was verified: An absolute import placed the contents of a synthetic outside-project file in the loaded system-prompt instructions.
Artifacts
Focused before-and-after execution script
- The script extracted and executed each revision’s Rust loader against the same synthetic private-file scenario, providing the reproducible check.
Parent revision Rust loader and fixture
- The generated Rust source contains the parent revision’s loader and the executed fixture, showing exactly what the before run compiled.
Parent revision output with private bytes absent
- The executed parent loader kept the absolute @path literal and reported that private bytes were absent from the loaded section.
PR revision Rust loader and fixture
- The generated Rust source contains the PR revision’s loader and the executed fixture, showing exactly what the after run compiled.
PR revision output with private bytes included
- The executed PR loader replaced the absolute @path with the synthetic private credential in the loaded section, confirming the leakage path.
- This script extracted the import functions from PR commit e07b665 and generated the original, guarded, and diagnostic Rust sources used in the runs.
Executed original import source
- This generated Rust source contains the PR commit’s import functions unchanged and was compiled for the original-behavior runs.
- This executed shell script applied the memory and time limits and captured each command, working directory, exit code, and output.
Original loader with /dev/zero
- The bounded original run exited 0 and kept the import line, showing that its failed device read was silent.
Device-read error under the memory limit
- The diagnostic run exposed `read_to_string(/dev/zero): out of memory`, showing what the original read site silently discarded.
- The generated guarded comparison rejected the nonregular target and kept the import line without attempting its read.
Original loader with a 2 MiB file
- The bounded original run expanded the file into 2,097,153 output bytes, demonstrating the missing size budget.
Guarded loader with a 2 MiB file
- The generated guarded comparison kept the import line instead of expanding the oversized file.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/jcode-base/src/prompt.rs
Line: 1001-1006
Comment:
**Private files enter prompts**
If a user opens an untrusted project whose AGENTS.md imports a readable private file outside the checkout, this code reads the file without a project-boundary check and places its contents in system-prompt instructions. This can disclose local private text to the model provider and must be fixed before merging.
**How this was verified:** An absolute import placed the contents of a synthetic outside-project file in the loaded system-prompt instructions.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| if seen.contains(&canonical) { | ||
| return None; | ||
| } | ||
| let text = std::fs::read_to_string(&canonical).ok()?; |
There was a problem hiding this comment.
If an @path line names a nonregular source such as /dev/zero, this code attempts to read it to completion without a file-type or byte limit. Under a memory limit, that read reached an out-of-memory error; a large regular file was also expanded without a size check. Import loading can therefore consume excessive memory or stall, and needs a bound before merging.
Artifacts
Focused before-and-after execution script
- The script extracted and executed each revision’s Rust loader against the same synthetic private-file scenario, providing the reproducible check.
Parent revision Rust loader and fixture
- The generated Rust source contains the parent revision’s loader and the executed fixture, showing exactly what the before run compiled.
Parent revision output with private bytes absent
- The executed parent loader kept the absolute @path literal and reported that private bytes were absent from the loaded section.
PR revision Rust loader and fixture
- The generated Rust source contains the PR revision’s loader and the executed fixture, showing exactly what the after run compiled.
PR revision output with private bytes included
- The executed PR loader replaced the absolute @path with the synthetic private credential in the loaded section, confirming the leakage path.
- This script extracted the import functions from PR commit e07b665 and generated the original, guarded, and diagnostic Rust sources used in the runs.
Executed original import source
- This generated Rust source contains the PR commit’s import functions unchanged and was compiled for the original-behavior runs.
- This executed shell script applied the memory and time limits and captured each command, working directory, exit code, and output.
Original loader with /dev/zero
- The bounded original run exited 0 and kept the import line, showing that its failed device read was silent.
Device-read error under the memory limit
- The diagnostic run exposed `read_to_string(/dev/zero): out of memory`, showing what the original read site silently discarded.
- The generated guarded comparison rejected the nonregular target and kept the import line without attempting its read.
Original loader with a 2 MiB file
- The bounded original run expanded the file into 2,097,153 output bytes, demonstrating the missing size budget.
Guarded loader with a 2 MiB file
- The generated guarded comparison kept the import line instead of expanding the oversized file.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/jcode-base/src/prompt.rs
Line: 1006
Comment:
**Imports can exhaust memory**
If an `@path` line names a nonregular source such as `/dev/zero`, this code attempts to read it to completion without a file-type or byte limit. Under a memory limit, that read reached an out-of-memory error; a large regular file was also expanded without a size check. Import loading can therefore consume excessive memory or stall, and needs a bound before merging.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| if trimmed.starts_with("```") || trimmed.starts_with("~~~") { | ||
| in_fence = !in_fence; |
There was a problem hiding this comment.
A three-backtick line inside a four-backtick Markdown fence does not close the fence, but this code treats it as a closing marker. It expands an @path example inside the fence and leaves a later import outside it unexpanded. This non-blocking error changes which instructions are loaded.
Artifacts
Focused four-backtick fence test script
- The executed script adds one temporary loader test, runs it before and after a temporary correction, and restores tracked files.
Test output with PR fence handling
- The focused test ran against the PR code and failed, showing the fenced import expanded and the later import left literal.
Test output with corrected fence handling
- The same test ran with temporary width-aware fence handling and passed, showing the fenced import preserved and the later import expanded.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/jcode-base/src/prompt.rs
Line: 994-995
Comment:
**Fence markers close incorrectly**
A three-backtick line inside a four-backtick Markdown fence does not close the fence, but this code treats it as a closing marker. It expands an `@path` example inside the fence and leaves a later import outside it unexpanded. This non-blocking error changes which instructions are loaded.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| seen.push(canonical.clone()); | ||
| let expanded = expand_agents_md_imports_inner(&text, &canonical, depth + 1, seen); |
There was a problem hiding this comment.
When two imported files each import shared.md, this code leaves the first expansion in seen. The second branch retains @shared.md instead of including its contents. This non-blocking error leaves expected instructions missing at that location.
Artifacts
Focused sibling-import test source
- This authored test creates two sibling imports of one shared file and requires both to expand.
Isolated before-and-after test command
- This authored command runs the test in a disposable worktree, applies the temporary correction for the second run, and removes the worktree.
Test output with the PR implementation
- The executed test failed because the second sibling retained `@shared.md` instead of expanding it.
Test output with the temporary stack-pop correction
- The same executed test passed with the shared rule expanded in both siblings.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/jcode-base/src/prompt.rs
Line: 1011-1012
Comment:
**Sibling imports look like cycles**
When two imported files each import `shared.md`, this code leaves the first expansion in `seen`. The second branch retains `@shared.md` instead of including its contents. This non-blocking error leaves expected instructions missing at that location.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Comments Outside DiffThese findings could not be posted inline.
|
892ceb2 to
07c1a1f
Compare
2119793 to
66499fe
Compare
66499fe to
ec4fc4f
Compare
A line whose trimmed text is exactly @<path> is replaced by that file's contents, matching the Claude Code convention, so a shared governance file referenced from ~/AGENTS.md or a project AGENTS.md actually loads. - absolute, ~/, and importer-relative paths - fenced code blocks and inline mentions are left alone - nesting capped at 5, cycles skipped - unreadable targets keep the original line, nothing dropped silently - expansion happens at load, so the per-session snapshot stays stable
ec4fc4f to
03cfd87
Compare
1 participant
Closes #1604
What
load_agents_md_files_from_dirsnow expands@pathimport lines, matching the Claude Code convention. A line whose trimmed text is exactly@<path>is replaced by the file's contents.~/, or relative to the importing file.Tests
Three new tests in
prompt_tests.rs:agents_md_expands_at_path_imports: absolute and relative imports, order preserved.agents_md_imports_keep_missing_fenced_and_inline_lines: missing target, fenced block, inline mention, and a line with a space all stay verbatim.agents_md_imports_stop_on_cycles: a→b→a and a self-import each load once.cargo test -p jcode-base --lib prompt: 67 passed.cargo fmt --checkclean.cargo clippy -p jcode-base --lib --tests -D warningsreports two errors, injev.rs:684andtranscript_sample.rs:176. Both are identical on unmodified master, so they are not from this change.Runtime check
A project
AGENTS.mdcontaining@/abs/path/rule.md, whererule.mdholds a secret word. Same prompt to both binaries:selfdev, private socket): answered with the secret word.@/…/rule.md. Jcode didn't pull in that file's contents."Tradeoff
A line that is only
@somethingand resolves to a real file is now treated as an import. That matches Claude Code, and the no-whitespace and file-must-exist rules keep false positives narrow.