ci(gates): Make API security, build, and artifact checks non-skippable (#395) - #443
Merged
dDevAhmed merged 2 commits intoSep 28, 2026
Conversation
DigiNodes#395) Signed-off-by: tomasbogado203-bit <tomasbogado203@gmail.com>
Contributor
|
@tomasbogado203-bit intake review of head
Please obtain issue authorization and the remediation scope decision first, then resolve conflicts and request fresh exact-head CI/review. |
Contributor
Maintainer stabilization triageNo merge or closure action is being taken now. This PR is on hold until the next Stellar Wave starts and the relevant baseline is green. Provisional disposition: Retain for later rebase. Next review: Re-evaluate the CI-gate changes after STAB-BE-001 and STAB-BE-002 restore the dependency/container baseline. Keep CI changes separate from dependency migration. The provisional cross-repository disposition is recorded in truthbounty-protocol PR #7. Existing issues remain open. Please do not rework or rebase this PR unless a maintainer explicitly activates and assigns the work. |
5 tasks
dDevAhmed
pushed a commit
that referenced
this pull request
Sep 25, 2026
Four stabilisation issues in one pull request, because they touch the same files and must stay consistent with each other. The theme is that several checks were reporting green without testing anything real. #516 STAB-BE-002 - reproducible container build Changed: the Dockerfile builder stage now runs `npm ci` instead of `npm install`, so the committed package-lock.json is authoritative and dependency drift fails the build instead of silently re-resolving. Added: .github/workflows/container-smoke.yml, which builds the image from a pristine `git archive` export with --pull --no-cache, proves a build with a deliberately drifted package.json is rejected, asserts the shipped image carries its runtime artifacts and carries no dev-only dependencies or environment files, and reports the declared Healthcheck/Cmd/User. Added: a "Supported Runtime and Toolchain" section to docs/DEPLOYMENT.md and `engines.npm: ">=10 <11"` in package.json, so the supported npm major is declared rather than assumed. `engines.node` is unchanged. Not touched, by instruction: the Dockerfile runner stage, `.dockerignore`, and any USER/HEALTHCHECK addition. Issue #498 (memplethee-lab) owns those; the two changes are in different Dockerfile stages so they merge cleanly. The `npx prisma generate` step in the builder is retained, consistent with the #517 work in this same commit and with open issue #392. Whether it should remain is a decision for #392 and is recorded in docs/PRISMA_INVENTORY.md rather than being silently taken here. #517 STAB-BE-003 - TypeORM-only baseline Changed: the real bug. ci.yml's "Run migration tests" step ran `npx prisma migrate reset --force` / `npx prisma migrate deploy`, which exercised the legacy Prisma migration set. The 17 TypeORM migrations in src/migrations/, the ones the application actually runs, were never applied, never rolled back and never compared against the entities. Replaced with a dedicated `schema-migration-gate` job that, against an empty PostgreSQL service database via src/config/data-source.ts: applies every migration in order; runs `npm run migration:generate` and fails if TypeORM can write a new migration (entities and committed migrations disagree), while treating a generator crash that is not "No changes in database schema were found" as an unresolved failure rather than a pass; then reverts and re-applies the latest migration to prove reversibility. Added: docs/PRISMA_INVENTORY.md, a repository search report classifying every remaining Prisma reference as load-bearing or dead, with reasons. Changed: CONTRIBUTING.md now states TypeORM is the persistence layer and that migrations run via the `migration:*` scripts. Changed: docs/DEPLOYMENT.md told operators to run `npx prisma migrate deploy` against the application schema. That is the wrong tool; it now documents `npm run migration:run` / `migration:revert`. Removed: the dead `/generated/prisma` entry in .gitignore. The generator output is pinned to src/generated/client, so that path can no longer exist. NOT met, and not claimed: "production and test dependency graphs contain no Prisma runtime/tooling packages". Prisma is load-bearing in this repository. src/prisma/prisma.service.ts is imported by production code in eleven modules (identity, auth, analytics, outbox, notifications, sybil-resistance, ai-assistant) and src/identity/identity.service.ts imports types straight from @prisma/client. src/dockerfile.spec.ts also asserts on prisma/schema.prisma, so deleting the schema breaks an active test. Removing the packages would break module resolution at bootstrap, not just at query time. Removing prisma/migrations/ is a data-retention decision, since they describe data that exists. #518 STAB-BE-004 - dependency vulnerability remediation Added: docs/DEPENDENCY_SECURITY.md. No scanner was run, so it contains no severities, advisory ids or CVSS scores; inventing any of those would be fabrication. It is a scaffold with a real procedure: the declared-versus- resolved version table read from package.json and package-lock.json, the finding table with the columns the acceptance criteria require (package path, severity, exploitability/runtime reachability, owner, follow-up, status), a reachability taxonomy the scanner cannot supply, the batching rule, and the risk-acceptance record format maintainers must file for any residual critical finding. Changed: `npm audit --audit-level=high` now writes its output to npm-audit-report.txt (via tee under `set -o pipefail`, so npm's non-zero exit on a high finding still fails the workflow) and that file is uploaded as the `npm-audit-report` artifact with 90-day retention, so the evidence for a given commit is retrievable. The existing TruffleHog and CodeQL steps and the Trivy image scan are unchanged. Changed: `security-scans` now runs `npm ci`, so the audit is evaluated against the tree that actually ships and every job installs deterministically. Not changed: no dependency version was bumped and package-lock.json was not touched. A lockfile edit that cannot be regenerated and verified locally is precisely the unverifiable change this issue warns about. The remediation batches are documented instead. #519 STAB-BE-005 - required CI gate set Changed: every `uses:` in ci.yml is now pinned to a 40-character commit SHA with a `# vX.Y.Z` comment. This includes the two that were previously pinned to mutable branches, trufflehog@main and trivy-action@master, which are now pinned to release tags v3.97.9 and v0.36.0. Every SHA was resolved from the GitHub API against the corresponding release tag; none was written from memory, and none was left as a TODO. Added: a `node-toolchain` job that asserts the running node and npm majors match `engines`, that the lockfile is lockfileVersion 3, and that `npm ci` did not modify package.json or package-lock.json. Changed: `sensitive-changes-check` no longer carries a job-level `if: github.event_name == 'pull_request'`, so it now runs on push to main as well as on pull requests, and prints push-appropriate guidance. Its path filter gained `src/migrations/**` and `src/prisma/**`: the TypeORM migrations are the real schema and were not in the sensitive set at all. Changed: top-level `permissions: {}` (deny by default) with per-job opt-in. `security-events: write` is granted only to `security-scans`, the only job that uploads CodeQL results. Every job needs `contents: read` for actions/checkout, so no job qualifies for `permissions: {}`. Added: docs/CI_GATES.md recording the stable job ids and display names for branch protection, the honesty properties of the gate set, the action pin table with the `gh api` command for re-pinning, and the overlap with open issues #395, #443, #497, #498 and #392 so the maintainer reconciles rather than duplicates. Audited by reading the workflow files: no `continue-on-error`, no `|| true`, and no `exit 0` masking in ci.yml or container-smoke.yml. The one `if:` on a step is `if: always()` on the audit-evidence upload, which exists so evidence is retained when the audit fails; it does not suppress that failure. The `|| true` and `exit 0` in .github/workflows/v2-policy-advisory.yml are not masking: that workflow is explicitly advisory, is named "(advisory)", and ends by saying findings do not fail it. Both triggers (push to main and pull_request to main) were already present and are kept. Already in place before this change, kept as-is: `npm ci` in build-and-test, the eslint and Jest steps, the generated-artifact drift check, `npm audit --audit-level=high` itself, TruffleHog, CodeQL, the Trivy image scan, and the push/pull_request triggers. Residual risks and things a reviewer should look at first: 1. The migration gate is expected to FAIL on its first run. src/config/data-source.ts line 3 imports `SnakeNamingStrategy` from `typeorm-naming-strategies`, but that package is in neither package.json nor package-lock.json, so the data source cannot load. This is a pre-existing defect in the TypeORM migration path, not something this change introduces: the old Prisma-based step never touched that file, which is why it went unnoticed. It was not fixed here because fixing it needs either an install (regenerating the lockfile, which cannot be verified in this environment) or a behavioural change to column naming that the 17 committed migrations were written against. The one-line fix is `npm install --save typeorm-naming-strategies@^4` plus a review of the lockfile diff. A red gate reporting a real defect is the intended outcome of replacing a gate that tested dead code; a green gate here would be worse. 2. Direction conflict, unresolved. Open issue #392 ("V2-BE-041 - Complete Prisma-Only Persistence Convergence") mandates the opposite end state from issue #517. Both cannot hold. This change implements the #517 direction because the protocol schema, the entities, the indexer projections and the new CI gate are all TypeORM. The conflict and a recommended reconciliation are recorded in docs/PRISMA_INVENTORY.md; this commit does not close #392. 3. `DATABASE_URL` is overloaded between the two persistence layers with incompatible meanings. src/config/data-source.ts treats any value as PostgreSQL; src/prisma/prisma.service.ts passes it to PrismaLibSql, which only accepts libsql/SQLite. `.env.example` sets `file:./dev.db` (correct for Prisma, broken for TypeORM) and `.env.docker` sets a postgres URL (the reverse). Recorded in docs/PRISMA_INVENTORY.md; not fixed, because renaming the variable would touch eleven modules. 4. The container smoke workflow reports, rather than gates on, two things owned by #498: the builder stage's `COPY . .` still pulls tracked env templates such as .env.docker into an intermediate layer (not into the shipped image, which is asserted clean), and the runner stage declares no HEALTHCHECK and no non-root USER. Both surface as `::warning::` plus step-summary output so they are not mistaken for a clean bill of health. The documented liveness probe route, GET /health/live, is asserted to exist in the source. 5. Drift that may need re-tuning after a first run: the exact wording TypeORM prints for "no changes", the npm ci lockfile-drift error text matched in the container smoke workflow, and whether the most recent migration is actually reversible. Each is a hard failure with a diagnostic rather than a silent pass, so a mismatch is visible rather than hidden. No install, build, test, lint, typecheck or container build was run by the author of this commit, and no coverage figure or CI result is claimed anywhere in it. Nothing here has been verified by execution. The claims made are of the form "the workflow does X" and "the file contains Y", established by reading the repository; the assertions in the new gates about TypeORM output text, npm error text and migration reversibility are predictions that need a maintainer run to confirm. Closes #516 Closes #517 Closes #518 Closes #519
Contributor
|
@tomasbogado203-bit this PR currently has merge conflicts with |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Repository: DigiNodes/truthbounty-api/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (3)
✨ 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 |
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.
Summary
Resolves #395 (V2-BE-044).
Scope of Changes
Acceptance Criteria