chore(ci): secret scanning + vulnerable-package check + coverage (Tier 1)#96
Conversation
Tier 1 security + coverage additions. All three run on every PR with no
GHAS dependency.
- .NET job: appends `--collect:"XPlat Code Coverage"` and uploads the
cobertura report as `dotnet-coverage` artifact. NO threshold enforced
yet — see CONTRIBUTING.md § Coverage for why (need measured baseline
before locking a floor).
- .NET job: `dotnet list package --vulnerable --include-transitive`
step that fails CI on any known CVE in the dependency graph,
including transitives. Dependabot covers ongoing advisory tracking;
this catches the case where a contributor adds a package with a
known vulnerability at PR time.
- New secret-scan job: Gitleaks scans the full diff for committed
secrets (API keys, passwords, tokens). Free for personal accounts on
private repos (phuong-labs is currently a personal namespace); a
comment notes the GITLEAKS_LICENSE requirement if the org moves to
paid plan.
CONTRIBUTING.md gets a new "Coverage" section explaining the
defer-threshold rationale. agent-checklist.md's CI-only gates list grows
to cover the three new automated gates.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds CI gates: enable XPlat coverage collection and upload Cobertura XML; run NuGet vulnerable-package detection to fail on advisories; add a TruffleHog secret-scan job for PRs; add a central System.Formats.Asn1 pin; and update CONTRIBUTING.md and agent checklist to document these gates. ChangesCI Quality and Security Gates
Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant DotnetTest
participant ArtifactUpload
participant NuGetScanner
participant TruffleHog
GitHubActions->>DotnetTest: run `dotnet test --collect:"XPlat Code Coverage"`
DotnetTest->>ArtifactUpload: emit `coverage.cobertura.xml` in TestResults/**
GitHubActions->>ArtifactUpload: upload `dotnet-coverage` artifact
GitHubActions->>NuGetScanner: run `dotnet list package --vulnerable --include-transitive`
NuGetScanner->>GitHubActions: advisory rows -> fail job if any
GitHubActions->>TruffleHog: run trufflesecurity/trufflehog on base..head (--only-verified)
TruffleHog->>GitHubActions: findings -> fail job
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
PR #96 CI run failed with: "missing gitleaks license. Go grab one at gitleaks.io and store it as a GitHub Secret named GITLEAKS_LICENSE". gitleaks-action v2 added a paid license requirement for all account types (the comment I left claiming "free for personal accounts" was based on stale information — the action's breaking update happened after that doc page snapshot). Confirmed by the action's own error output pointing at the announcement page. Switch to TruffleHog: also widely used for secret scanning, MIT-licensed, no paid plan required for any account type. `--only-verified` queries the alleged service for token validity before reporting, cutting the classic high-false-positive rate that makes secret scanners noisy. Updated agent-checklist.md to match. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Previous commit used a hallucinated SHA that didn't exist in the trufflesecurity/trufflehog repo. Pin to the actual v3.95.3 commit. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Job name was 'Secret scanning (TruffleHog)' — the tool name in the required-check string means a future tool swap breaks branch protection rules silently (the rule keeps requiring the old name, which no longer exists, but appears 'satisfied' because the missing check looks skipped). Rename to 'Secret scanning'; move tool reference into a comment and the step name. Same pattern as our '.NET — Build and Test' (tool-agnostic, not '.NET — dotnet test'). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The vulnerable-packages CI check added earlier in this PR did its job: flagged High-severity GHSA-447r-wph3-92pm in System.Formats.Asn1 7.0.0 across every project that transitively pulls OpenIddict or the ASP.NET cryptography stack. This commit fixes the underlying CVE — not the check. - Directory.Packages.props: add <PackageVersion> entry for 8.0.1 so the version is centrally tracked alongside the other deps. - Axis.Shared.Infrastructure.csproj: add explicit <PackageReference> Include="System.Formats.Asn1"/>. The patched version propagates to every consumer through the project-reference chain. Why explicit reference rather than CentralPackageTransitivePinningEnabled: that property pins ALL transitives globally, which caused PageBuilder (scaffold project with no real source) to resolve unrelated packages back to .NET Framework 1.x versions. The targeted explicit reference gets the CVE patched without touching unrelated resolution graphs. Comment text in Directory.Packages.props uses prose rather than literal CLI flags because XML comments forbid the two-hyphen sequence (it would terminate the comment early). MSBuild's parser silently broke when an earlier draft of this comment contained that sequence — surfaced as NU1015 errors with seemingly unrelated package references. Verified: `dotnet list package --vulnerable --include-transitive` now reports "no vulnerable packages" across all 47 projects. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Summary
Tier 1 security + coverage additions to PR CI. All three run automatically without GHAS provisioning.
secret-scanjob) — scans full diff for committed secrets. Free for personal accounts on private repos; license note added for future org move.dotnetjob) —dotnet list package --vulnerable --include-transitivefails CI on any known CVE, catching the contributor-adds-bad-package-at-PR-time case that Dependabot only catches post-merge.--collect:"XPlat Code Coverage"todotnet testand uploads cobertura as artifact (dotnet-coverage). No threshold enforced yet — see CONTRIBUTING.md § Coverage for why (need measured baseline first).Linked spec
No feature — Tier 1 follow-up to PR #95 (architecture conventions). References:
Requirements & rules followed
CONTRIBUTING.md,agent-checklist.md.TODO/FIXME/NotImplementedException/ placeholder / stub undersrc/,tests/,frontend/src/What's next
WebApplicationFactory).GetRequiredService, hardcoded connection strings,DateTime.Now, plus.Result/.Wait()with Task type info).dotnet-coverageartifact, set<Threshold>to baseline minus 5pp intests/Directory.Build.props.Summary by CodeRabbit
Chores
Documentation