Harden temp writes in safe-output evaluation script#43054
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot inline script code into step in objective-impact-report and remove .js file. |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| EXPR_GITHUB_REPOSITORY: ${{ github.repository }} | ||
| run: bash scripts/prepare-objective-impact-report-dataset.sh | ||
| run: | |
There was a problem hiding this comment.
@copilot use actions/github-script instead of shell
There was a problem hiding this comment.
Addressed in cabf5fc: the safe-output preparation step now uses actions/github-script instead of a shell heredoc, and I removed the redundant explicit github-token input.
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Addressed in |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Triage Result
Batch: pr-batch:security-hardening (with 43057) Rationale: CodeQL-flagged insecure temp file creation in active evaluation script. Behavior-preserving fix. Draft state and dirty merge state need resolution before merge.
|
|
@copilot merge main and recompile |
There was a problem hiding this comment.
Pull request overview
This PR addresses a CodeQL finding around insecure temporary file creation in the safe-output evaluation generation logic used by the objective-impact-report workflow. It preserves the existing behavior while switching the atomic-write implementation to use a private mkdtempSync(...) directory and renameSync(...) finalization.
Changes:
- Removed
scripts/prepare-objective-impact-safe-output-evaluations.cjsand moved the evaluation logic into the workflow as anactions/github-scriptpre-agent step. - Replaced ad hoc temp-file naming with
fs.mkdtempSync(...)+renameSync(...), and improved error diagnostics by splitting write vs rename failures. - Updated the dataset prep script to stop invoking the removed Node script; updated the compiled
.lock.ymlaccordingly.
Show a summary per file
| File | Description |
|---|---|
| scripts/prepare-objective-impact-safe-output-evaluations.cjs | Removed standalone safe-output evaluation script (logic moved into workflow step). |
| scripts/prepare-objective-impact-report-dataset.sh | Removed call to the deleted safe-output evaluation script. |
| .github/workflows/objective-impact-report.md | Added a pre-agent actions/github-script step that generates safe-output issue evaluation outputs using hardened temp writes. |
| .github/workflows/objective-impact-report.lock.yml | Regenerated compiled workflow to include the new pre-agent step and updated metadata hashes. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 4/4 changed files
- Comments generated: 0
- Review effort level: Low
…nce-fix-insecure-temp-file # Conflicts: # .github/workflows/objective-impact-report.lock.yml Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Addressed in |
|
@copilot please run the
|
|
🎉 This pull request is included in a new release. Release: |
CodeQL flagged insecure temporary file creation in
scripts/prepare-objective-impact-safe-output-evaluations.cjs. During review, this script was confirmed to be live code used by the activeobjective-impact-reportworkflow, so the fix keeps the behavior and removes the unsafe temp-file pattern.Security fix
fs.mkdtempSync(...).renameSync, avoiding predictable temp paths and reducing TOCTOU/symlink risk.Script cleanup
cryptoimport.Failure diagnostics