vuln-triage-reporter

# Vulnerability Triage, Reporter & Remediation Right-size MSRC/ITD vulnerability severity for Android Authenticator & Broker using **deep, evidence-based codebase analysis**, produce on-call/WBR reports, and — when asked — **remediate kept findings end-to-end** (implement the fix, test it, open a public-repo-safe PR; see Step 4.6). > The skill name is `vuln-triage-reporter` for stable invocation, but its scope is **triage → report → > remediate**. The reporting half stops at a dispatch-ready spec; the remediation half (Step 4.6, > [references/remediation-execution.md](references/remediation-execution.md)) can ship the fix itself. This skill is for **on-call engineers during their on-call week**. Default scope is the **past 7 days** (the rotation length), parameterized so it can be widened. > ⚠️ **PUBLIC SKILL — DO NOT COMMIT SENSITIVE INFORMATION HERE.** This repo is mirrored to a public > GitHub repo. Apply this test to anything you add: *"could an outsider with **no** Microsoft access act > on this?"* If yes, it does **not** belong in this skill. > > **NEVER put here (genuinely sensitive — actionable without access):** > - **Telemetry sampling rates or per-product coverage percentages** (these are an evasion map). > - **Internal security-control logic** — exact flight names, the precise conditions under which a > security check is bypassed/skipped, and `file:line` into private submodules describing such logic. > - **PII / customer data / tenant GUIDs / UPNs / aliases**, and **finding content paired with an IcM ID**. > > **OK to include (opaque — useless without corp access):** IcM numbers, IcM team-routing IDs, > service-tree GUIDs, team/service/codenames. These are inert to an outsider (IcM, ServiceTree, FireWatch, > S360 are all corp-auth-gated). > > **All investigation OUTPUTS are sensitive and live OUTSIDE the repo** in the private workspace > `$VULN_TRIAGE_WORKSPACE` (default `~/vuln-triage-workspace`) — never under the repo tree. > **Any future edit to this skill must preserve these rules.** > > 🔒 **MANDATORY before ANY commit that touches this skill: run the public-repo safety check** > (`scripts/safety_check.py`, see "Pre-Commit Safety Check" below). Never commit skill changes without it. > This is non-negotiable — sensitive information committed to a public repo cannot be un-leaked. > **Related skills.** This is the security-vulnerability counterpart to `incident-investigator` (which > handles auth-failure/log incidents). For all codebase exploration you **MUST** use `codebase-researcher` > — see the hard requirement in "Non-Negotiables" below. --- ## Requirements — verify BEFORE any work (HARD GATE) > 🛑 **Do NOT begin discovery, investigation, or reporting until every requirement below is satisfied.** > If any is missing, **stop and tell the user exactly what to fix** — partial or *stale* environments > produce **confidently wrong verdicts**, which are worse than no verdict. Run > `python scripts/preflight.py`, report PASS/FAIL per item, and only proceed on an all-PASS. > > **Order of operations:** run the **intake interview** (Step -1) *first* — it takes ~60 seconds and > tells you what the engineer actually wants — then run this environment gate **before launching any > agent**, and fold any FAILs into the plan echo. Don't make someone answer four questions only to be > told their checkout is unusable, and don't burn 30 minutes investigating the wrong thing in a > perfectly-configured environment. ### One command runs the whole gate ```powershell python .github/skills/vuln-triage-reporter/scripts/preflight.py # --skip-fetch offline / no network checks # --ests <path> identity-service checkout (default %ESTS_ROOT% or C:\src\ESTS-Main) # --json machine-readable ``` Exit **0** = safe to begin · Exit **1** = **stop and report**. Everything below is what it checks and why. > **If `python` opens the Microsoft Store** (or reports "Python was not found"), you have hit the Windows > App-Execution-Alias stub, not a real interpreter — every script in this skill will fail the same way. > Use the real install directly, e.g. > `& "$env:LOCALAPPDATA\Programs\Python\Python312\python.exe" <script>`, or prepend that directory to > `$env:PATH` for the session. ### 1. Full `android-complete` checkout WITH submodules The investigation greps **real source**. The app/broker code lives in **git-ignored submodules** that are **not** present in a bare clone or in a git **worktree**: - `authenticator/PhoneFactor/` — Microsoft Authenticator app + MSA SDK - `broker/AADAuthenticator/`, `broker/broker4j/` — broker app + library - `common`, `msal`, `adal` must also be populated. **Work from the main `android-complete` checkout (e.g. `C:\src\android-complete`), NOT a worktree** — worktrees created for skill edits typically lack the submodules. If those folders are missing/empty, the user must run `git droidSetup` / `git submodule update --init --recursive` **before** any triage. > **Why it's a hard gate:** a grep against a missing module returns nothing, and "no results" reads as > "the sink isn't there" — silently down-classifying a real finding. ### 2. Identity-service (**ESTS**) source — REQUIRED, not optional Many broker/MSAL findings turn on **what the token service does with the request**: how a grant is validated, whether the caller/application identity is checked, what is required versus optional on a given endpoint. Without ESTS source those questions cannot be answered, and the finding either stalls at *"unverifiable server-side boundary"* or — far worse — gets **guessed**. **This is a real, repeated cost:** two findings in one shift sat at Medium confidence with an open server-side question purely because the repo wasn't checked out. With it, both resolved to High in under 20 minutes, and the answer **changed the severity**. - Default location `C:\src\ESTS-Main`, or set `$env:ESTS_ROOT`, or pass `--ests <path>`. - Must be a **git checkout** (the skill runs history queries against it), and **current**. - If the engineer doesn't have it, **say so during intake** — cloning is slow, so surface it before they start waiting on an investigation that cannot finish. ### 3. Every repo CURRENT, and pointing at the RIGHT REMOTE Two separate checks, and the second one is the one that bites. **(a) Up to date.** Fetch and confirm 0 behind. A finding investigated against a stale checkout can report "no fix exists" for something that shipped weeks ago. **(b) Correct remote — a successful `git fetch` does NOT mean you are looking at the live repo.** When a repo migrates hosts, the retired location can keep resolving and keep serving a **frozen snapshot**. `git fetch` exits 0. `git pull` says *"Already up to date."* Nothing warns you. Meanwhile every `git log --all`, `git branch -a --contains` and Gate-0 "is this already covered?" query silently covers only history **up to the migration date**. > **Real failure this caused:** a broker checkout still pointed at the retired host, frozen ~3 weeks > earlier. Gate 0 reported *"fix exists but was never merged"* for a fix that had in fact landed **and > shipped**. Two findings were over-rated, and an escalation went out to leadership with the wrong > premise — twice, because the first correction was made from the same stale mirror. > > **The broker module has migrated** — see `docs/broker-remote-migration.md` in this repo for the > current host/slug and the one-line `git remote set-url` repair. `preflight.py` asserts the expected > remote per module and fails loudly on drift. ### 4. MCP servers / tooling | Capability | Used for | Required? | If missing | |------------|----------|-----------|------------| | **IcM MCP** (`search_incidents`, `get_incident*`, `get_teams_by_name`) | Discover `[MSRC]`/`[ITD]` findings + pull incident detail | **Required** for discovery (Steps 0–1) | Stop — cannot scope the week. (User can still paste IcM IDs to triage a specific finding.) | | **`codebase-researcher` subagent** | The mandatory two-pass code investigation | **Required** | Stop — the skill's core (Non-Negotiable #2/#3) cannot run. | | **ADO MCP** (`mcp_ado_wit_*`) | Create PBIs (Step 6) | Optional | Fall back to the ADO **REST API** + `az` token (see Step 6). | | **`az` CLI, logged in** | Live status report (Step 7) + REST PBI fallback | Optional (only for Steps 6–7) | Status report still renders without live state; PBI creation needs it if no ADO MCP. | | **`gh` authenticated per host** | Reading private-repo history/PRs | **Required** when a module lives on a different host | `gh auth status --hostname <host>`; `gh` routes by host, so stay logged into both. Auth is **interactive** — surface it at intake, not mid-run. | | **FireWatch / Security MCP** | — | **N/A — not reachable** | ITD findings are intake **manually** (Step 2); do not wait on a Security MCP. | ### 5. Private workspace `$VULN_TRIAGE_WORKSPACE` (default `~/vuln-triage-workspace`) must be writable — **all investigation outputs live there, OUTSIDE the repo** (they are sensitive). Never write findings under the repo tree. > IcM MCP / `codebase-researcher` availability is confirmed by the agent's own tool list — verify they are > present before Step 0. If the IcM MCP is down, the discovery step cannot run. --- ## Why This Skill Exists (read this first) The security team files MSRC/ITD vulnerabilities against us, each with a **pre-assigned classification** (e.g. FireWatch/Glasswing: `IMPORTANT`, `Tier 1 — Direct Exploit`). **That classification is an input, not a verdict.** Our job is to **agree with it or rebut it with documented code evidence**, so that engineering effort is allocated to what actually matters versus competing priorities. These findings are frequently **over-rated**: a real weakness exists, but the codebase already has **defense-in-depth** (flight gates, allow-lists, package/signature checks, non-exported components, root-only reachability) that prevents real-world mass exploitation. ### ⚠️ The failure we are correcting **In past investigations, AI agents did NOT analyze deeply enough.** They read the vulnerable sink, saw a plausible exploit, and either rubber-stamped the filed severity OR claimed defense-in-depth existed without proving it. **Both are failures.** The recurring mistake: stopping at the first or second layer of analysis and missing mitigating (or aggravating) controls that exist **beyond** the obvious code path. **The rule: always look for coverage beyond.** For every finding, you must actively hunt for controls in *adjacent* layers — the caller, the manifest, the IPC boundary, sibling handlers, flight defaults, build config, and the runtime reachability conditions — before concluding anything. A shallow "no mitigation found" is the exact error this skill exists to prevent. If you cannot find a control, you must show the *searches you ran* that justify its absence (mirror `codebase-researcher`'s "Not Found" discipline). ### Tell the defense-in-depth story — but only what you can prove Most findings filed against us are, in practice, **covered by some defense-in-depth mechanism**. When you have **sufficient evidence**, say so explicitly in a **"Defense-in-Depth: Why Likely Not Exploited"** section — the concrete reason real-world exploitation is unlikely (the gating flag, the server-validated number-match, the non-default path, the signature allow-list, etc.). This is what right-sizes severity. **Verification-boundary discipline (critical for honesty).** We own the **Authenticator client** and the **Broker/Common libraries** — we can prove things about *that* code. We do **not** own: - **Downstream consuming apps** (Outlook, Teams, OneAuth, other MSAL callers) — a caller may add its own validation, pick the browser path, pass a nonce, etc. We cannot observe this. - **Server-side** (eSTS / MFA backend / issuance) — **but see below: this boundary is often crossable.** > **⚠️ Do not retreat to "server-side, cannot verify" before you have tried.** With the **ESTS source > checkout** required by the Requirements gate, questions about what the token service validates are > frequently **answerable in first-party source**. Two findings in one shift sat at Medium confidence on an > assumed server-side backstop; reading ESTS resolved both to High in under 20 minutes — and **changed the > severity**. Treat "unverifiable boundary" as a conclusion you must *earn*, not a default. > > When you do cross into ESTS, keep the same discipline as anywhere else: cite `file:line`, quote the code, > and separate **verified in source** from **inferred from protocol convention**. And note what remains > genuinely outside *any* repo — operational controls such as Conditional Access evaluation, risk/fraud > scoring, or per-tenant policy are not in the token-pipeline source, and should not be assumed in either > direction. For anything still outside our boundary after that effort, **do not assert it as fact**. Add a **"Scope & Verification Boundary"** disclaimer stating: it is possible downstream services apply additional checks, but we cannot conclude definitively, and it would be worth investigating. **Only confirm what you can.** This cuts both ways — never claim "safe" *or* "exploitable" about a boundary you couldn't verify. --- ## Non-Negotiables 0. **Satisfy the Requirements hard gate FIRST.** Before any discovery/investigation/reporting, run `python scripts/preflight.py` and verify the environment per the **"Requirements — verify BEFORE any work"** section: full `android-complete` checkout **with submodules**, on the **main checkout not a worktree**; **ESTS source present and current**; **every repo current AND on the correct remote**; IcM MCP + `codebase-researcher` available; writable private workspace. If any item FAILs, **stop and tell the user what to fix** — do not begin work in a partial or stale environment. A missing submodule silently turns a real sink into a false "no sink"; a **stale or retired remote** silently turns a shipped fix into "no fix exists," and a `git fetch` that exits 0 is **not** proof you are looking at the live repository. 1. **Run investigations in PARALLEL.** Each finding is independent. Dispatch one investigation per finding concurrently (use the `codebase-researcher` subagent / `runSubagent`, or parallel `Explore` agents). Do **not** process findings sequentially when more than one is in scope. 2. **MUST use `codebase-researcher`** for every code-evidence step. Do not free-hand grep and call it analysis. The classification's credibility rests on cited `file:line` evidence gathered systematically. 3. **MANDATORY adversarial verification pass.** After the first investigation classifies a finding, dispatch a **second, independent `codebase-researcher`** whose only job is to **break the conclusion** — challenge every cited mitigation, hunt for a bypass, and try to reach the sink another way. Only after the challenger reports do you finalize. Record the outcome and set a **Confidence** level (High/Medium/Low). This is the core correction for the past failure — a single pass is not trustworthy. See "The two-pass model" below. 4. **Preserve the "Searches Run" audit trail VERBATIM.** Every investigation (both passes) must end with a `## Searches Run (audit trail)` section listing the actual search patterns/paths run and what each returned — especially the searches that returned **nothing** (the absence proofs behind every "no mitigation found" / "not reachable" claim). This is non-optional: the subagent's granular tool calls are not retained, so this section IS the audit trail. Copy it into the finding's report; do not summarize it away. 5. **Every severity call needs evidence.** Cite the sink AND every mitigating/aggravating control with `file:line`. No control found? Show the searches that prove the absence. 6. **Agree-or-rebut explicitly.** State FireWatch's filed classification, then state ours, then the delta and the evidence that justifies any change. 7. **Coverage gate FIRST, then solution the ones we keep.** Run **Gate 0**: if the cited sink is **already neutralized by an existing control** (an upstream allow-list/validator, flight default, signature/package check, non-exported component, server-side number-match…), cited with `file:line` on the **shipping branch**, classify it **`Won't-Fix (Already-Covered)`** and **close it out — ship nothing** (the safest outcome; a redundant fix in a >1B-user library is regression risk for zero security gain). We have been getting a high volume of findings that are already covered — but **not all are**, so the gate requires a cited control, never a hunch. Gate 0 has **six** outcomes, not two — see the table in Step 4. For every **kept** finding, produce a **dispatch-ready Remediation Spec** (root cause, fix approach, files to change, test plan, risks/rollout) — see [references/remediation-spec.md](references/remediation-spec.md). 8. **No PoC payloads or PII** in committed artifacts. Keep detail at engineering-triage level. 9. **Scripts, not one-liners.** Use the committed scripts in `scripts/` for discovery, scaffolding, transcription, and roll-up so the weekly run is repeatable. 10. **Generate the HTML evidence record per finding.** The master report's table is a summary; the real proof lives in one HTML subpage per finding (sink + defense-in-depth sweep + remediation spec + the verbatim "Searches Run" audit). Generate them with `scripts/build_research_pages.py` and link each master-table row to its subpage. Reviewers must be able to verify every severity call without chat access. 11. **Run the public-repo safety check before committing.** Any commit touching this skill MUST be preceded by `scripts/safety_check.py` (see "Pre-Commit Safety Check"). A non-zero exit blocks the commit. 12. **Map to an IcM Sev, conservatively.** Translate the analytical tier to the team's IcM severity (Sev2/2.5/3/4) using the mapping in [references/severity-rubric.md](references/severity-rubric.md).