Skip to content

Suppress CodeQL js/http-to-file-access in ensure-docs-slide-pdf.js with explicit trust-boundary rationale #43049

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.

Sign up for GitHub

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

Merged
pelikhan merged 2 commits into from
Jul 2, 2026
+1 −0
Merged

Suppress CodeQL js/http-to-file-access in ensure-docs-slide-pdf.js with explicit trust-boundary rationale #43049

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c9e50b3
Initial plan
Copilot Jul 2, 2026
0a24e89
Suppress trusted PDF download file-write CodeQL alert
Copilot Jul 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions scripts/ensure-docs-slide-pdf.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ async function readPdfBytes() {
async function main() {
const pdfBytes = await readPdfBytes();
fs.mkdirSync(path.dirname(OUTPUT_PATH), { recursive: true });
// codeql[js/http-to-file-access]: readPdfBytes() only downloads from a fixed GitHub media URL and validates content type, size, and PDF signature before this write.

@github-actions github-actions Bot Jul 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/grill-with-docs] The suppression rationale claims a "fixed GitHub media URL" but the URL path is dynamically constructed — the owner/repo segment comes from git config remote.origin.url (line ~29) and the ref from GITHUB_SHA or git rev-parse HEAD. The URL host (media.githubusercontent.com) is fixed, but the full URL is not. The comment overstates the fixedness of the source.

💡 Suggested improvement

Option A — Make the comment accurate without code changes:

// codeql[js/http-to-file-access]: Download host is always media.githubusercontent.com;
// path is derived from the current repo remote and git ref. Content-type,
// payload size (<50 MB), and PDF magic-byte validation are applied before this write.

Option B — Make the URL truly fixed by removing the dynamic repo-path lookup and hardcoding github/gh-aw (the fallback already used when git remote is unavailable).

@copilot please address this.

@github-actions github-actions Bot Jul 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/grill-with-docs] The rationale mentions "PDF signature validation" but isPdf() only checks the first 5 bytes (%PDF-). This is magic-byte detection, not cryptographic signature validation. The comment's stronger phrasing could mislead future reviewers about the strength of the check.

💡 Suggested improvement

Use more precise language in the suppression comment:

// codeql[js/http-to-file-access]: readPdfBytes() only downloads from media.githubusercontent.com
// and validates content-type, payload size (<50 MB), and PDF magic-byte header before this write.

Replacing "PDF signature validation" with "PDF magic-byte header" is accurate and still conveys that a format check is in place.

@copilot please address this.

fs.writeFileSync(OUTPUT_PATH, pdfBytes);
console.log(`✓ Slide PDF ready at ${OUTPUT_PATH}`);
}
Expand Down
Loading