Skip to content

Add path validation and logging to template rendering JavaScript#16137

Closed
Copilot wants to merge 5 commits intomainfrom
copilot/fix-template-rendering-security
Closed

Add path validation and logging to template rendering JavaScript#16137
Copilot wants to merge 5 commits intomainfrom
copilot/fix-template-rendering-security

Conversation

Copy link
Contributor

Copilot AI commented Feb 16, 2026 •
edited
Loading

Security Review: Template Rendering JavaScript Code

Analysis Complete ✅

Reviewed template rendering code in actions/setup/js/ for security issues related to file path handling.

Security Issues Identified ✅

  • render_template.cjs: No path validation on GH_AW_PROMPT env var before fs operations
  • interpolate_prompt.cjs: No path validation on GH_AW_PROMPT env var before fs operations
  • substitute_placeholders.cjs: No path validation on file parameter before fs operations
  • file_helpers.cjs: Uses path.join() without normalizing paths, potential directory traversal
  • runtime_import.cjs: Has security checks but lacks detailed logging

Fixes Implemented ✅

  • Created path_helpers.cjs with comprehensive path validation helpers:
    • validateAndNormalizePath() - Validates and normalizes file paths
    • validatePathWithinBase() - Ensures paths stay within base directory
    • validateDirectory() - Validates directory existence and type
    • safeJoin() - Safely joins and normalizes path segments
  • Applied path validation to all file operations in render_template.cjs
  • Applied path validation to all file operations in interpolate_prompt.cjs
  • Applied path validation to substitute_placeholders.cjs
  • Improved path handling in file_helpers.cjs with normalization
  • Added comprehensive logging to all template rendering operations
  • Added 31 tests for path traversal attack prevention
  • All tests passing (4168 passed, 22 skipped)
  • Code formatted and linted successfully
  • Fixed github-script compatibility - Added typeof core !== "undefined" checks to ALL modified files

Security Features

Path Validation:

  • All file paths validated using path.normalize() and path.resolve()
  • Protection against directory traversal attacks (../ patterns)
  • Protection against null byte injection
  • Protection against absolute path escapes

Logging:

  • Detailed logging in all template rendering operations (when core is available)
  • Path validation steps logged with [moduleName] prefixes
  • Security violations logged with context before throwing errors
  • File operations logged with sizes and status messages
  • All logging calls protected with typeof core !== "undefined" checks

Testing

  • 31 new tests for path_helpers module covering:
    • Path normalization and validation
    • Directory traversal prevention
    • Null byte injection prevention
    • Windows/Unix path handling
    • Security violation scenarios
  • All existing tests pass with new security features
  • Test mocks updated to support new logging patterns
  • Works in both test environment and github-script action context
Original prompt

Do a deep review of the template rendering JavaScript code and fix security issues associated to incorrect file path solution (make sure to clean/normalize using path module).

Add logging.

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 2 commits February 16, 2026 16:04
@pelikhan
Add path validation and logging to template rendering JavaScript code
d8ba064 
- Created path_helpers.cjs with secure path validation functions
- Updated render_template.cjs with path validation and detailed logging
- Updated interpolate_prompt.cjs with path validation and logging
- Updated substitute_placeholders.cjs with path validation and logging
- Updated file_helpers.cjs to use safeJoin and add logging
- Enhanced runtime_import.cjs with comprehensive logging
- Added comprehensive tests for path_helpers module
- Fixed test mocks to support new logging calls

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
@pelikhan
Run prettier formatting on modified JavaScript files
3d971b9 
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix security issues in template rendering JavaScript code Add path validation and logging to template rendering JavaScript Feb 16, 2026
Copilot AI requested a review from pelikhan February 16, 2026 16:10
@pelikhan pelikhan added the smoke label Feb 16, 2026
@github-actions
Copy link
Contributor

github-actions bot commented Feb 16, 2026

🧪 Smoke Temporary ID is now testing temporary ID functionality...

@github-actions
Copy link
Contributor

github-actions bot commented Feb 16, 2026

🧪 Smoke Project is now testing project operations...

@github-actions
Copy link
Contributor

github-actions bot commented Feb 16, 2026 •
edited
Loading

💫 TO BE CONTINUED... Smoke Claude failed! Our hero faces unexpected challenges...

@github-actions
Copy link
Contributor

github-actions bot commented Feb 16, 2026 •
edited
Loading

🌑 The shadows whisper... Smoke Codex failed. The oracle requires further meditation...

@github-actions
Copy link
Contributor

github-actions bot commented Feb 16, 2026

