Skip to content

Security review: 17 findings (3 High, 7 Medium, 5 Low, 2 Info) — fix PR in #32 #33

@joslat

Description

@joslat

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.jsonpackage-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:

Whatever's most useful to the team — please let me know.

Links

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions