microsoft / microsoft/Build-CLI
Security review: 17 findings (3 High, 7 Medium, 5 Low, 2 Info) — fix PR in #32
Nobody has claimed this yet.
- Dominant language
- TypeScript
- Stars
- 293
- Forks
- 85
- PR merge metrics
- No merged PRs in 30d
Description
Summary
While starting to use Build-CLI in another project, I did a two-pass security review of the CLI (@microsoft/events-cli@0.2.0), the microsoft-build skill, the plugin manifests, and the CI / workflow surface. The review surfaced 17 findings — 3 High, 7 Medium, 5 Low, 2 Info — across resource exhaustion, npm supply-chain risk, indirect prompt injection, terminal-escape injection, untrusted JSON deserialization, open-redirect, floating-tag CI pinning, and local input-validation gaps.
A fix PR is already up — #32 — closing every actionable finding with zero new runtime dependencies, bumping CLI 0.2.0 → 0.3.0, plugin manifest 1.0.1 → 1.0.2, skill frontmatter 0.4 → 0.5, and adding ~66 new test cases (104 total, all green). Three Info findings are documented and explicitly accepted as non-defects.
Why an issue alongside the PR?
CONTRIBUTING.md and the PR template both suggest opening an issue (or discussion) for larger changes so direction can be aligned before code lands. The PR was drafted before I read that guidance — this issue captures the same scope for tracking/discussion. I'm happy to split the PR into smaller pieces, defer parts, or rework the approach if the maintainers prefer.
Findings overview
| Sev | ID | Area | One-liner |
|---|---|---|---|
| High | H1 | CLI / fetch | fetch() had no timeout — slow upstream hangs the CLI (and any agent waiting on it) |
| High | H2 | CLI / fetch | response.json() had no size cap — multi-GB payload exhausts memory |
| High | H3 | Skill / supply chain | npx -y @microsoft/events-cli unpinned and auto-consented across all 15 examples in SKILL.md |
| Med | M1 | CLI / fetch | No Content-Type check before JSON parse |
| Med | M2 | CLI / IO | as CacheMeta / as Session[] casts instead of runtime validation |
| Med | M3 | CLI / output | Catalog text written to TTY without ANSI/OSC/control-char stripping (clipboard-write via OSC 52) |
| Med | M4 | CI | Workflows used floating @v4 tags instead of commit SHAs |
| Med | M5 | CLI | parseInt(--limit) accepted negatives, NaN, multi-billion values |
| Med | M6 | Skill | Catalog content flowed into agent reasoning as authoritative facts (indirect prompt injection vector) |
| Med | M7 | CLI / fetch | Followed HTTP redirects without origin validation |
| Low | L1 | CLI / cache | Cache writes were not atomic |
| Low | L2 | repo | package.json ↔ package-lock.json version drift |
| Low | L3 | CLI / cache | JSON parse failures on cache files silently swallowed |
| Low | L4 | CLI | --limit accepted unbounded values (subsumed into M5 fix) |
| Low | L5 | CLI / cache | Tampered nextCheckAt could suppress revalidation indefinitely |
| Info | I1 | CLI | as casts paired with M2 fix; one remaining cast documented as safe |
| Info | I2 | CLI | 'displayValue' in field walked the prototype chain |
Full per-finding write-up with file/line refs, CWE mappings, mitigations, and historical supply-chain precedent for H3 lives in docs/security-review-2026-05-20.md on the PR branch.
A note on SECURITY.md
SECURITY.md directs vulnerability reports through aka.ms/SECURITY.md rather than public issues. The judgement call here was that none of these are directly-exploitable zero-days in a running production service — they are hardening gaps that compound the blast radius of an upstream compromise (catalog poisoning, npm hijack of @microsoft/events-cli or a transitive dep). The CLI also has no auth, no server, no PII handling, and runs only as the local user.
If maintainers feel this should have gone through the private channel anyway, I'm happy to:
- Close PR #32 and this issue;
- Re-submit privately via
aka.ms/SECURITY.md; - Or split the supply-chain / prompt-injection items out and keep the lower-risk hygiene items public.
Whatever's most useful to the team — please let me know.
Links
- PR: #32
- Branch:
joslat:security/hardening-2026-05 - Security review:
docs/security-review-2026-05-20.md - Fix plan / tracking table:
docs/security-fix-plan-2026-05-20.md
Acknowledgements
Same as the PR — thanks to the MAF office-hours crew for the conversation, and to Bruno Capuano for the original CLI walk-through.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with PR #32 and the referenced docs/security-review-2026-05-20.md and docs/security-fix-plan-2026-05-20.md; CONTRIBUTING.md explains why this issue was opened alongside the PR. Review the 17 findings and the proposed fixes, then confirm the maintainers' preferred scope and whether the PR and tracking issue can be closed.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- nodejs, typescript
- Domain
- ci-cd, cli, security
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 15/100