Skip to content

🐛 fix(ci): relativise ZAP SARIF URIs with originalUriBaseIds for code scanning#404

Merged
s-b-e-n-s-o-n merged 1 commit into
mainfrom
fix/zap-sarif-uri-base
May 29, 2026
Merged

🐛 fix(ci): relativise ZAP SARIF URIs with originalUriBaseIds for code scanning#404
s-b-e-n-s-o-n merged 1 commit into
mainfrom
fix/zap-sarif-uri-base

Conversation

@s-b-e-n-s-o-n
Copy link
Copy Markdown
Contributor

@s-b-e-n-s-o-n s-b-e-n-s-o-n commented May 29, 2026

Summary

After PR #402 (auth-readiness gate) and PR #403 (correct credentials), DAST now scans successfully — but Upload ZAP SARIF still fails:

Code Scanning could not process the submitted SARIF file: SARIF URI scheme "http" did not match the checkout URI scheme "file"

Per SARIF spec §3.14.14, DAST results must store origins in `originalUriBaseIds` and use relative URIs with `uriBaseId` references. The current converter emits raw `http://localhost:3333/...\` URIs in `artifactLocation.uri`, which Code Scanning rejects.

Changes

`scripts/zap-json-to-sarif.mjs`:

  • New `resolveArtifactLocation()` helper detects http(s) URIs, strips the origin, returns `{ uri: relativePath, uriBaseId: 'TARGET' }`. Non-http fallback values (`nodeName`, site `@name`, `'zap-target'`) pass through unchanged with no `uriBaseId`.
  • `convertZapJsonToSarif()` collects distinct origins into `originalUriBaseIds` at the run level.

`scripts/zap-json-to-sarif.test.mjs` updated assertions for the new shape.

Verification

  • `node --test scripts/zap-json-to-sarif.test.mjs` → 10/10 pass
  • Pre-push gates all green (coverage, build, biome, qlty)
  • Post-merge CI Verify run 26613337624 confirms this is the last blocker: every DAST step passed except `Upload ZAP SARIF`

Test plan

  • CI Verify on this PR passes lint/coverage/scripts-test
  • Post-merge CI Verify on main: DAST → Upload ZAP SARIF succeeds (the canonical proof)

@s-b-e-n-s-o-n
🐛 fix(ci): relativise ZAP SARIF URIs with originalUriBaseIds for code…
9db2db7 
… scanning

GitHub Code Scanning rejects SARIF where artifactLocation.uri uses the
http scheme because it doesn't match the file:// checkout URI scheme.
Per SARIF spec §3.14.14, DAST results must store the origin in
originalUriBaseIds and use a relative path as the uri. The converter now
strips the http(s) origin into an originalUriBaseIds TARGET key and
writes only the path as uri. Non-http fallback values (nodeName, site
@name, 'zap-target') pass through unchanged.
@vercel
Copy link
Copy Markdown

vercel Bot commented May 29, 2026 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
drydock-demo Ready Ready Preview, Comment May 29, 2026 2:34am
drydock-website Ready Ready Preview, Comment May 29, 2026 2:34am

biggest-littlest
biggest-littlest approved these changes May 29, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@biggest-littlest biggest-littlest left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — proper SARIF originalUriBaseIds shape. Last DAST blocker after #402/#403.

ALARGECOMPANY
ALARGECOMPANY approved these changes May 29, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@ALARGECOMPANY ALARGECOMPANY left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 29, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@s-b-e-n-s-o-n s-b-e-n-s-o-n merged commit d5ece86 into main May 29, 2026
24 checks passed
@s-b-e-n-s-o-n s-b-e-n-s-o-n deleted the fix/zap-sarif-uri-base branch May 29, 2026 02:50
@s-b-e-n-s-o-n s-b-e-n-s-o-n mentioned this pull request May 29, 2026
Merged
1 task
s-b-e-n-s-o-n added a commit that referenced this pull request May 29, 2026
@s-b-e-n-s-o-n
🐛 fix(ci): preserve leading slash in ZAP SARIF artifact URIs so root …
ee674b9 
…scans validate (#405)

Follow-up to #404. Root scans were producing empty SARIF artifactLocation.uri; keep the leading slash so root = '/', paths = '/foo'.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@biggest-littlest biggest-littlest biggest-littlest approved these changes

@ALARGECOMPANY ALARGECOMPANY ALARGECOMPANY approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@s-b-e-n-s-o-n @biggest-littlest @ALARGECOMPANY