Phase 9 Sub-phase A: JSON / SARIF / OSCAL / CSV exports - #32
Merged
Conversation
Lands the format-layer of the "evidence source for GRC platforms" story — no vendor APIs, no credentials, no lock-in. Downloads available both from the CLI (`.grc/exports/`) and the dashboard (per-repo dropdown + org-level aggregate button). ## Per-CVE capture `scanner/rules/dependencies.ts` now parses the full `vulnerabilities` object from `npm audit --json` into a new `DependencyVulnerability[]` on the manifest. Each entry carries advisory id, package, severity, CVSS score, vulnerable range, fix availability, and paths. Stable sort (critical first, then by package name) so exports are reproducible. Backward-compat: the field is optional on Manifest; older KV entries evaluate to an empty array. ## Exporters - **JSON** — `GRCExport` envelope (manifest + NIST CSF eval + EU AI Act eval + full risk register). Schema-versioned. - **SARIF 2.1.0** — conformant output. Rule catalog covers secret leaks, dependency vulnerabilities, unprotected routes, and AI prohibited / high-risk findings. CVSS severity maps to SARIF level. Fingerprints let the GitHub Security tab dedupe across runs. Rules only emitted when they actually produced results. - **OSCAL v1.1.2 Assessment Results** — one result per framework (NIST CSF always, EU AI Act when evaluated). Each evaluated control becomes an observation; non-pass non-NA controls also become findings. Cross-refs preserved as OSCAL props. - **CSV (4 files)** — flat tables for NIST CSF controls, EU AI Act articles, risk register, dependency vulnerabilities. RFC 4180 quoting. Repo/branch/commit/scan_date columns on every row so org-level concatenation is unambiguous. ## CLI Every scan also writes exports to `.grc/exports/`: - `<repo>-<branch>-<commit>.json` - `<repo>-<branch>-<commit>.sarif` - `<repo>-<branch>-<commit>.oscal.json` - `<repo>-<branch>-<commit>-nist-csf.csv` - `<repo>-<branch>-<commit>-eu-ai-act.csv` - `<repo>-<branch>-<commit>-risks.csv` - `<repo>-<branch>-<commit>-vulnerabilities.csv` `.grc/` is already gitignored so these don't pollute consumer repos. ## Dashboard Two new entry points: - `GET /export/:owner/:name/:format` — per-repo, respects `?branch=<name>` (defaults to main/master via findByBranch). EXPORT dropdown on each repo card; the button reads the selected branch from the existing combobox. - `GET /export/all/:format` — org-level bundle. Main/master only across all repos. JSON returns an array of per-repo GRCExports; CSV endpoints concatenate per-repo rows keeping one header. SARIF and OSCAL org aggregation deferred — merge semantics need more thought and nobody's asked. The ORG EXPORT (MAIN) button is right-aligned above the repo list on the home page; per-repo EXPORT buttons sit in the controls bar next to CHECK PRODUCTION. ## Tests 11 new Vitest assertions in `scanner/generators/exports/exports.test.ts` cover: - JSON: schema field, schemaVersion, arrays typed correctly - CSV: RFC 4180 quoting of commas / quotes / newlines; header + row count; graceful handling of manifests missing the optional vulnerabilities field - SARIF: valid 2.1.0 envelope, $schema, severity → level mapping, per-AI-system result emission, focused rule list - OSCAL: valid 1.1.2 envelope, UUID shape, conditional EU AI Act result, observation/finding counts Total test suite: 62/62 passing. ## Known gaps - SARIF upload step in `action.yml` is deferred. Shipping it requires consumers to add `security-events: write` to their workflow permissions block. Want signal on real demand before asking for another permission. - Org-level SARIF + OSCAL bundles aren't produced. They'd need deduplication semantics and stable UUIDs across repos; waiting for a concrete ask. - Scanner dogfood: no `package-lock.json` vulns right now so the exported SARIF from this repo is empty. Routes still serve a valid empty SARIF envelope (verified).
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e33c4ec12a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Comment on lines
+860
to
+864
| const lines = csv.split("\n"); | ||
| // Skip the header on each subsequent file, drop trailing empty line. | ||
| for (let i = 1; i < lines.length; i++) { | ||
| const line = lines[i]!; | ||
| if (line.length > 0) rows.push(line); |
There was a problem hiding this comment.
The org-level CSV aggregator splits each per-repo CSV on raw \n and reassembles line-by-line, which corrupts valid CSV rows that contain embedded newlines inside quoted fields. The exporters already emit such values (for example, evidence/description fields), so this can produce malformed aggregate CSV files where one logical row becomes multiple broken rows. Merge logic should strip only the first header record from each file without tokenizing by newline.
Useful? React with 👍 / 👎.
Comment on lines
+123
to
+124
| const loc = parseLocation(finding); | ||
| return { |
There was a problem hiding this comment.
Secret findings are passed directly to parseLocation as if each entry were path[:line], but the scanner stores human-readable strings like "API key found in <file>". That means the SARIF artifactLocation.uri is prose instead of a repository-relative path, so secret-leak results cannot be reliably mapped to files in code-scanning consumers.
Useful? React with 👍 / 👎.
Comment on lines
+795
to
+799
| if (!existing) { | ||
| // Accept whatever we see first; will be replaced if a main turns up. | ||
| byRepo.set(repo, entry); | ||
| continue; | ||
| } |
There was a problem hiding this comment.
The org export selector stores the first branch encountered for each repo and only replaces it if a main/master entry is later found. For repos that currently lack main/master manifests, this includes an arbitrary non-main branch in /export/all/*, which conflicts with the endpoint’s stated main-only semantics and can leak feature-branch state into audit bundles.
Useful? React with 👍 / 👎.
Three review findings, all real. Fixes plus regression tests.
## P1 — Org CSV merge corrupts quoted-newline rows
concatCsv split each per-repo CSV on raw `\n` and reassembled,
which tears through valid RFC 4180 rows that contain embedded
newlines inside quoted fields. Our own CSV generators quote
evidence / description / title cells, and the risk-register
test already exercises that path — so every org CSV download
was liable to emit a broken evidence row.
Fix: new splitHeaderAndBody() walks the string character-by-
character tracking in-quotes state, finding the first UNQUOTED
newline. Everything before is the header, everything after is
the body. Doubled-quote escape (`""`) and CRLF endings are
handled. concatCsv now concatenates bodies without ever
tokenizing them.
Refactored into scanner/generators/exports/concat.ts so the
helper sits next to the CSV emitters it operates on, gets
unit-tested, and stays out of the worker file. concat.test.ts
covers: embedded newlines preserved, single header regardless
of input count, CRLF parsing, doubled-quote escape, empty input
and header-only input edge cases.
## P1 — SARIF secret path
Secret findings in the manifest are human-readable strings —
"OpenAI API key found in src/config.ts" — not plain
`path[:line]` tokens. parseLocation was receiving the whole
sentence and putting prose into artifactLocation.uri, so
code-scanning consumers couldn't match findings to files.
Fix: new parseSecretFinding() splits on " found in " to
extract the label ("OpenAI API key") and the location
("src/config.ts" or "lib/creds.ts:42"). SARIF results now
carry a correct repo-relative path, startLine when present,
and a clean message. Fingerprint extended to include the
label so two different secret types in the same file don't
collide.
Regression test added: two fixture findings (one with line
number, one without) verified to produce correct artifact
locations + startLine.
## P2 — Org export leaks feature branches when main is absent
mainEntryPerRepo recorded the first branch seen per repo and
only replaced it if a main/master scan turned up later. If a
repo only had feature-branch manifests in KV (a common early
state), an arbitrary feature branch would ride along in
/export/all/* — directly contradicting the endpoint's
"main/master only" semantics and leaking WIP state into
audit bundles.
Fix: skip non-main/master entries entirely during the
gathering phase. Repos with no main/master manifest are
excluded from the org bundle rather than silently substituted.
If a repo has both (shouldn't happen), "main" wins over
"master".
## Verification
- 71/71 tests passing (9 new: 5 for concatCsv + 1 for SARIF
secret parsing + 3 previously).
- Lint clean.
- Smoke tests pass.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Lands the format layer of Phase 9 — "evidence source for GRC platforms" without vendor APIs, credentials, or lock-in. Downloads available from the CLI (
.grc/exports/) and the dashboard (per-repo dropdown + org-level button).Per-CVE capture (data layer prerequisite)
scanner/rules/dependencies.tsnow parses the fullvulnerabilitiesobject fromnpm audit --jsoninto a newDependencyVulnerability[]on the manifest. Each entry carries advisory id, package, severity, CVSS, vulnerable range, fix availability, paths. Stable sort so exports are reproducible. Optional field → older KV entries stay valid.Exporters (new)
GRCExportenvelope: manifest + NIST CSF eval + EU AI Act eval + full risk register. Schema-versioned.CLI output
Every scan writes 7 files to
.grc/exports/..grc/already gitignored.Dashboard
GET /export/:owner/:name/:format— per-repo. Respects?branch=<name>; defaults to main/master. EXPORT dropdown on each repo card, button reads branch from the existing combobox.GET /export/all/:format— org-level, main/master only. JSON returns an array of per-repo GRCExports; CSV endpoints concatenate rows keeping one header. SARIF + OSCAL org aggregation deferred (merge semantics need more thought, nobody's asked).Tests
11 new Vitest assertions in
scanner/generators/exports/exports.test.ts:vulnerabilities$schema, severity → level mapping, per-AI-system emission, focused rule listTest suite: 62/62 passing.
Test plan
npm test/npm run lint/npm run smoke:dashboard/npm run scan -- .all green locallywrangler dev+ POST a fixture manifest with 1 critical CVE + 1 high-risk AI system → all 12 endpoints (7 per-repo + 5 org-level) return HTTP 200 with plausibly-sized bodies. SARIF from that fixture correctly containsgrc/dependency-vulnerability(error) andgrc/ai-high-risk(warning) results.What's deferred
action.yml— requires consumers to addsecurity-events: write. Holding off until someone actually wants the GitHub Security tab integration. Format compatibility is there regardless.Open-source-readiness status
All three original blockers (#28 auth, #29 CI, #30 fork-path) + polish (#31) shipped. This PR starts the feature-roadmap phase — first concrete piece of Phase 9 "evidence source for GRC platforms".