Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions SECURITY.md
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,21 @@ install from a committed lock file, so what deploys is the tree that was
reviewed. The deploy authenticates to Azure with OIDC — there is no stored
publish profile or password.

The container scan blocks on **fixable CRITICAL and HIGH** findings. `ignore-unfixed`
drops anything without a patch, so what remains is actionable by definition.
Lower severities are still uploaded to the Security tab, where they can be seen
coming before they are worth failing a build over.

A separate job, `gate-probes.yml`, shows each of those gates something it must
reject and fails if any of them does not. That exists because a check which
cannot fail looks exactly like a check with nothing to report: the container
scan ran for its entire life with `exit-code: 0`, reporting findings faithfully
and blocking nothing, until 28 had accumulated behind a passing job.

That probe is why the severity scope above is trustworthy. It parses
`container-build.yml` rather than grepping it, and fails if no Trivy step can
actually fail the job — including the two ways that are easy to miss:
`continue-on-error` with nothing checking the step's outcome, and a gate built on
a SARIF-format scan, where `trivy-action` silently ignores the severity filter
and blocks on everything. The second was a real defect, found on 2026-08-17 and
fixed the same day; the probe now refuses it.
107 changes: 106 additions & 1 deletion docs/development/dependency-audit.md
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,111 @@ makes that guarantee explicit rather than incidental.

---

_See also: [Testing guide](testing.md) · [CI/CD pipeline testing](../deployment/pipeline.md)._
## 5. Worked example — the stuck Dependabot queue (August 2026)

Twenty-two Dependabot pull requests were open and red. None of them was individually wrong; they were
blocked by four separate causes, none of which a rebase could fix. It is worth reading as a
troubleshooting order, because the first check would have saved most of the time.

### Check the base branch before any individual PR

`Build API image & scan` is a required check, and it had gone red on `main` on a scheduled scan. A
required check failing on the base branch blocks **every** open pull request behind it. Nineteen of
the twenty-two were queued behind a failure none of them caused and none could clear.

```bash
gh run list --workflow container-build.yml --branch main --limit 5
```

The cause was **CVE-2026-62901** (HIGH, .NET denial of service) in runtime 10.0.10, inside the
digest-pinned `aspnet:10.0` base image. Microsoft had already published 10.0.11; nothing was
rebuilding to collect it. Digest pinning buys reproducibility and costs automatic patches, so the
pins need refreshing as routine maintenance:

```bash
docker buildx imagetools inspect mcr.microsoft.com/dotnet/sdk:10.0 --format '{{.Manifest.Digest}}'
docker buildx imagetools inspect mcr.microsoft.com/dotnet/aspnet:10.0 --format '{{.Manifest.Digest}}'
```

> **Re-running the workflow does not help.** A re-run replays the original commit — same stale
> digests, same old workflow file. Only a new head commit picks up a fixed base.

### `NU1004` — one bump, several lock files

```
error NU1004: The project references todoapp.application whose dependencies has changed.
The packages lock file is inconsistent with the project dependencies
so restore can't be run in locked mode.
```

Bumping `Microsoft.EntityFrameworkCore` in one project changes **every** `packages.lock.json` that
resolves it transitively — five of them here. Dependabot regenerates only the lock file belonging to
the project it edited, so the pull request is born failing and stays that way. Nothing about the base
branch is wrong, so rebasing changes nothing.

Fix by hand, moving the whole family in one commit:

```bash
dotnet restore TodoApp.sln --force-evaluate # regenerate every lock file
dotnet restore TodoApp.sln --locked-mode # prove the CI check now passes
dotnet build TodoApp.sln -c Release && dotnet test TodoApp.sln -c Release
```

### `codeql-action` sub-actions must move together

```
Loaded a configuration file for version '4.37.6', but running version '4.37.4'
```

`init`, `analyze` and `upload-sarif` must run the same version. Dependabot opens one pull request per
sub-action, so each one *creates* that mismatch on its own branch — whichever lands first breaks the
build. They cannot go green separately and have to move in one commit. Verify the tag SHA rather than
trusting the PR title; those said 4.37.6 while 4.37.7 was current.

