ci: exercise migrations against MySQL and PostgreSQL#1629
Conversation
The existing `test` job builds the schema via `dataSource.synchronize()` in
`apps/<service>/test/app/database.ts`, so migration files in
`apps/<service>/src/adapters/database/migrations/{mysql,postgres}/` are never
exercised. SQL syntax errors and broken `down()` regressions only surface in
production deployments.
Add a `tests-migrations` job that runs the migration CLI end-to-end for
server-core, server-storage, and server-telemetry against fresh MySQL 9 and
Postgres 18 containers: `run` -> revert all in reverse order -> `run`
(idempotency replay). Every `up()` / `down()` is exercised, not just the latest.
A pre-flight step counts compiled migrations and fails loudly if zero — without
this guard, typeorm silently reports "No migrations are pending" with exit 0
when the CLI runs from the wrong working directory, which is the same silent-
pass failure mode this job exists to fix.
Pin `docker-compose.yml` images to `mysql:9` and `postgres:18` to match the
versions the existing test job uses, so local migration runs match CI.
Refs authup/authup#3067, authup/authup#3068.
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds CI migration validation and docs: a ChangesDatabase Migration Testing
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 @.agents/testing.md:
- Around line 249-257: Update the local testing instructions to match the CI
flow by replacing the single forward migration step with a full run → revert xN
→ run sequence and add a Postgres environment-variable variant; specifically,
expand the shown CLI invocation (the DB_TYPE=... node dist/cli/index.mjs
migration run example) to demonstrate running migrations, reverting the last N
migrations (revert command with an example N), and re-running migrations, and
provide both MySQL and Postgres env examples (DB_TYPE=postgres with DB_PORT,
DB_USERNAME, DB_PASSWORD, DB_DATABASE adjusted) so the documented local flow
mirrors CI and covers both databases.
In @.github/workflows/main.yml:
- Around line 139-164: The tests-migrations job is inheriting default token
permissions; add an explicit least-privilege permissions block to that job by
inserting a permissions: mapping under the tests-migrations job (the job named
tests-migrations in the workflow) with at minimum contents: read so the job only
has repository read access for checkout; ensure the permissions mapping is at
the same indentation level as name/needs/runs-on and does not grant any other
scopes.
- Around line 194-196: The MySQL readiness probe uses `mysqladmin ping` without
authentication; update the Docker Compose exec command (the branch handling
`matrix.db = "mysql"`) to pass the configured root password so the probe
authenticates (e.g., include `-pstart123` or read `MYSQL_ROOT_PASSWORD`) when
running `mysqladmin ping -h127.0.0.1 --silent` to ensure consistent, reliable
health checks across MySQL image versions.
- Around line 166-167: The workflow uses a floating actions/checkout tag ("uses:
actions/checkout@v6") which is vulnerable to tag hijacking and leaves
credentials persisted; update the Checkout step ("name: Checkout") to pin
actions/checkout to a specific commit SHA (replace `@v6` with @<commit-sha>) and
add a with: persist-credentials: false setting (and optionally fetch-depth: 1)
so credentials are not persisted to the workspace.
🪄 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
Run ID: 5273e77f-46ab-40a4-9efc-fce1eb692ecd
📒 Files selected for processing (3)
.agents/testing.md.github/workflows/main.ymldocker-compose.yml
| tests-migrations: | ||
| name: Test Migrations | ||
| needs: [build] | ||
| runs-on: ubuntu-latest | ||
|
|
||
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| service: [ server-core, server-storage, server-telemetry ] | ||
| db: [ mysql, postgres ] | ||
| include: | ||
| - db: mysql | ||
| port: 3306 | ||
| user: root | ||
| - db: postgres | ||
| port: 5432 | ||
| user: postgres | ||
|
|
||
| env: | ||
| DB_TYPE: ${{ matrix.db }} | ||
| DB_HOST: 127.0.0.1 | ||
| DB_PORT: ${{ matrix.port }} | ||
| DB_USERNAME: ${{ matrix.user }} | ||
| DB_PASSWORD: start123 | ||
| DB_DATABASE: app | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -type f -name "main.yml" | head -20Repository: PrivateAIM/hub
Length of output: 229
🏁 Script executed:
fd "main.yml" .github/workflows/Repository: PrivateAIM/hub
Length of output: 85
🏁 Script executed:
cat -n .github/workflows/main.yml | sed -n '135,170p'Repository: PrivateAIM/hub
Length of output: 1349
🏁 Script executed:
cat -n .github/workflows/main.yml | sed -n '165,230p'Repository: PrivateAIM/hub
Length of output: 3149
Add explicit least-privilege permissions for tests-migrations.
This job currently inherits default token permissions. Set an explicit minimal block with contents: read to avoid accidental over-privilege drift. The job only requires repository read access for the checkout step.
Suggested hardening
jobs:
tests-migrations:
+ permissions:
+ contents: read
name: Test Migrations🧰 Tools
🪛 zizmor (1.25.2)
[warning] 139-213: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
🤖 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/main.yml around lines 139 - 164, The tests-migrations job
is inheriting default token permissions; add an explicit least-privilege
permissions block to that job by inserting a permissions: mapping under the
tests-migrations job (the job named tests-migrations in the workflow) with at
minimum contents: read so the job only has repository read access for checkout;
ensure the permissions mapping is at the same indentation level as
name/needs/runs-on and does not grant any other scopes.
| - name: Checkout | ||
| uses: actions/checkout@v6 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n .github/workflows/main.yml | sed -n '160,175p'Repository: PrivateAIM/hub
Length of output: 654
🌐 Web query:
GitHub Actions checkout action security best practices pinning commit SHA
💡 Result:
Pinning GitHub Actions to a full-length commit SHA is considered the industry standard for supply chain security [1][2][3]. Unlike version tags (e.g., @v4) or branch references (e.g., @main), which are mutable and can be moved or compromised to point to malicious code, a full-length commit SHA provides an immutable, cryptographically verifiable reference [4][5][6]. Why Pinning to SHA is Critical 1. Prevents Tag Hijacking: Attackers can gain control of a repository or its maintainer accounts and move existing version tags to point to "imposter" commits containing malicious payloads, such as credential scrapers [7][8][3]. Workflows using tag-based references will automatically execute this malicious code without warning [7][3]. 2. Ensures Immutability: Once a commit exists in Git, its SHA cannot be altered without changing the code itself [5]. Pinning to a SHA guarantees that your workflow executes the exact, validated code every time [4][5]. 3. Enables Policy Enforcement: GitHub now allows administrators to enforce full commit SHA pinning at the repository and organization levels [9][10]. Workflows that do not use a full SHA will fail, preventing the accidental use of unpinned actions [5][10][6]. Best Practices for Implementation - Use Full-Length SHAs: Always use the full-length commit SHA, not a truncated version [1][2]. - Include Descriptive Comments: To maintain readability and ease of updates, include the version tag in a comment on the same line [11][12][9]. Example: uses: actions/checkout@b4ffde6 # v4.1.1 - Automate Updates: Manually tracking SHAs can be burdensome [5][3]. Use automated dependency management tools like Dependabot or Renovate to monitor for updates and create pull requests when new, safe versions are available [11][3][6]. - Adopt Cooldown Periods: To further mitigate risk, implement a "cooldown" or minimum age policy (e.g., 7-14 days) before adopting new action versions [6]. This waiting period helps ensure that any malicious code introduced via a new release is discovered and remediated before you incorporate it into your environment [6]. - Monitor Transitive Dependencies: Even if your top-level action is pinned, ensure the action itself does not pull in unpinned dependencies [5][6]. Organizations may enforce this by applying SHA-pinning policies that inspect the entire action dependency tree [5]. If you find an action that is not pinned, you can typically retrieve its current SHA using the command: git ls-remote https://github.com/[owner]/[repo] [tag/branch] [11]
Citations:
- 1: https://docs.github.com/en/actions/reference/security/secure-use
- 2: https://dev.to/nkuziev-sec/why-fixed-container-image-versions-matter-lessons-from-the-trivy-supply-chain-attack-29ke
- 3: https://www.aikido.dev/blog/checklist-github-actions
- 4: https://www.stepsecurity.io/blog/pinning-github-actions-for-enhanced-security-a-complete-guide
- 5: https://www.romainlespinasse.dev/posts/github-actions-commit-sha-pinning/
- 6: https://www.wiz.io/blog/github-actions-security-guide
- 7: https://gbhackers.com/compromised-github-action/
- 8: https://undercodenews.com/github-action-supply-chain-attack-exposes-ci-cd-secrets-through-memory-scraping-malware/
- 9: https://dev.to/ciberconscientes/github-actions-why-you-should-pin-your-actions-to-a-specific-version-1513
- 10: https://github.blog/changelog/2025-08-15-github-actions-policy-now-supports-blocking-and-sha-pinning-actions/
- 11: Pin all GitHub Actions to immutable SHA commits for supply chain security github/gh-aw#5691
- 12: Pin GitHub Actions to commit SHAs in release workflow github/gh-aw#10021
🌐 Web query:
GitHub Actions checkout action persist-credentials false security best practice
💡 Result:
Setting persist-credentials: false in the actions/checkout action is considered a security best practice to prevent the automatic storage of Git authentication credentials (such as the GITHUB_TOKEN) within the runner's local Git configuration [1][2][3]. By default, actions/checkout configures Git to use a provided token (often the GITHUB_TOKEN) to authenticate subsequent Git operations (e.g., git fetch, git push) [1][4]. When persist-credentials is true (the default), these credentials are saved in a way that makes them accessible to any other process, step, or action running on the same runner [4][3][5]. If a workflow step or a third-party action is compromised, an attacker could potentially scrape these credentials from the disk [4][2][5]. Key considerations include: - Purpose of persist-credentials: false: It ensures that sensitive tokens are not stored in the environment where they could be inadvertently exposed, especially if the repository or artifacts are archived or accessed by untrusted code [2][3]. - Modern Mitigation: While recent updates to actions/checkout have improved security by moving credential storage to a separate, isolated location under $RUNNER_TEMP (using Git's includeIf mechanism) rather than the main .git/config file, explicitly setting persist-credentials: false remains the recommended best practice for workflows where authenticated Git operations are not required [1][6][7]. - Defense in Depth: Because the GITHUB_TOKEN is automatically available to many workflows, simply setting persist-credentials: false is not a complete security solution [4][5]. You should also enforce the principle of least privilege by explicitly defining the required permissions for your jobs at the workflow or job level (e.g., setting contents: read rather than the default, more permissive settings) [4][8][9]. To implement this, add the parameter to your checkout step: yaml - uses: actions/checkout@v4 with: persist-credentials: false Using this setting ensures that your workflow does not leave unnecessary, persistent credentials on the runner disk [2][3].
Citations:
- 1: https://github.com/actions/checkout/blob/main/README.md
- 2: https://cheatsheetseries.owasp.org/cheatsheets/GitHub_Actions_Security_Cheat_Sheet.html
- 3: https://yossarian.net/til/post/actions-checkout-can-leak-github-credentials/
- 4: Remove
persist-credentialsor change the default tofalseactions/checkout#485 - 5: Remove
persist-credentialsor change the default tofalseactions/checkout#485 - 6: Persist creds to a separate file actions/checkout#2286
- 7: https://github.com/actions/checkout
- 8: https://www.aikido.dev/blog/checklist-github-actions
- 9: https://dev.to/devactivity/boosting-github-productivity-solving-docker-pull-permissions-in-cicd-3cpj
Harden checkout: pin to a commit SHA and disable credential persistence.
Using a floating action tag (@v6) and persisted credentials weakens supply-chain security. Commit SHA pinning prevents tag hijacking attacks where compromised maintainer accounts can move version tags to malicious code, while disabling credential persistence prevents token exposure if workflow steps are compromised.
Suggested hardening
- - name: Checkout
- uses: actions/checkout@v6
+ - name: Checkout
+ uses: actions/checkout@<pinned_commit_sha_for_v6>
+ with:
+ persist-credentials: false🧰 Tools
🪛 zizmor (1.25.2)
[warning] 166-167: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 167-167: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 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/main.yml around lines 166 - 167, The workflow uses a
floating actions/checkout tag ("uses: actions/checkout@v6") which is vulnerable
to tag hijacking and leaves credentials persisted; update the Checkout step
("name: Checkout") to pin actions/checkout to a specific commit SHA (replace `@v6`
with @<commit-sha>) and add a with: persist-credentials: false setting (and
optionally fetch-depth: 1) so credentials are not persisted to the workspace.
There was a problem hiding this comment.
Pull request overview
Adds a dedicated CI job to exercise TypeORM migrations end-to-end (apply all → revert all → re-apply) against fresh MySQL and PostgreSQL containers for the server services, ensuring migration SQL is actually validated in CI rather than relying on dataSource.synchronize().
Changes:
- Add
tests-migrationsjob in CI to run migration CLI forward, full revert, and idempotency replay forserver-core,server-storage, andserver-telemetryacross MySQL/PostgreSQL. - Add a pre-flight guard that fails if no compiled migrations are present in
dist/.../migrations/<db>/. - Pin
docker-compose.ymlDB images to match the versions already used by the CItestjob.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
docker-compose.yml |
Pins MySQL/PostgreSQL images used for local/CI compose-based DB startup. |
.github/workflows/main.yml |
Introduces the tests-migrations job to run migration CLI against MySQL/PostgreSQL with a compiled-migrations sanity check. |
.agents/testing.md |
Documents why migrations need separate CI coverage and how to run the flow locally. |
Comments suppressed due to low confidence (1)
docker-compose.yml:9
- The MySQL service healthcheck runs
mysqladmin pingwithout-p...even thoughMYSQL_ROOT_PASSWORDis set. This can keep the container in an unhealthy state (and can confuse local dev / CI troubleshooting). Consider adding the password (and user if needed) to the healthcheck command, consistent with the GitHub Actionstestjob health-cmd.
image: mysql:9
restart: always
healthcheck:
test: [ "CMD", "mysqladmin" ,"ping", "-h", "localhost" ]
interval: 3s
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - name: Wait for ${{ matrix.db }} to be ready | ||
| run: | | ||
| if [ "${{ matrix.db }}" = "mysql" ]; then | ||
| timeout 120 bash -c 'until docker compose exec -T mysql mysqladmin ping -h127.0.0.1 --silent; do sleep 2; done' |
- mysqladmin ping now passes -uroot -pstart123 to avoid Access denied on MySQL 8+ images (matches the existing test job). - Expand the local repro in .agents/testing.md to show the full run -> revert xN -> run cycle and a Postgres env variant.
The release commit 14e5cca bumped apps/client-ui/package.json (and others) to use ^0.9.0 of @privateaim/kit / @privateaim/server-kit, but the package-lock.json node for apps/client-ui still recorded the previous ^0.8.31 constraints. As a result npm ci could not satisfy the lockfile and failed with 'Missing: @privateaim/kit@0.8.43 from lock file', breaking CI on master and every branch since the release. Regenerating the lockfile records the workspace 0.9.0 versions plus the 0.8.43 registry copies that server-storage-kit (^0.8.21) and server-test-kit (^0.8.37) still pull in — no package.json changes.
server-storage-kit and server-test-kit referenced @privateaim/kit / @privateaim/server-kit at ^0.8.21 / ^0.8.37, but the workspace versions are now 0.9.0. npm therefore pulled the registry copies of those packages into nested node_modules — and the published 0.8.43 tarballs do not contain the dist/ directory, so any consumer of server-storage-kit or server-test-kit failed at import time with: Error [ERR_MODULE_NOT_FOUND]: Cannot find module '.../server-storage-kit/node_modules/@privateaim/server-kit/dist/index.mjs' Bumping the constraints to ^0.9.0 makes npm use the local workspace package (which has dist/), eliminating the nested broken installs. Drops 102 lines of stale registry entries from package-lock.json that were added by the previous regenerate.
Mirrors authup/authup@cb89ead85: - Migration count step now uses 'shopt -s nullglob' + bash array glob instead of 'ls | wc -l'. Under set -e -o pipefail (GHA default) the pipe form can fail when the directory does not exist — which is precisely the case this step is designed to detect. - mysqladmin readiness probe now passes the password via the MYSQL_PWD environment variable rather than -p on the command line, eliminating the 'Using a password on the command line interface can be insecure' warning.
Aligns the migration job with the existing test job in this workflow, which already uses GHA services: for mysql:9 / postgres:18. Removes the separate 'Start mysql/postgres' and 'Wait for ... ready' steps — GHA manages container lifecycle and health-check polling. The docker-compose.yml still exists for local development; only the CI job switches off it. docs say to run 'docker compose up' locally; that remains accurate.
Summary
tests-migrationsCI job runs the migration CLI end-to-end against fresh MySQL and PostgreSQL containers forserver-core,server-storage, andserver-telemetry:migration run→ revert all in reverse order →migration run(idempotency replay). Everyup()/down()gets exercised, not just the latest migration.apps/<service>/dist/adapters/database/migrations/<db>/and fails loudly if zero. Without this guard, typeorm silently reports "No migrations are pending" with exit 0 when the CLI runs from the wrong working directory — the same silent-pass failure mode this job exists to fix.docker-compose.ymlimages tomysql:9andpostgres:18to match the versions used by the existingtestjob, eliminating drift between the migration job and local development.Why
The existing
testjob in.github/workflows/main.ymlruns against MySQL and PostgreSQL — but the test database module overrides the schema bootstrap withdataSource.synchronize()(seeapps/<service>/test/app/database.ts), which builds tables from entity metadata. Migration SQL files inapps/<service>/src/adapters/database/migrations/{mysql,postgres}/are silently ignored, so broken DDL only surfaces in production deployments.Mirrors the fix landed in authup/authup#3068 (issue authup/authup#3067).
What it catches
down()regressions in any migration (not just the latest)What it still doesn't catch
UPDATE/INSERTmigrations against pre-existing rows (the migrations run against an empty DB). Listed as the stretch goal in CI: migrations are not exercised against MySQL or PostgreSQL authup/authup#3067 — not addressed here.Test plan
tests-migrationsmatrix combinations (3 services × 2 DBs) greendist/adapters/database/migrations/<db>/is empty1 migrations were found in the source codeline on firstmigration run, the matching reverted lines during the revert loop, and1 migrations are new migrations must be executedon the idempotency replayLocal verification (already done): full
run → revert × N → runcycle against the docker-compose MySQL and PostgreSQL containers for all three services — every migration applied, reverted, and re-applied cleanly.Summary by CodeRabbit
Chores
Documentation