diff --git a/.claude/hooks/git-guardrails.mjs b/.claude/hooks/git-guardrails.mjs index 5099c629..f9bd8264 100644 --- a/.claude/hooks/git-guardrails.mjs +++ b/.claude/hooks/git-guardrails.mjs @@ -1,5 +1,7 @@ #!/usr/bin/env node -// PreToolUse(Bash) hook: block git invocations that violate Orbit's git workflow. +// PreToolUse(Bash|PowerShell) hook: block git invocations that violate Orbit's git +// workflow. Registered on both shell tools because a matcher naming only "Bash" leaves +// the identical command issued through the PowerShell tool completely unguarded. // Enforces the CLAUDE.md "Git workflow" rules deterministically instead of relying // on prose the model can drift past: // - "Branch protection on main. No direct pushes. Squash-merge only." @@ -8,7 +10,7 @@ // `git push` issued while HEAD is on main/master. Feature-branch pushes, new // commits, and PRs are untouched. reset --hard and checkout -- are intentionally // NOT blocked: CLAUDE.md allows them with judgment. Exits 0 silent (allow) or 2 -// with stderr feedback (block). Any error exits 0 so the hook never wedges Bash. +// with stderr feedback (block). Any error exits 0 so the hook never wedges a shell tool. import { readFileSync } from "node:fs" import { execFileSync } from "node:child_process" @@ -71,12 +73,12 @@ try { if (/^(?:main|master)$/.test(branch)) { process.stderr.write( `BLOCKED git command (Orbit git workflow):\n ${command}\n\n` + - `HEAD is on '${branch}'. Pushing from the protected branch is forbidden — switch to a feature branch and open a PR.\n`, + `HEAD is on '${branch}'. Pushing from the protected branch is forbidden. Switch to a feature branch and open a PR.\n`, ) process.exit(2) } } catch { - // Can't determine the branch (no repo at that path, git error) — fail open. + // Can't determine the branch (no repo at that path, git error): fail open. } } } diff --git a/.claude/settings.json b/.claude/settings.json index 372fe77a..6620320c 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -6,6 +6,12 @@ "hooks": [ { "type": "command", "command": "node \"$CLAUDE_PROJECT_DIR/.claude/hooks/git-guardrails.mjs\"" } ] + }, + { + "matcher": "PowerShell", + "hooks": [ + { "type": "command", "command": "node \"$CLAUDE_PROJECT_DIR/.claude/hooks/git-guardrails.mjs\"" } + ] } ] } diff --git a/.github/workflows/claude-review.yml b/.github/workflows/claude-review.yml index 7e45f3f0..33069552 100644 --- a/.github/workflows/claude-review.yml +++ b/.github/workflows/claude-review.yml @@ -18,7 +18,13 @@ jobs: fetch-depth: 0 - uses: anthropics/claude-code-action@e0cf66d1d257526b5d07f141838c338921cb8455 # v1 - if: ${{ github.actor != 'dependabot[bot]' }} + # Identify Dependabot by the pull request AUTHOR, not by github.actor. + # github.actor becomes whoever re-runs the workflow, so a human re-run of a + # Dependabot run would let this step through, and CLAUDE_CODE_OAUTH_TOKEN + # lives only in the Actions secret store, never the Dependabot one, so the + # action would run with an empty token and fail. Matches the author check + # dependabot-auto-merge.yml already uses. + if: ${{ github.event.pull_request.user.login != 'dependabot[bot]' }} with: claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} prompt: | diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml new file mode 100644 index 00000000..16c95b2c --- /dev/null +++ b/.github/workflows/codeql.yml @@ -0,0 +1,87 @@ +name: CodeQL + +# CodeQL advanced setup, replacing default setup (ORB-164). +# +# Default setup does not run on Dependabot pull requests (github/codeql-action +# issue 2858), so the three contexts branch protection requires on main, +# "Analyze (csharp)", "Analyze (javascript-typescript)" and "Analyze (actions)", +# never reported on a Dependabot head and every Dependabot pull request stayed +# permanently BLOCKED. An advanced-setup workflow is an ordinary workflow, so it +# triggers on a Dependabot head like every other job in this repository, and the +# job-level permissions block below is honoured there. GitHub's "Troubleshooting +# Dependabot on GitHub Actions" documents both halves: a Dependabot-triggered run +# "receives a read-only GITHUB_TOKEN and does not have access to any secrets", and +# "you can use the permissions key in your workflow to increase the access for the +# token", which is exactly what the analyze job does for security-events. +# +# The job name is load bearing. A matrix job produces one check run per leg, +# named by interpolating the job name, so "Analyze (${{ matrix.language }})" +# over these three languages reproduces the three required contexts verbatim. +# Renaming this job, or changing a matrix language value, silently makes main +# unmergeable for every pull request. +# +# The language set matches what default setup covered: csharp, +# javascript-typescript and actions. C# is analyzed with a real build rather +# than buildless extraction, because this repository generates code at build +# time (LoggerMessage source generators, openapi.json) and because a +# GitHub-side feature flag can turn build-mode "none" into "autobuild" without +# warning. The build steps are the ones API Tests already proves. + +on: + push: + branches: [main] + pull_request: + branches: [main] + schedule: + - cron: "0 5 * * 1" + +permissions: + contents: read + +jobs: + analyze: + name: Analyze (${{ matrix.language }}) + runs-on: ubuntu-latest + timeout-minutes: 45 + + permissions: + security-events: write + actions: read + contents: read + + strategy: + fail-fast: false + matrix: + include: + - language: csharp + build-mode: manual + - language: javascript-typescript + build-mode: none + - language: actions + build-mode: none + + steps: + - uses: actions/checkout@v7 + + - name: Setup .NET + if: matrix.build-mode == 'manual' + uses: actions/setup-dotnet@v6 + with: + dotnet-version: "10.0.x" + + - name: Initialize CodeQL + uses: github/codeql-action/init@f205ea1c3313d32999d8d6a48b4f6530d4437b38 # v4.37.4 + with: + languages: ${{ matrix.language }} + build-mode: ${{ matrix.build-mode }} + + - name: Build + if: matrix.build-mode == 'manual' + run: | + dotnet restore + dotnet build --no-restore + + - name: Perform CodeQL Analysis + uses: github/codeql-action/analyze@f205ea1c3313d32999d8d6a48b4f6530d4437b38 # v4.37.4 + with: + category: "/language:${{ matrix.language }}" diff --git a/.github/workflows/sonarcloud.yml b/.github/workflows/sonarcloud.yml index b32174c4..da54dc33 100644 --- a/.github/workflows/sonarcloud.yml +++ b/.github/workflows/sonarcloud.yml @@ -12,7 +12,6 @@ permissions: jobs: sonarcloud: name: SonarCloud Analysis - if: ${{ github.actor != 'dependabot[bot]' }} runs-on: ubuntu-latest steps: diff --git a/AGENTS.md b/AGENTS.md index 65bbb2b3..1035e109 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -30,6 +30,11 @@ copy; the gates are the enforcement. - Never push or force-push to `main`/`master`. Branch to `feature/`|`fix/`|`chore/`, open a PR, squash-merge only. Never reuse a squash-merged branch. +- Never perform an admin merge, in any shape: no `gh pr merge --admin`, no direct + `PUT /repos/{owner}/{repo}/pulls/{number}/merge`, and no GraphQL `mergePullRequest` + mutation. Naming the two raw API calls is deliberate; forbidding only the CLI flag + leaves both API paths open. The admin override exists for Thomas alone. If a merge + genuinely needs it, STOP and ask Thomas to merge it himself. - Never bypass the git hooks: no `--no-verify` (or its `-n` commit alias), no `--no-gpg-sign` and no `commit.gpgsign=false`. Fix what a hook flags, then commit. - Never `git worktree remove --force`: on Windows it follows a junction and deletes the @@ -40,6 +45,40 @@ copy; the gates are the enforcement. The fluent `migrationBuilder.CreateIndex(...)`/`DropIndex(...)` API is already safe. The Guard Migrations CI job enforces this over changed `Migrations/*.cs` files. +### Never assume an external interface. Check it, then use it. + +Before you read a field, flag, subcommand, exit code, or response shape from anything +outside this repository, confirm it exists. External means a CLI (`orca`, `gh`, `git`, +`codex`, `dotnet`), an HTTP API, or a NuGet package you did not write. + +Confirm it against the response itself, in this order: + +1. Run a real invocation and read what came back. +2. If the command writes, read the installed package's own source where it builds the + response. +3. If neither is possible, use schema introspection. + +`--help` proves that a flag or a subcommand exists and nothing whatever about a response +body, so it never confirms a field. Documentation, your memory of a similar tool, and what +the shape "should" obviously be are never authority. + +**Never satisfy an assumption by writing the fixture that agrees with it.** You author both +the code and its test double, so a fixture built from a guess makes the suite prove that +your code matches your belief, not that it works. A green test over an invented field is +worth less than no test at all, because it buys false confidence. + +**If you cannot confirm the field, do not read it.** Redesign so the unknown is not on the +path: use a value the interface already returns and the codebase already reads, or make the +operation's success depend only on the exit code. Say in the pull request body which field +you wanted and why you could not confirm it. Treating an unconfirmed field as a failure +signal is not failing closed, it is inventing a failure, because "the call failed" and "the +field does not exist" arrive as the same value. + +When your change reads an external field, put the evidence in the pull request body, +whether or not the codebase already reads that field elsewhere. An existing read is not +evidence: it may be the unverified guess this rule exists to catch. You may cite the pull +request that first proved the field instead of running the command again. + ## Code Review Rules Only what no gate can check; mechanical findings belong to CI. Flag P0/P1 only. @@ -55,3 +94,8 @@ Only what no gate can check; mechanical findings belong to CI. Flag P0/P1 only. 4. **A background job or notification that assumes server-local "today"** (schedule windows, streak cutoffs): correctness depends on per-user timezones and no test asserts the boundary hour. +5. **A field, flag, exit code, or response shape read from an external interface with no + evidence in the pull request body, or a test fixture asserting a shape no evidence + supports.** No CI job can see this: the suite is green precisely because the author + wrote both the code and the fixture. Safe path: the complete redacted response shape + plus a way to re-derive it, or a design that does not read the unconfirmed field.