📰 BREAKING: Smoke Copilot is now investigating this pull request. Sources say the story is developing...

@github-actions
Copy link
Contributor

github-actions bot commented Feb 16, 2026 •
edited
Loading

❌ Smoke Copilot SDK failed. Please review the logs for details.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 16, 2026

❌ Smoke Temporary ID encountered failures. Check the logs for details.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 16, 2026

❌ Smoke Project encountered failures. Check the logs for details.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 16, 2026

📰 DEVELOPING STORY: Smoke Copilot reports failed. Our correspondents are investigating the incident...

@pelikhan
Copy link
Contributor

pelikhan commented Feb 16, 2026

@copilot fix compilation issue. Make sure the code is meant to run in actions/github-script

See https://github.com/github/gh-aw/actions/runs/22069956599/job/63771664130#step:34:1

@pelikhan
Fix path_helpers.cjs to work in github-script context
29e4202 
- Add typeof core !== "undefined" checks before using core methods
- Prevents ReferenceError when core is not available globally
- Follows pattern used in other modules like sanitize_content.cjs

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Feb 16, 2026

@copilot fix compilation issue. Make sure the code is meant to run in actions/github-script

See https://github.com/github/gh-aw/actions/runs/22069956599/job/63771664130#step:34:1

Fixed in commit 29e4202. Added typeof core !== "undefined" checks before all core.info() and core.warning() calls in path_helpers.cjs to ensure compatibility with github-script action context. This follows the same pattern used in other modules like sanitize_content.cjs.

Copilot AI requested a review from pelikhan February 16, 2026 16:33
@pelikhan pelikhan added smoke and removed smoke labels Feb 16, 2026
@github-actions
Copy link
Contributor

github-actions bot commented Feb 16, 2026 •
edited
Loading

🌑 The shadows whisper... Smoke Codex failed. The oracle requires further meditation...

@github-actions
Copy link
Contributor

github-actions bot commented Feb 16, 2026

🧪 Smoke Project is now testing project operations...

@github-actions
Copy link
Contributor

github-actions bot commented Feb 16, 2026

🧪 Smoke Temporary ID is now testing temporary ID functionality...

@github-actions
Copy link
Contributor

github-actions bot commented Feb 16, 2026

📰 BREAKING: Smoke Copilot is now investigating this pull request. Sources say the story is developing...

@github-actions
Copy link
Contributor

github-actions bot commented Feb 16, 2026 •
edited
Loading

💫 TO BE CONTINUED... Smoke Claude failed! Our hero faces unexpected challenges...

@github-actions
Copy link
Contributor

github-actions bot commented Feb 16, 2026 •
edited
Loading

❌ Smoke Copilot SDK failed. Please review the logs for details.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 16, 2026

❌ Smoke Temporary ID encountered failures. Check the logs for details.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 16, 2026

❌ Smoke Project encountered failures. Check the logs for details.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 16, 2026

📰 DEVELOPING STORY: Smoke Copilot reports failed. Our correspondents are investigating the incident...

@pelikhan pelikhan removed the smoke label Feb 16, 2026
@pelikhan
Copy link
Contributor

pelikhan commented Feb 16, 2026

@copilot the global object are not setup in substitute step

https://github.com/github/gh-aw/actions/runs/22069956599/job/63771664130#step:34:1

@pelikhan
Add typeof core checks to all modified template rendering files
9280171 
- Fixed substitute_placeholders.cjs to check for core availability
- Fixed render_template.cjs to check for core availability
- Fixed interpolate_prompt.cjs to check for core availability
- Fixed file_helpers.cjs to check for core availability
- Fixed runtime_import.cjs to check for core availability
- All logging calls now wrapped with typeof core !== "undefined"
- Ensures compatibility when core global is not yet initialized

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Feb 16, 2026

@copilot the global object are not setup in substitute step

https://github.com/github/gh-aw/actions/runs/22069956599/job/63771664130#step:34:1

Fixed in commit 9280171. Added typeof core !== "undefined" checks to all files modified for path validation:

  • substitute_placeholders.cjs
  • render_template.cjs
  • interpolate_prompt.cjs
  • file_helpers.cjs
  • runtime_import.cjs

All logging calls are now wrapped to ensure compatibility when core global is not yet initialized by github-script.

@pelikhan
Copy link
Contributor

pelikhan commented Feb 16, 2026

@copilot revert previous commit and setup globals in the generate step code like all the other GitHub script steps

@pelikhan pelikhan closed this Feb 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pelikhan pelikhan Awaiting requested review from pelikhan

Assignees

@pelikhan pelikhan

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@pelikhan