Add security skill - #64238
Conversation
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Several security guarantees are inaccurate or overly broad and should be corrected before publication.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Balanced
Findings: 4
New issues introduced by this change (4)
| Severity | Finding |
|---|---|
.github/skills/security-report-check/SKILL.md — Account for non-emission filesystem side effects |
|
.github/skills/security-report-check/SKILL.md — Do not guarantee normal unwinding for fatal exits |
|
.github/skills/security-report-check/SKILL.md — Correct the spelling of adversarially |
|
.github/skills/security-report-check/SKILL.md — Scope the workspace-trust claim to editor integrations |
What changed in this PR
Adds security guidance for evaluating reports involving untrusted TypeScript input.
Changes:
- Defines compiler security guarantees and non-guarantees.
- Documents content-mapper and language-service trust boundaries.
- Provides examples of valid and invalid security reports.
| File | Description |
|---|---|
.github/skills/security-report-check/SKILL.md |
Adds security-report assessment guidance. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
Daniel Rosenwasser (DanielRosenwasser)
left a comment
There was a problem hiding this comment.
Had a few nits I'd like but good as-is if you want.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Daniel Rosenwasser <DanielRosenwasser@users.noreply.github.com>
Co-authored-by: Daniel Rosenwasser <DanielRosenwasser@users.noreply.github.com>
Co-authored-by: Daniel Rosenwasser <DanielRosenwasser@users.noreply.github.com>
Co-authored-by: Daniel Rosenwasser <DanielRosenwasser@users.noreply.github.com>
Co-authored-by: Daniel Rosenwasser <DanielRosenwasser@users.noreply.github.com>
Co-authored-by: Daniel Rosenwasser <DanielRosenwasser@users.noreply.github.com>
Co-authored-by: Daniel Rosenwasser <DanielRosenwasser@users.noreply.github.com>
Co-authored-by: Daniel Rosenwasser <DanielRosenwasser@users.noreply.github.com>
Co-authored-by: Daniel Rosenwasser <DanielRosenwasser@users.noreply.github.com>
Co-authored-by: Daniel Rosenwasser <DanielRosenwasser@users.noreply.github.com>
Co-authored-by: Daniel Rosenwasser <DanielRosenwasser@users.noreply.github.com>
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
Several security guarantees inaccurately describe crash handling, build-info authority, and workspace-trust enforcement.
Review tier: Balanced
Findings: None
Issues resolved since last review (4)
| Severity | Finding |
|---|---|
.github/skills/security-report-check/SKILL.md — Scope the workspace-trust claim to editor integrations View resolved comment |
|
.github/skills/security-report-check/SKILL.md — Correct the spelling of adversarially View resolved comment |
|
.github/skills/security-report-check/SKILL.md — Do not guarantee normal unwinding for fatal exits View resolved comment |
|
.github/skills/security-report-check/SKILL.md — Account for non-emission filesystem side effects View resolved comment |
Previously missed findings (2)
In code that hasn't changed since last review
.github/skills/security-report-check/SKILL.md:47
--build --cleanderives deletion candidates from the parsed configuration viaParsedCommandLine.GetOutputFileNames;cleanProjectdoes not read paths from the.tsbuildinfocontents (tsc/internal/execute/build/buildtask.go:793-815). A crafted build-info file therefore cannot redirect this operation, so this wording would incorrectly reject such a report without investigation.
.github/skills/security-report-check/SKILL.md:80- A crafted build-info file can supply cached file paths and influence incremental decisions, but emit and clean output paths come from the current parsed compiler configuration. Claiming that
.tsbuildinfoalone enables arbitrary writes overstates its authority and could cause valid reports about a new write primitive to be dismissed.

Try to inform "security researchers" (agents) that tsc taking a while to run is not a security boundary, etc