Repository navigation
Run the tests, database checks and secret scan on every pull request (app review batch 6a) - #27
Merged
Merged
Conversation
infra/neon/tests/run-local.sh builds a local-mode database (bootstrap.sql, every migration, after-core.sql) and a stub-mode one (local_auth_stub.sql and every migration), then runs each SQL test on its own copy of the one its header names, and the lock order test the same way. Tests written for a Neon branch are skipped and said to be. It reads the connection from the PG* variables and accepts only a loopback server, since the auth stand-ins create cluster roles that must never reach Neon. Every database it makes carries a random run id that no database had when the run began, creating one fails rather than reuse a taken name, and cleanup (also on an interrupt) drops only those. --create builds one database for the local API and worker tests. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A6P7aTcMBccooYYr4xYnVS
The local API's sign-in throttle emptied its whole list once it held more than 1,000 entries, so failing to sign in as 1,001 other addresses lifted every running lockout and gave a guesser ten more tries. Entries now stay in order of their latest failure and each failure drops only the expired ones from the front, so a lockout runs its full 15 minutes. apps/local-api/tests/sign-in-lockout.mjs locks an account out, fails 1,001 other sign-ins and checks the account is still locked (it got 200 for the right password before this change). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A6P7aTcMBccooYYr4xYnVS
Setting RESEND_API_KEY, RESEND_FROM and APP_URL, perhaps for the worker, was enough for the local API to email real invitations from a development machine. It now also needs LOCAL_API_SEND_EMAIL=yes, and logs at startup whether invitation email is on or off and why. Without the opt-in the sender gets the existing "not configured here" answer (503) and can copy the link as before. The decision lives in apps/local-api/src/invitation-email.ts so tests/invitation-email-opt-in.mjs can check it without a server. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A6P7aTcMBccooYYr4xYnVS
The root package.json now names pnpm 10.34.6 in packageManager, so CI, corepack and pnpm itself all use one version instead of whatever is installed. 10.34.6 reads the current lockfile unchanged. The hash lets corepack check the download. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A6P7aTcMBccooYYr4xYnVS
vite 7.3.2 is in range of two advisories fixed in 7.3.5: a server.fs.deny bypass through Windows alternate paths, and the bundled launch-editor disclosing an NTLM hash through UNC paths. The catalog now asks for ^7.3.5, which resolves to 7.3.7, the current 7.3 release. The lockfile changes only vite and the references to it: the peer suffixes of the packages that depend on it, and vite's own fdir, picomatch and tinyglobby, now the versions vitest already uses. Nothing new is downloaded beyond vite itself. minimumReleaseAge is unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A6P7aTcMBccooYYr4xYnVS
ShellCheck prefers a glob to parsing ls. The count is only for the progress line. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A6P7aTcMBccooYYr4xYnVS
Finding 20 from the app review: nothing ran the tests automatically. .github/workflows/ci.yml has three jobs, each with a time limit and read-only access to the repository: - web: installs the web app, local API and reminder worker with the frozen lockfile, typechecks all three, runs the web tests and builds. - database: on a PostgreSQL 16.15 service, runs every test in infra/neon/tests through run-local.sh, starts the local API on its own database and runs its HTTP tests, then runs the reminder worker's tests, each on a fresh database. The service sorts text as C.UTF-8, like the local servers the tests were written against. - secrets: gitleaks, built from source with go install, scans the files and the commits the pull request or push adds, with findings redacted. Older history is left out: it still holds the key from finding 1. Actions are pinned to full commit hashes. Nothing deploys or uses a secret. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A6P7aTcMBccooYYr4xYnVS
The database README points at run-local.sh, the local README says invitation email needs LOCAL_API_SEND_EMAIL=yes as well as the Resend settings, the local API README covers the two new tests, and the handoff records CI and the pnpm and vite pins. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A6P7aTcMBccooYYr4xYnVS
…hole The secret scan read only the commits a change added, so after a force push, or on the first push of a branch, it read the newest commit alone and a secret added and removed within the push went unseen. It now reads every commit behind the checkout, which takes about eight seconds. .gitleaksignore leaves out the two old commits that hold the Google API key from app review finding 1, by fingerprint only. That key still has to be revoked. run-local.sh checked PGHOST, but libpq connects to PGHOSTADDR or a PGSERVICE entry instead when they are set. It now refuses to start while PGHOSTADDR, PGSERVICE or PGSERVICEFILE is set. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A6P7aTcMBccooYYr4xYnVS
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
The first run on GitHub failed the history scan. This container's clone was shallow, so the scan here had seen 318 commits; GitHub's checkout holds all 658, and two more findings sit in the older ones. The Google API key from app review finding 1 is also in docs/SESSION-REFERENCE.md at c5fae88 (the same key, compared by hash), and the earlier Supabase app's web push (VAPID) private key is in apps/web/.env.example at fb1d3c8. Neither is in the files today. Both join .gitleaksignore by fingerprint (commit, file, rule and line; no secret), so any other secret, old or new, still fails the scan. With the full history fetched, the scan here now sees the same 658 commits and passes. The Google key is the one already to be revoked; the web push key belongs to the old app's send-notification function and should be revoked there too. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A6P7aTcMBccooYYr4xYnVS
Owner
Author
|
The Vercel preview for Generated by Claude Code |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
This branch had an error being deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Before: nothing ran the tests on a pull request, so a change could reach
mainwith a failing test, a broken build or a committed secret; that is how the lost security headers and the re-committed key got there. The database tests could only be run one file at a time by hand. On a development machine, failing to sign in as 1,001 other addresses lifted every running sign-in lockout, setting the Resend keys for the worker was enough for the local API to email real invitations, any installed pnpm was used (pnpm 9 fails on the lockfile), and vite 7.3.2 carried two development server advisories.After: every pull request and every push to
mainruns three checks, "Web app", "Database, local API and worker" and "Secret scan", andinfra/neon/tests/run-local.shruns every database test locally the same way. A lockout runs its full 15 minutes, the local API emails invitations only when told to, pnpm is pinned to 10.34.6 and vite is 7.3.7.This is batch 6a of the app review fixes. It is built on #26 (batch 3), so the checks also run on the batches stacked above it; the diff here shows only this batch.
What changed, by finding
.github/workflows/ci.yml. Web app installs the web app, local API and reminder worker from the frozen lockfile, typechecks all three, runs the web tests and builds. Database, local API and worker runs every test ininfra/neon/testson fresh PostgreSQL 16 databases, then the local API's HTTP tests against its own database, then the reminder worker's tests. Secret scan builds gitleaks 8.30.1 from source and scans the files and every commit behind them, so a secret added and removed again is still caught, with anything found redacted..gitleaksignoreleaves out, by fingerprint only, the three old commits holding the key from finding 1 and the one holding the earlier Supabase app's web push key. Every action is pinned to a commit, the token is read-only, and nothing deploys or uses a secret. ESLint and type-checking the test files come in batch 6b.packageManagerpins pnpm 10.34.6 with its hash. It reads the current lockfile unchanged, and pnpm 11 switches to it when run in this repository.run-local.shbuilds a local-mode and a stub-mode database from the migrations and runs each test on a copy, refusing any host but loopback (including throughPGHOSTADDRor a service file) and dropping only the databases it made. The local API's lockout list now drops only expired entries, so a running lockout survives any number of other failures. Real invitation email from the local API needsLOCAL_API_SEND_EMAIL=yesas well as the Resend settings, and the API says at start-up which way it is set. vite moves to 7.3.7; the lockfile changes vite, the references to it, and vite's own picomatch and tinyglobby, which move to versions the lockfile already held.Needs the owner
apps/web/.env.example; the current app sends no web push. The Google key from finding 1 is in a third old commit as well, and revoking it covers all three.actions/checkout,actions/setup-node,actions/setup-goandpnpm/action-setup.main. A run uses about 5 to 6 runner minutes.pnpm install --frozen-lockfile --filter @commune/web(inapps/web/vercel.jsonor the project settings) would stop production builds installing every workspace app, including the mobile app's GitHub-hosted package..deps-weblocally to pick up vite 7.3.7.How it was checked
Each job's steps were run as written, in order, on a fresh clone of this branch, with a local PostgreSQL 16.15 server standing in for the service container and pnpm 10.34.6 taken from the pin:
Separately: actionlint and ShellCheck report nothing; a made-up token planted in the files, in a new commit, or added and then removed in the same change fails the secret scan without the token appearing in any output; and a failing local API or worker test fails its job, the local API's log is printed, and every database is still dropped. The new lockout test failed on the old code (the right password was accepted after 1,001 other failures) and three of the four opt-in tests fail if the opt-in check is removed.
On GitHub, the first run's Web app and Database jobs passed, setup actions, service container and health check included, in about two and a half minutes each.
How
The database job reaches PostgreSQL only on loopback with throwaway credentials, and
run-local.shnames every database it makes with a prefix and a random run id, so an interrupted run cleans up after itself and never touches anything else. The service sorts text as C.UTF-8, like the local servers the tests were written against. gitleaks is built withgo install, which checks the source against Go's checksum database, rather than run through its GitHub Action, which needs a licence for an organisation's repository.🤖 Generated with Claude Code
https://claude.ai/code/session_01A6P7aTcMBccooYYr4xYnVS