### The npm side — a real advisory, held open by nothing rebuilding

`deploy.yml` gates the SPA on `npm audit --audit-level=high`, and it had been red since 8 August on:

| Severity | Package | Advisory | Path |
| -------- | ------- | -------- | ---- |
| **High** | `nanoid` 3.3.16 | [GHSA-2v37-7h3g-55p8](https://github.com/advisories/GHSA-2v37-7h3g-55p8) — custom generators can loop indefinitely when size is zero | transitive via `postcss` 8.5.25 |

Patched in **3.3.18**, which `postcss`'s `^3.3.16` range already accepts — so it is a lock-file-only
change with no `package.json` edit. Same shape as the base image: the fix existed, nothing had
rebuilt to collect it.

> **Watch what `npm audit fix` touches.** It made the right `nanoid` change, but the npm in use
> (11.x on node 24) *also* stripped `libc` fields from six optional platform-specific packages —
> metadata selecting between glibc and musl builds of native binaries. Unrelated churn inside a
> security fix is how a platform-resolution bug arrives unnoticed. Check the diff; if it reaches
> beyond the advisory, patch the single entry by hand and verify the integrity hash against
> `registry.npmjs.org`.

### Two Dependabot behaviours worth knowing

- **`@dependabot rebase` can be a silent no-op.** It replies *"already up-to-date with `<branch>`"*
when its change still applies cleanly — which is **not** the same as containing your base commits.
Verify rather than believe it, and use `@dependabot recreate` to rebuild from the current base:

```bash
git fetch origin "+refs/pull/<n>/head:refs/remotes/pr/<n>"
git merge-base --is-ancestor <fix-commit> pr/<n> && echo "has the fix" || echo "does NOT"
```

- **Check results belong to a commit, not a branch.** Merging a fix into the base does not turn a
pull request's stale red X green. Here that mattered: two branches still carried the vulnerable
`nanoid` 3.3.16 while displaying checks from a week earlier, so merging either would have
**reverted** the security fix.

### The durable fix

Both structural problems — lock files and the `codeql-action` trio — recur every time those packages
update. `groups` in `dependabot.yml` makes Dependabot open one pull request per group rather than one
per package, so related updates move together by default.

---

_See also: [Testing guide](testing.md) · [CI/CD pipeline testing](../deployment/pipeline.md) ·
[Lessons learned](../lessons.md#security-scanning-codeql-gitleaks--dependabot)._

> **← Back to the main [README](../../README.md).**
32 changes: 30 additions & 2 deletions docs/development/security-remediation.md
Original file line number Diff line number Diff line change
Expand Up @@ -286,13 +286,41 @@ been:
- scans with Trivy and uploads SARIF to the Security tab — closing the container-scanning gap
listed as still-open under M6.

Trivy reports rather than blocks: base-image CVEs appear and are fixed on Microsoft's schedule,
not ours, so failing the build would just train everyone to ignore a red X.
**Trivy blocks, and has since 2026-08-02.** The original reasoning here was that base-image CVEs are
fixed on Microsoft's schedule rather than ours, so failing the build would only train everyone to
ignore a red X. That argument does not survive `ignore-unfixed: true`, which is set on the same step:
Trivy drops everything without a patch, so whatever it still reports is fixable by definition. The 28
findings sitting in the Security tab were not waiting on Microsoft — Microsoft had already shipped,
and nothing was rebuilding to collect it.

**The gate is CRITICAL/HIGH, and only became so on 2026-08-17.** `severity: CRITICAL,HIGH` had been
set on a step using `format: sarif`, and `trivy-action` ignores `severity` when it builds SARIF — it
logs `Building SARIF report with all severities` and scans everything. `exit-code: '1'` on that step
therefore failed the job on any fixable finding at any severity, including LOW and MEDIUM, while
every comment and input in the file said CRITICAL,HIGH. The gate now hangs off the table-format scan,
where the severity input is honoured; SARIF is a separate, non-blocking step that still uploads every
severity, so the Security tab keeps the full picture.

**The scan no longer discards its own findings.** Failing the Trivy step skipped `upload-sarif`, so
the Security tab received results only on runs that had nothing to report — the findings were thrown
away in precisely the case they were collected for. A red run printed `exit code 1` and named no CVE.
The scan now runs with `continue-on-error`, the upload runs under `always()`, and a later step raises
the failure; blocking behaviour is unchanged. The table pass prints the CVEs and their fixed versions
into the log, so a failing run says out loud what it wants bumped.

**Now closed.** Base images are pinned by digest, not tag — a tag is mutable, so the same
Dockerfile could otherwise produce a different image tomorrow. The refresh command is in a comment
above each `FROM`, and the container-build workflow proves the digests still resolve.

**Digest pins need refreshing on a schedule, and nothing does it automatically.** Pinning by digest
buys reproducibility at the cost of not receiving base-image patches. On 2026-08-17 the pinned
`aspnet:10.0` still carried **CVE-2026-62901** (HIGH, .NET denial of service) in runtime 10.0.10,
patched by Microsoft in 10.0.11; the weekly scheduled scan went red and, because the container job is
a required check, blocked all 22 open Dependabot pull requests behind a failure none of them caused.
The refresh is the same maintenance done on 2026-08-02 — read the current digests with
`docker buildx imagetools inspect mcr.microsoft.com/dotnet/aspnet:10.0 --format '{{.Manifest.Digest}}'`
and update both `FROM` lines.

---

### M11 — Actions pinned to mutable tags
Expand Down
54 changes: 54 additions & 0 deletions docs/lessons.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ The ones that cost the most time — jump to the section for the full story:
- **Drag-and-drop is dead on mobile** → [frontend notes](development/frontend-notes.md): the native HTML5 DnD API is touch-blind.
- **Light mode is ignored in a phone browser** → [frontend notes](development/frontend-notes.md#darklight-mode--mobile-browsers-force-darken-a-light-only-page): mobile auto-dark force-darkens a light page; opt out with `color-scheme: only light` (the `only` keyword — `light dark` does not opt out).
- **CodeQL flags a log line as "log forging"** → [Security scanning](#security-scanning-codeql-gitleaks--dependabot): user input (e.g. `Request.Path`) logged even via structured logging; strip `\r`/`\n` before logging.
- **Every Dependabot PR is red and none of them caused it** → [Security scanning](#security-scanning-codeql-gitleaks--dependabot): a required check failing on `main` blocks every open PR behind it; fix `main` first, then re-run.
- **A red CI gate names no CVE, just `exit code 1`** → [Security scanning](#security-scanning-codeql-gitleaks--dependabot): Trivy writing SARIF prints nothing readable, and failing the step skips the upload.
- **`severity: CRITICAL,HIGH` on a Trivy step does nothing** → [Security scanning](#security-scanning-codeql-gitleaks--dependabot): `trivy-action` ignores `severity` when `format: sarif`, so the gate blocks on every severity.
- **A NuGet bump fails with `NU1004` in CI but the PR looks fine** → [Security scanning](#security-scanning-codeql-gitleaks--dependabot): one bump changes several `packages.lock.json` files; Dependabot regenerates only one.
- **`@dependabot rebase` says "already up-to-date" but the branch is behind** → [Security scanning](#security-scanning-codeql-gitleaks--dependabot): that means "my change still applies", not "contains your base commits" — use `@dependabot recreate`.

## Database (SQLite vs Azure SQL)

Expand Down Expand Up @@ -116,6 +121,55 @@ Making the repo public does **not** expose your Actions secrets — provided you
GitHub-side protections (secret scanning, push protection, the `protect-main` ruleset) — lives in
**[Secret hygiene](deployment/secret-hygiene.md)**.

### A gate that works but can't report reads as noise (August 2026)

Twenty-two Dependabot PRs sat blocked for two weeks. The instinct was that CI had become noisy and the
old runs were clutter worth deleting. Deleting them would have destroyed the evidence and left two
live HIGH vulnerabilities in place. Both gates were working correctly the entire time; neither could
say what it had found.

- **One red required check on `main` blocks every open PR.** `Build API image & scan` went red on a
scheduled scan, and because it is required, all 22 PRs queued behind a failure none of them caused
and none could clear. Check `main`'s own status before investigating any individual PR.
- **A failing Trivy step skips `upload-sarif`.** SARIF is machine-readable, so a red run printed
`exit code 1` and named no CVE — and because the step failed, the findings never reached the
Security tab. The tab received results *only* on runs with nothing to report. Fix: scan with
`continue-on-error`, upload under `always()`, raise the failure in a later step, and add a
`format: table` pass so the log names the CVE and its fixed version.
- **`trivy-action` ignores `severity` when `format: sarif`.** It logs `Building SARIF report with all
severities` and scans everything, so a gate built on that step blocks on LOW and MEDIUM while the
configuration claims CRITICAL,HIGH. Gate on the table scan, where the input is honoured, and keep
SARIF as a separate non-blocking reporter.
- **Digest-pinned base images do not receive patches.** Pinning buys reproducibility and costs you
automatic security updates. `aspnet:10.0` carried **CVE-2026-62901** (HIGH, .NET DoS) in runtime
10.0.10 until the pins were refreshed to pick up 10.0.11.
- **Re-running a workflow replays the old commit.** It does not pick up a fixed base branch — the
re-run used the same stale digests and the same old workflow file. Only a new head commit helps.

### Dependabot cannot express "these must land together" (August 2026)

Two failure modes where the PR is born red and no amount of rebasing helps, because nothing about the
base branch is wrong:

- **Lock files.** Bumping one package changes every `packages.lock.json` that resolves it
transitively — five of them here for a single `Microsoft.EntityFrameworkCore` bump. Dependabot
regenerates only the one belonging to the project it edited, so `dotnet restore --locked-mode`
fails with `NU1004`. Fix by hand: bump the family together, run
`dotnet restore TodoApp.sln --force-evaluate`, and commit every regenerated lock file.
- **`codeql-action` sub-actions.** `init`, `analyze` and `upload-sarif` must run the same version, or
the job fails with `Loaded a configuration file for version X, but running version Y`. Dependabot
opens one PR per sub-action, so each one *creates* that mismatch. They have to move in one commit.

Both are avoidable with `groups` in `dependabot.yml`, which makes Dependabot open one PR per group.

- **`@dependabot rebase` can be a silent no-op.** It replies *"already up-to-date with <branch>"* when
its change still applies cleanly — which is not the same as containing your base commits. Verify
with `git merge-base --is-ancestor <fix-commit> <pr-head>` rather than taking the reply at face
value; use `@dependabot recreate` to actually rebuild from the current base.
- **Check results belong to a commit, not a branch.** Merging a fix into the base does not turn a
PR's stale red X green, and the PR can keep showing a check from days earlier. That stale check is
also how a PR can look one green tick from mergeable while its branch would *revert* a security fix.

## Config / secrets

- Passwordless DB access uses a managed identity: enable system-assigned identity, then `CREATE USER ... FROM EXTERNAL PROVIDER` with `db_datareader` / `db_datawriter` / `db_ddladmin` roles.
Expand Down
2 changes: 2 additions & 0 deletions scripts/check-docs-drift.sh
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,8 @@ check_absent "double-submit companion cookie" \
"CSRF uses a double-submit cookie (replaced: the SPA cannot read an API-domain cookie)" || drift=1
check_absent "refresh token is persisted so a page reload" \
"the refresh token is persisted client-side (it is an httpOnly cookie)" || drift=1
check_absent "reports rather than blocks" \
"the container scan reports without blocking (it fails the build on fixable CRITICAL/HIGH)" || drift=1
[ "$drift" -eq 0 ] && ok "no known-false claims present"

# ---- 3. Controls the docs promise must actually be wired ----------------------
Expand Down