fix(ci): resolve 3 SonarCloud vulns (S7637 x2, S8707)

[thomasluizon]

Fixes 3 SonarCloud VULNERABILITIES, all CI-config-only (.github/), no app code.

Findings

Rule	File:Line	Fix
githubactions:S7637	.github/workflows/test.yml:177	Pin oasdiff/oasdiff-action/breaking to full SHA 024f6c399f9a21ada1addb0f9a36ce1bfac995f1 (# v0.1.6)
githubactions:S7637	.github/workflows/dependabot-auto-merge.yml:16	Pin dependabot/fetch-metadata to full SHA 25dd0e34f4fe68f24cc83900b1fe3fe149efef98 (# v3.1.0)
pythonsecurity:S8707	.github/scripts/check_coverage.py:18	Validate/confine the CLI-supplied path before filesystem access

SHA ↔ tag verification

Each SHA was cross-checked against the moving major tag it replaces via gh api:

• oasdiff/oasdiff-action v0 → 024f6c39… — identical to release v0.1.6.
• dependabot/fetch-metadata v3 → 25dd0e34… — identical to release v3.1.0.

Trailing # vX.Y.Z comments are retained as allowed tooling context so Dependabot can still track updates.

Path validation (S8707)

results_dir came from sys.argv[1] and was joined into a glob pattern, then each match was ET.parsed — a CLI-controlled path reaching the filesystem unvalidated. A _confine() helper now resolves the results dir and each globbed report against the process working directory (the CI workspace) and rejects anything that escapes it (traversal or absolute path outside the tree). Legitimate inputs (./TestResults) are unaffected.

Testing

No Python test harness exists in this .NET/xUnit repo and no CI step runs .github/scripts/ tests, so a committed unit test would be dead code. Validation was done via an out-of-repo smoke test:

• valid relative ./TestResults → parses and gates correctly (50% pass@40, fail@90);
• ../../etc traversal → rejected, exit 1;
• absolute path outside cwd → rejected, exit 1;
• _confine unit checks reject ../, prefix-attack (/repo-evil vs /repo), accept valid subpaths.

python -m py_compile and PyYAML safe_load on both workflows pass.

Refs thomasluizon/orbit-ui-mobile#243

[claude]

Code Review: PR #323 — fix(ci): resolve 3 SonarCloud vulns (S7637 x2, S8707)

Recommendation: APPROVE

Summary

CI-config-only PR (no src/ changes): pins two third-party GitHub Actions (oasdiff/oasdiff-action/breaking, dependabot/fetch-metadata) to full commit SHAs with a trailing # vX.Y.Z comment, and adds a _confine() path-traversal guard to .github/scripts/check_coverage.py before it globs/parses coverage XML from a CLI-supplied directory. All three changes match the SonarCloud findings cited in the PR body (19 additions / 4 deletions across 3 files), and the code is correct on inspection.

Findings

Critical / High / Medium: None. Low / Info: none posted per the rubric's signal gate.

Security review (dimension #12, manual)

• Supply-chain (S7637): both actions moved from a mutable major tag (@v0, @v3) to a full SHA with the version kept as a trailing comment — the correct pattern, and it preserves Dependabot's SHA-pin update flow. Could not independently re-derive the two SHAs against upstream tags (oasdiff-action@v0.1.6, fetch-metadata@v3.1.0) — no outbound network access in this review session. PR #323's own "OpenAPI Breaking-Change Gate" check (which invokes the pinned oasdiff-action) was green at review time, an operational sanity check that the pin at least resolves to a working action build.
• Path traversal (S8707): _confine(candidate, base) in .github/scripts/check_coverage.py resolves via os.path.realpath (collapses symlinks) and rejects unless resolved == base or resolved.startswith(base + os.sep). The + os.sep detail avoids the classic prefix-bypass bug where "/repo-evil".startswith("/repo") would be a false negative. Applied to both the CLI-supplied results_dir and, again, to every glob-matched report path before ET.parse() — the second call catches an in-tree symlink resolving outside base that the raw glob path wouldn't reveal. Logic is sound.
• XXE note (pre-existing, not a regression): ET.parse() (stdlib, not defusedxml) is unchanged by this diff and processes CI's own dotnet-generated Cobertura XML, not attacker input — out of scope here.
• No credentials/auth logic touched.

Subagents

Agent	Verdict
security-reviewer	N/A — gate is "any src/ code changed"; diff touches only .github/, nothing in Orbit.* Controllers/Infrastructure. Security dimension walked manually instead since this is itself a security PR.
contract-aligner	N/A — no DTO/Controller route/packages/shared type touched.

Validation

Check	Result
Build (dotnet)	N/A — CI scope, runs as a separate required check
Tests (dotnet)	N/A — same

Deferred

• Dimensions 8/9/10 (DESIGN.md, parity, i18n), 11 (contract drift), 13 (backend hard rules), 14 (FEATURES.md parity): N/A — no apps/*, DTO, or orbit-api/src/** surface touched.
• SHA↔tag mapping: author-asserted in the PR body, not independently reverified (no network egress available in this review environment) — see security section above.
• All 3 changed files (check_coverage.py, dependabot-auto-merge.yml, test.yml) received a full verdict; none skipped.

What's good

• Scoped precisely to the 3 flagged findings — other actions/* refs deliberately left tag-pinned, keeping the diff minimal.
• startswith(base + os.sep) rather than the naive startswith(base) — the fix actually closes the prefix-bypass gap instead of just looking like it does.
• Honest testing note: no Python test harness exists in this repo, so validation was done out-of-repo rather than adding dead in-repo test code.

Recommendation

Merge as-is. Optional non-blocking follow-up: a zizmor/pin-github-action-style CI check that machine-verifies SHA pins resolve to their annotated tag, since this review couldn't do that verification itself.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[claude]

Code Review: PR #323 — fix(ci): resolve 3 SonarCloud vulns (S7637 x2, S8707)

Scope: PR #323 in thomasluizon/orbit-api (.github/ only — 3 files, 19 additions / 4 deletions)
Recommendation: APPROVE

Summary

Three targeted SonarCloud fixes, all CI-config-only: pin two third-party GitHub Actions (oasdiff/oasdiff-action/breaking, dependabot/fetch-metadata) to full commit SHAs with a trailing version comment, and add a _confine() path-containment guard in .github/scripts/check_coverage.py so the CLI-supplied results_dir and each globbed report path are resolved and checked against the CI working directory before glob/ET.parse touch them. No src/ application code changed. The fix root-causes the path-traversal finding (containment, not a blocklist) and correctly guards against the classic startswith(base) prefix-bypass (/repo-evil vs /repo) by checking base + os.sep.

Findings

Critical

None.

High

None.

Medium

None.

Low / Info

• _confine(report, base) inside the per-report loop in main() (.github/scripts/check_coverage.py:40) is not wrapped in the same try/except ValueError as the initial results_dir check, so a symlink that resolves outside base (a coverage.cobertura.xml symlink planted inside an otherwise-legitimate TestResults/) would raise an unhandled ValueError and print a raw traceback instead of the clean stderr message + return 1 used elsewhere. CI still fails closed (non-zero exit either way), so this is a polish/UX gap, not a security hole — not blocking.

Subagents

Agent	Verdict
security-reviewer	N/A — gate is "any src/ code changed"; this diff touches only .github/scripts/ and .github/workflows/, not orbit-api/src/. Performed the security analysis (path-traversal / prefix-bypass / symlink-escape) inline instead, per rubric dimension 12.
contract-aligner	N/A — no DTO, Controller route, or packages/shared type touched.

Validation

Check	Result
Build (dotnet)	N/A — CI adaptation: Build runs as a separate required check in this repo, and this diff has no .cs changes.
Tests (dotnet)	N/A — no application code changed. Python has no test harness in this repo (correctly called out in the PR body).
SHA↔tag re-verification	Not independently re-run in this session — gh api calls to repos/oasdiff/oasdiff-action/git/refs/tags/v0.1.6 and repos/dependabot/fetch-metadata/git/refs/tags/v3.1.0 required network approval unavailable in this job. Relying on the author's documented cross-check in the PR body.

Deferred — N/A dimensions & files not verdicted

• Dimensions 3, 8, 9, 10, 11, 13, 14 — all N/A, diff never touches apps/*, DTOs, orbit-api/src/, or the user-facing feature surface.
• Dimension 4 (comment policy) — the _confine() docstring is Python; no-comments.cjs is JS/TS-only and ORBIT0001 is C#-only, neither applies to this file. No narration comments found regardless.
• All 3 changed files (check_coverage.py, dependabot-auto-merge.yml, test.yml) were read in full diff context and given a verdict above.
• Live SHA↔tag re-verification — network access unavailable in this session; not re-run independently.

What's good

• Root-causes S8707 with real containment (os.path.realpath + startswith(base + os.sep)), correctly avoiding the common str.startswith(base) prefix-bypass bug.
• Confines both the top-level results_dir argument and each individually globbed report path — closes the symlink-escape gap that confining only the directory would leave open.
• SHA pins retain the # vX.Y.Z trailing comment so Dependabot can still track updates.
• PR body documents the exact verification method (tag→SHA cross-check, traversal/absolute-path/prefix-attack manual smoke tests) — good transparency for a fix that's hard to unit-test in-repo.

Recommendation

Approve as-is. Optional low-priority follow-up (not blocking): wrap the _confine(report, base) call inside the report loop in the same try/except as the initial check so a symlink-escape attempt prints a clean stderr message instead of a raw traceback — cosmetic only, since either path already fails CI with a non-zero exit.