chore(ci): editorconfig, format gate, NuGet cache, dependabot expansion, CodeQL - #60
Conversation
Foundation hygiene PR enhancing GitHub Actions CI for production scale. No application code semantics change. CI workflow: - `setup-dotnet` built-in NuGet cache (`cache: true`) shaves restore time - `dotnet format --verify-no-changes` step closes the Gate 1 gap (CLAUDE.md required it locally; CI was not enforcing) - `dotnet test --logger trx` + `actions/upload-artifact` for PR annotations + post-mortem analysis - `timeout-minutes` per job — deadlocked test no longer burns minutes - New `dependency-review` job catches vulnerable transitive deps on PR - New `codeql.yml` workflow for SAST scanning (C# + JS/TS), `security-extended,security-and-quality` queries, weekly + PR runs Dependabot: - Extends the existing actions-only config with NuGet (Directory.Packages) and frontend npm. Per-ecosystem groups keep noise down: Microsoft.*, WolverineFx*, OpenIddict, Serilog, TanStack, Vite, Biome. .editorconfig: - Project was missing this canonical .NET config. Without it, `dotnet format` on Windows used CRLF while CI on Linux used LF, producing false-positive whitespace errors locally. Minimal version here enforces LF, UTF-8 (no BOM), trim trailing whitespace, indent per file type. Comprehensive style/naming/analyzer rules are deferred to a follow-up issue so this PR stays scoped. BOM fixes: - New charset rule surfaced 9 files (auto-generated migrations + test fixtures) with UTF-8 BOM. `dotnet format` strips them. No behaviour change. Module README touches (drift requirement): - E04, E05, E06 each get a short, accurate detail about EF migrations in the Infrastructure row (naming the migrations the BOM fix touched). Identity is not in the drift script's path map.
|
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 (1)
📝 WalkthroughWalkthroughThis PR standardizes repository formatting with ChangesRepository Quality and CI/CD Standards
Estimated Code Review Effort🎯 2 (Simple) | ⏱️ ~12 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 |
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/build-and-test.yml (1)
11-13:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winGrant
pull-requests: writefor dependency-review PR comment summaries
comment-summary-in-pr: on-failurerequirespull-requests: write, but the workflow only grantspermissions: contents: read.actions/setup-dotnetcache keys are based onpackages.lock.json; withcache-dependency-path: '**/*.csproj'(and nopackages.lock.jsonin the repo), cache invalidation won’t track CPM-only changes inDirectory.Packages.props.🔧 Suggested fix
dependency-review: name: Dependency review runs-on: ubuntu-latest timeout-minutes: 5 + permissions: + contents: read + pull-requests: write steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: persist-credentials: false🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/build-and-test.yml around lines 11 - 13, The workflow currently only grants permissions: contents: read which prevents comment-summary-in-pr: on-failure from posting PR summary comments; update the permissions block to include pull-requests: write (in addition to contents: read) so comment-summary-in-pr can create PR comments, and adjust the actions/setup-dotnet cache config by changing cache-dependency-path from '**/*.csproj' to include packages.lock.json and/or Directory.Packages.props (e.g., '**/packages.lock.json' and 'Directory.Packages.props') so cache keys reflect CPM-only changes.
🧹 Nitpick comments (1)
.github/workflows/build-and-test.yml (1)
30-31: ⚡ Quick winInclude central package metadata in NuGet cache key inputs.
Using only
**/*.csprojmisses central package version bumps inDirectory.Packages.props, reducing cache effectiveness.♻️ Suggested refactor
with: dotnet-version: 8.0.x cache: true - cache-dependency-path: '**/*.csproj' + cache-dependency-path: | + **/*.csproj + Directory.Packages.props + **/packages.lock.json🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/build-and-test.yml around lines 30 - 31, The NuGet cache key currently only tracks project files via cache-dependency-path '**/*.csproj' and therefore misses central package version changes in Directory.Packages.props; update the workflow step that defines cache-dependency-path to include Directory.Packages.props (e.g., add '**/Directory.Packages.props' alongside '**/*.csproj') so cache keys change when central package metadata is updated and caching remains correct.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In @.github/workflows/build-and-test.yml:
- Around line 11-13: The workflow currently only grants permissions: contents:
read which prevents comment-summary-in-pr: on-failure from posting PR summary
comments; update the permissions block to include pull-requests: write (in
addition to contents: read) so comment-summary-in-pr can create PR comments, and
adjust the actions/setup-dotnet cache config by changing cache-dependency-path
from '**/*.csproj' to include packages.lock.json and/or Directory.Packages.props
(e.g., '**/packages.lock.json' and 'Directory.Packages.props') so cache keys
reflect CPM-only changes.
---
Nitpick comments:
In @.github/workflows/build-and-test.yml:
- Around line 30-31: The NuGet cache key currently only tracks project files via
cache-dependency-path '**/*.csproj' and therefore misses central package version
changes in Directory.Packages.props; update the workflow step that defines
cache-dependency-path to include Directory.Packages.props (e.g., add
'**/Directory.Packages.props' alongside '**/*.csproj') so cache keys change when
central package metadata is updated and caching remains correct.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 76de20bc-6b61-43de-8c1d-4087ee52e2b8
📒 Files selected for processing (15)
.editorconfig.github/dependabot.yml.github/workflows/build-and-test.yml.github/workflows/codeql.ymldocs/epics/E04-workflow-builder/README.mddocs/epics/E05-form-builder/README.mddocs/epics/E06-workflow-engine/README.mdsrc/Modules/FormBuilder/Axis.FormBuilder.Infrastructure/Migrations/20260519044541_AddFormWorkflowReferences.cssrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Infrastructure/Migrations/20260518034752_AddUniqueConstraintOnWorkflowName.cssrc/Modules/WorkflowEngine/Axis.WorkflowEngine.Infrastructure/Migrations/20260517150217_AddExecutionSteps.cssrc/Modules/WorkflowEngine/Axis.WorkflowEngine.Infrastructure/Migrations/20260519044503_AddWorkflowActiveStatuses.cssrc/Modules/WorkflowEngine/Axis.WorkflowEngine.Infrastructure/Migrations/20260520012358_AddWorkflowSnapshot.cstests/Modules/FormBuilder/Axis.FormBuilder.Infrastructure.Tests/Fixtures/FormBuilderDatabaseFixture.cstests/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Infrastructure.Tests/Fixtures/WorkflowBuilderDatabaseFixture.cstests/Modules/WorkflowEngine/Axis.WorkflowEngine.Infrastructure.Tests/Fixtures/WorkflowEngineDatabaseFixture.cs
…nly) Two CI failures on PR #60: 1. dependency-review fails on private repos without GitHub Advanced Security: "Dependency review is not supported on this repository." Comment the job out with a note explaining when to re-enable; Dependabot continues to cover the same threat model. 2. CodeQL init/analyze pinned to a SHA I had not verified; GitHub could not resolve it. Use the actual v4 floating-tag SHA from github/codeql-action.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/build-and-test.yml:
- Around line 124-143: Reintroduce a lightweight PR-time security gate by adding
a new job (e.g., dependency-security-gate) instead of the full commented
dependency-review job: have it checkout the repo and run two dedicated steps —
(1) NuGet/.NET audit: setup-dotnet, run dotnet restore and then run dotnet build
(or dotnet restore/build) with warnings promoted to errors for NU1900–NU1905
(use MSBuild/dotnet's -warnaserror:NU1900;NU1901;NU1902;NU1903;NU1904;NU1905 so
those NuGet audit warnings fail CI), and (2) npm/frontend audit: setup-node,
install deps and run npm audit --audit-level=high (relying on its non-zero exit
code to fail the job); name the job clearly (dependency-security-gate or
re-enable dependency-review) so reviewers can find and adjust it later.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: acb93460-0e0a-49e6-9257-976e043d30e7
📒 Files selected for processing (2)
.github/workflows/build-and-test.yml.github/workflows/codeql.yml
| # NOTE: actions/dependency-review-action requires GitHub Advanced Security | ||
| # for private repositories (paid). Re-enable this job by uncommenting once | ||
| # GHAS is provisioned, or if/when the repo becomes public. In the meantime, | ||
| # Dependabot (configured in .github/dependabot.yml) opens PRs for new | ||
| # advisories against existing dependencies, which covers the same threat | ||
| # model with a delay. | ||
| # | ||
| # dependency-review: | ||
| # name: Dependency review | ||
| # runs-on: ubuntu-latest | ||
| # timeout-minutes: 5 | ||
| # steps: | ||
| # - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 | ||
| # with: | ||
| # persist-credentials: false | ||
| # - name: Dependency review | ||
| # uses: actions/dependency-review-action@67d4f4bd7a9b17a0db54d2a7519187c65e339de8 # v4.8.0 | ||
| # with: | ||
| # fail-on-severity: high | ||
| # comment-summary-in-pr: on-failure |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For private GitHub repositories, what are the current availability/licensing requirements of actions/dependency-review-action, and what are recommended PR-time alternatives (NuGet + npm) that can fail CI on high/critical vulnerabilities without GitHub Advanced Security?
💡 Result:
Availability/licensing for actions/dependency-review-action on private repos (2026-05-23) 1) Private GitHub repositories - The dependency review action is available for private repositories only if the owning organization has a GitHub Advanced Security license. [1][2][3] - The action checks PR dependency changes and “will raise an error if any vulnerabilities or invalid licenses are being introduced.” [1][2] - License enforcement notes: license checking is not supported on GitHub Enterprise Server because the API does not return license information. [1] 2) Practical implication - If your organization does not have GitHub Advanced Security, you should not expect this action to provide the vulnerability/license enforcement experience on private repos; it is explicitly tied to Advanced Security for private repos. [1][2][3] Recommended PR-time alternatives (NuGet + npm) that can fail CI on high/critical vulnerabilities without GitHub Advanced Security A) NuGet (.NET): use NuGet Audit in a dedicated CI job and treat audit findings as failures - Microsoft documents “NuGet Audit” support for CI and describes running audits in CI while selectively treating warnings as errors in the audit pipeline. [4] - This is explicitly framed as a way to have audit runs “fail” in CI for known vulnerabilities while keeping other pipelines from being blocked. [4] B) npm (Node): use an audit tool with severity thresholds, and ensure it fails the build - npm’s built-in auditing (npm audit) can be used in CI, but you must handle reliability issues (for example, audit network timeouts can fail installs). One documented fix is to control audit behavior so the CI doesn’t fail due to network timeouts when you don’t need npm to audit as part of install. [5] - Since your requirement is “fail CI on high/critical vulnerabilities,” the recommended pattern is: 1) Disable/avoid audit during npm install (so CI doesn’t fail from transient audit endpoint issues), and 2) Run a separate security audit step (or a dedicated audit command) that enforces failure based on severity. The dotnet/aspnetcore PR specifically discusses adding audit=false to .npmrc to avoid audit being part of npm ci installs and causing failures from timeouts. [5] What I would implement as a PR-time gate (high level) - For NuGet: add a PR workflow step/job that runs NuGet Audit and configures MSBuild/CI so that audit findings cause a non-zero exit (only in the security gate job). [4] - For npm: avoid letting npm ci run audits during dependency install (to reduce audit-related CI flakiness), and run an explicit vulnerability audit command in a dedicated step that fails on high/critical. [5] Note: I did not find an authoritative single source in this round that names a specific “PR-time npm tool + exact high/critical threshold flags” equivalent to GitHub dependency-review-action. The sources above only directly support (a) that npm install audits can cause hard CI failures from timeouts and (b) that turning off install-time audit via .npmrc is a documented mitigation. [5] If you want, tell me your CI system (GitHub Actions/GitLab/etc.) and whether you use npm, yarn, or pnpm, and I’ll do a second targeted search for an authoritative npm auditing CLI that supports severity thresholds and a CI-fail contract.
Citations:
- 1: https://github.com/actions/dependency-review-action/tree/refs/heads/main
- 2: https://github.blog/changelog/2022-04-06-github-action-for-dependency-review-enforcement/
- 3: https://github.com/marketplace/actions/dependency-review
- 4: https://learn.microsoft.com/en-us/nuget/concepts/auditing-packages
- 5: Disable npm audit to prevent ETIMEDOUT build failures dotnet/aspnetcore#66465
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the referenced lines around the commented dependency-review job
sed -n '100,170p' .github/workflows/build-and-test.yml
# Confirm whether dependency-review appears elsewhere
rg -n "dependency-review" .github/workflows/build-and-test.yml .github/workflows || true
# Look for any existing dependency/vulnerability scanning jobs/steps in the workflow
rg -n "(dependabot|audit|vuln|vulnerability|security|osv|trivy|grype|snyk|npm audit|nuget audit|dependency)" .github/workflows/build-and-test.yml || trueRepository: phuong-labs/axis
Length of output: 2814
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Find other PR-time dependency/vulnerability scanning in .github/workflows =="
rg -n "(dependabot|audit|vuln|vulnerability|security|osv|trivy|grype|snyk|npm audit|nuget audit|dependency-review-action|osv-scanner|gitleaks|semgrep)" .github/workflows || true
echo
echo "== Check Dependabot config =="
if [ -f .github/dependabot.yml ]; then
sed -n '1,220p' .github/dependabot.yml
else
echo "Missing .github/dependabot.yml"
fi
echo
echo "== Search other GitHub config for dependency scanning =="
rg -n "(dependency-review|nuget audit|npm audit|osv|trivy|grype|snyk)" .github || trueRepository: phuong-labs/axis
Length of output: 3693
🌐 Web query:
npm audit --audit-level high ci fail exit code
💡 Result:
What you should expect 1) npm audit will fail in CI (non-zero exit code) when vulnerabilities are found at or above the threshold you set with --audit-level. The npm docs state that by default npm audit exits non-zero if any vulnerability is found, and that --audit-level “changes the command’s failure threshold” (it does not filter the report output) [1][2]. They also state that the command exits with code 0 if no vulnerabilities are found, and that if vulnerabilities are found, the exit code depends on the audit-level config [1][2]. 2) The specific command you gave: npm audit --audit-level high Using --audit-level=high means the audit should exit non-zero only when the results include a vulnerability with severity “high” or higher (docs example uses “moderate” as the rule, but the same mechanism applies to other levels) [1][2]. Why you might still see CI “pass” or exit 0 unexpectedly A) npm/Node version-specific behavior There is a reported npm CLI bug where exit codes were not being set properly in certain Node 20 versions: a user reported that npm audit “always exists with a 0 exit code, even if a vulnerability is found” for some versions [3]. If you’re seeing audit not failing, check your Node/npm versions and consider testing against an earlier LTS (the issue discussion reports reverting to Node 18 LTS as a workaround) [3]. B) CI scripting/step issues (not npm audit itself) Make sure the CI step is actually running npm audit (and that you’re not masking the exit code), and that the shell/CI runner is configured to fail the job on non-zero exit codes. Recommended CI usage - Run: npm audit --audit-level=high - Ensure your CI fails on any non-zero exit code from that command. If you need, you can also force a consistent behavior by pinning Node/npm versions in CI (to avoid version-specific regressions like the one reported above) [3][1].
Citations:
- 1: https://docs.npmjs.com/cli/v9/commands/npm-audit/
- 2: https://docs.npmjs.com/cli/v8/commands/npm-audit/
- 3: [BUG] exit codes not being set properly in node v20 npm/cli#6399
🌐 Web query:
NuGet Audit in CI treat findings as errors fail the build
💡 Result:
To make NuGet Audit findings fail your CI build (treat them as errors), you do this via MSBuild warning-as-error configuration using the NuGet Audit warning codes (NU1900–NU1905). NuGet Audit itself produces MSBuild warnings during restore when known vulnerable packages are used, and MSBuild can be configured to treat specific warning codes as errors so the restore/build fails. [1] 1) Fail the build for NuGet Audit warnings in the audit pipeline Use WarningsAsErrors together with the NuGetAuditCodes list (NU1900–NU1905). Microsoft Learn shows an approach that enables this only for your CI audit pipeline (e.g., via an AuditPipeline variable) and otherwise does not treat them as errors. [1] Example (.csproj or Directory.Build.props): NU1900;NU1901;NU1902;NU1903;NU1904;NU1905
Citations:
- 1: https://learn.microsoft.com/en-us/nuget/concepts/auditing-packages
- 2: https://devblogs.microsoft.com/dotnet/nugetaudit-2-0-elevating-security-and-trust-in-package-management/
- 3: https://learn.microsoft.com/en-us/dotnet/core/compatibility/sdk/9.0/nugetaudit-transitive-packages
Reintroduce a PR-time dependency vulnerability gate (fallback when GHAS isn’t available).
build-and-test.yml comments out the entire dependency-review job (lines 124-143), and there’s no other PR-time dependency vulnerability gate in the workflow; Dependabot creates follow-up PRs, but that doesn’t block the introducing PR from merging. Add a lightweight CI “security gate” instead:
- NuGet/.NET: run a dedicated audit/restore step and fail the job by treating NuGet Audit warning codes (NU1900–NU1905) as MSBuild errors in that CI job (optionally set
NuGetAuditMode=directif you only want direct deps). - npm/frontend: run
npm audit --audit-level=highin a dedicated step and rely on its non-zero exit code to fail CI (pin Node/npm if you hit exit-code inconsistencies).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/build-and-test.yml around lines 124 - 143, Reintroduce a
lightweight PR-time security gate by adding a new job (e.g.,
dependency-security-gate) instead of the full commented dependency-review job:
have it checkout the repo and run two dedicated steps — (1) NuGet/.NET audit:
setup-dotnet, run dotnet restore and then run dotnet build (or dotnet
restore/build) with warnings promoted to errors for NU1900–NU1905 (use
MSBuild/dotnet's -warnaserror:NU1900;NU1901;NU1902;NU1903;NU1904;NU1905 so those
NuGet audit warnings fail CI), and (2) npm/frontend audit: setup-node, install
deps and run npm audit --audit-level=high (relying on its non-zero exit code to
fail the job); name the job clearly (dependency-security-gate or re-enable
dependency-review) so reviewers can find and adjust it later.
CodeQL analysis ran successfully end-to-end (171 queries, no findings) but the upload step failed with "Code Security must be enabled for this repository to use code scanning" — same root cause as dependency-review: GitHub Advanced Security is required on private repos for code scanning. Remove the workflow file entirely so PRs aren't blocked by an unsupportable check. Build-and-test.yml carries an inline comment documenting both jobs (dependency-review + CodeQL) with restore instructions, so re-enabling once GHAS is provisioned is a single revert.
1 participant
Summary
Foundation hygiene PR enhancing the GitHub Actions CI for production scale. No application code semantics change — the only
src/touches are charset (BOM) fixes that the new.editorconfigsurfaced on auto-generated migration files.CI workflow (
build-and-test.yml)setup-dotnetbuilt-in NuGet cache shaves restore time across PRsdotnet format --verify-no-changesstep closes the Gate 1 gap (CLAUDE.md required it locally; CI was not enforcing — that's how the DataModeling format gunk slipped through earlier)dotnet test --logger trx+actions/upload-artifactfor PR annotations + post-mortemtimeout-minutesper job — deadlocked test no longer burns minutesdependency-reviewjob catches vulnerable transitive deps on PR (GitHub-native, fails on high+ severity)CodeQL (
codeql.ymlnew workflow)security-extended,security-and-qualityqueriesmainDependabot (
dependabot.ymlextended)Microsoft.*,WolverineFx*,OpenIddict*,Serilog*,TanStack,Vite,Biome.editorconfig (new — was missing)
dotnet formaton Windows used CRLF while CI on Linux used LF → false-positive whitespace errors locallyBOM fixes (drift-required side effect)
dotnet formatstrips. Zero behaviour change.Linked spec
Requirements & rules followed
dotnet build0/0,dotnet format --verify-no-changesgreen, all unit + integration projects green locally (Testcontainers, Docker on dev machine)./scripts/check-doc-drift.shgreen; module README touches in same PR.editorconfigas canonical .NET style config, format step now CI-enforced. Follow-up issue tracked for comprehensive style/naming rules.TODO/FIXME/ placeholder / stubFollow-up
A separate issue will track the comprehensive
.editorconfig(full C# style + naming + analyzer-severity rules) — that requires analysing existing patterns anddotnet formatmass-application, which doesn't belong bundled with CI workflow changes.Summary by CodeRabbit