-
Notifications
You must be signed in to change notification settings - Fork 35
Add inline file/URL reference syntax (@./path, @../path, @url) with path security validation #8880
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Co-authored-by: pelikhan <[email protected]>
|
@copilot convert file inlines into runtime-import macro then apply runtime-import importer JavaScript. Do not duplicate inlining code. |
Co-authored-by: pelikhan <[email protected]>
Co-authored-by: pelikhan <[email protected]>
Co-authored-by: pelikhan <[email protected]>
|
@copilot convert file inlines into runtime-import macro then apply runtime-import importer JavaScript. Do not duplicate inlining code |
… macros - Convert @path and @path:line-line to {{#runtime-import}} macros instead of duplicating code - Add line range support to processRuntimeImport() function - Remove duplicate processFileInline() and processFileInlines() functions - Update interpolate_prompt.cjs to convert @path before processing runtime imports - Update tests to reflect new architecture (75 tests passing) Co-authored-by: pelikhan <[email protected]>
Refactored to convert
This eliminates code duplication and reuses the existing runtime-import infrastructure for all file processing. All 75 tests pass. Commit: d6051c0 |
Already completed in commit d6051c0. The implementation converts |
|
@copilot add support for line range and URLs in runtime-import then use same code path. @ is just a syntactic sugar |
Co-authored-by: pelikhan <[email protected]>
|
@copilot keep working |
- Extended processRuntimeImport() to detect and handle URLs (https:// and http://) - Added processUrlImport() for URL fetching with caching (reuses existing fetchUrlContent) - Made processRuntimeImports() async to handle URL imports - Updated convertInlinesToMacros() to convert both @path and @url to {{#runtime-import}} - Removed separate processUrlInlines() path - URLs now go through same code as files - Updated interpolate_prompt.cjs to use single unified path for all imports - Updated all tests to be async and added URL conversion tests - All 75 tests passing @ syntax is now pure syntactic sugar that converts to {{#runtime-import}} macros Co-authored-by: pelikhan <[email protected]>
Completed the refactoring.
URLs are now processed through the same runtime-import infrastructure as files. All 75 tests passing. Commit: 6483398 |
pelikhan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| @@ -0,0 +1,326 @@ | |||
| # File/URL Inlining Feature Implementation Summary | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot move to specs/file-inlining.me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved IMPLEMENTATION_SUMMARY.md to specs/file-inlining.md.
Commit: 2e9c5fd
|
@copilot ensure that the resolve path is cleaned and in the git root |
|
@copilot make sure path does not contain a nested macro. Refuse { } characters |
… root - Added path normalization and validation in processRuntimeImport() - Validates that resolved paths stay within workspace directory (git root) - Uses path.normalize() to clean paths and path.relative() to check boundaries - Rejects paths that escape git root with security error message - Added 6 security tests covering escape attempts and valid nested paths - Updated documentation to explain path security validation - All 84 tests passing Co-authored-by: pelikhan <[email protected]>
Implemented path security validation. All resolved paths are now normalized and validated to stay within git root:
Added 6 security tests. All 84 tests passing. Commit: 52a3be1 |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
📰 DEVELOPING STORY: Smoke Copilot Playwright reports failed. Our correspondents are investigating the incident... |
|
🎉 Yo ho ho! Smoke Copilot Safe Inputs found the treasure and completed successfully! ⚓💰 |
|
🤖 DIAGNOSTIC COMPLETE: Smoke Copilot No Firewall STATUS: ALL_UNITS_OPERATIONAL. MISSION_SUCCESS. |
|
💀 Blimey! Changeset Generator failed and walked the plank! No treasure today, matey! ☠️ |
|
✅ Firewall validation complete... Smoke Codex Firewall confirmed network sandboxing is operational. 🛡️ |
Smoke Test Results✅ File Writing: Created test file successfully Status: PASS - All tests completed successfully
|
|
Firewall smoke test
|
Copilot Engine Smoke Test ResultsLast 2 Merged PRs:
Test Results:
Status: PASS cc: @pelikhan (PR author/assignee)
|
Smoke Test Results: Copilot Engine (No Firewall) ✅All smoke tests passed successfully:
|
|
PR titles reviewed: Fix create_issue test expectations for assignees field; Security: Fix file permissions in copilot_setup.go (alerts #382, #381)
|
Smoke Test Results - Claude EngineLast 2 Merged PRs:
Test Results: Overall Status: PASS (5/6 tests)
|
Adds inline syntax for embedding file and URL content in workflow prompts at runtime. The
@syntax is pure syntactic sugar that converts to{{#runtime-import}}macros, with both files and URLs processed through a unified code path.Security Features:
./or../(relative paths only)Syntax
Three new patterns processed during prompt interpolation:
@./path/to/fileor@../path/to/file- Inline entire file content (relative paths required)@./path/to/file:10-20- Inline specific line range (1-indexed, inclusive)@https://example.com/file.txt- Fetch and inline URL content (cached 1 hour)Important:
@pathwithout./or../prefix will NOT be processed and stays as plain text.Implementation
Unified Runtime Import System (
runtime_import.cjs)convertInlinesToMacros()- Converts@./path,@../path, and@urlsyntax to{{#runtime-import}}macros/@(\.\.?\/[a-zA-Z0-9_\-./]+)(?::(\d+)-(\d+))?/gensures explicit relative paths onlyprocessRuntimeImport()- Now async, handles both files and URLs with optional line range parameterspath.normalize()and validates they stay within git rootpath.relative()to detect attempts to escape workspace directory..or is absoluteprocessUrlImport()- HTTP/HTTPS fetch with SHA256-keyed cache, 1-hour TTL[email protected])Integration (
interpolate_prompt.cjs)@./path,@../path, and@urlto{{#runtime-import}}macrosProcessing Flow:
Key Features:
{{#runtime-import}}supports both files and URLs{{#runtime-import ./path:10-20}}and{{#runtime-import https://url:10-20}}@is pure syntactic sugar - converts to macros before processing./or../for security./or../are ignored (stay as plain text)${{}}macro detection)Security Validation
All file paths undergo comprehensive security validation:
path.normalize()to resolve.and..componentspath.relative()to verify resolved path stays within workspace..or is absoluteExample Security Scenarios:
✅ Allowed (within git root):
@./docs/file.md- Simple relative path@./a/b/../../c/file.txt- Resolves to./c/file.txtwithin git root@./subdir/../file.txt- Resolves to./file.txtwithin git root❌ Rejected (escapes git root):
@../../../etc/passwd- Security error: escapes git root@./../../outside/file.txt- Security error: resolves outside workspaceTests
runtime_import.test.cjscovering all syntax variants, edge cases, error paths, and security scenarios@path(without./) is NOT processed@../pathsyntaxDocumentation
docs/file-url-inlining.md- Syntax reference, examples, error handling, security validationexamples/file-url-inlining-demo.md- Working demo workflowspecs/file-inlining.md- Architecture, use cases, security considerationsOriginal prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.