diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cf15fbc3..08541afa 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -8,19 +8,23 @@ on: branches: - main -permissions: - contents: read +# Deny by default; each job below opts in to only the scopes it needs. +# Every job checks out the repository, so `contents: read` is the floor. +# `security-events: write` is granted only to the job that uploads CodeQL results. +permissions: {} jobs: build-and-test: name: Build, Lint, and Test + permissions: + contents: read runs-on: ubuntu-latest steps: - name: Checkout code - uses: actions/checkout@v7 + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Setup Node.js - uses: actions/setup-node@v7 + uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 with: node-version: '20' cache: 'npm' @@ -43,11 +47,143 @@ jobs: - name: Run unit and integration tests run: npm run test:cov - - name: Run migration tests + node-toolchain: + name: Node/npm Toolchain + permissions: + contents: read + runs-on: ubuntu-latest + steps: + - name: Checkout code + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + + - name: Setup Node.js + uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 + with: + node-version: '20' + cache: 'npm' + + # Proves the toolchain pinned in `engines` and documented in + # docs/DEPLOYMENT.md is the one CI actually runs, and that installs are + # deterministic (lockfile-only, no resolution at install time). + - name: Assert the runtime satisfies package.json engines run: | - # Simulates a fresh db migration and rollback - npx prisma migrate reset --force - npx prisma migrate deploy + set -euo pipefail + echo "node $(node --version) / npm $(npm --version)" + + node_major=$(node -p "process.versions.node.split('.')[0]") + npm_major=$(npm --version | cut -d. -f1) + engines_node=$(node -p "require('./package.json').engines.node") + engines_npm=$(node -p "require('./package.json').engines.npm") + engines_node_major=$(node -p "require('./package.json').engines.node.replace('>=','').split(' ')[0]") + engines_npm_major=$(node -p "require('./package.json').engines.npm.replace('>=','').split(' ')[0]") + lockfile_version=$(node -p "require('./package-lock.json').lockfileVersion") + + echo "engines.node=$engines_node engines.npm=$engines_npm lockfileVersion=$lockfile_version" + + if [[ "$node_major" != "$engines_node_major" ]]; then + echo "::error::Node $node_major is not the supported major version $engines_node_major (engines.node=$engines_node)." + exit 1 + fi + + if [[ "$npm_major" != "$engines_npm_major" ]]; then + echo "::error::npm $npm_major is not the supported major version $engines_npm_major (engines.npm=$engines_npm)." + exit 1 + fi + + if [[ "$lockfile_version" != "3" ]]; then + echo "::error::package-lock.json declares lockfileVersion $lockfile_version; the supported npm major writes lockfileVersion 3." + exit 1 + fi + + # `npm ci` is the only install command any gate uses. Re-resolving the + # tree here would defeat the point of committing a lockfile. + if ! git diff --exit-code -- package.json package-lock.json; then + echo "::error::package.json or package-lock.json changed during install." + git --no-pager diff -- package.json package-lock.json + exit 1 + fi + + echo "Toolchain and lockfile are consistent with the declared engines." + + schema-migration-gate: + name: Schema Migration and Drift Gate + permissions: + contents: read + runs-on: ubuntu-latest + services: + postgres: + image: postgres:15-alpine + env: + POSTGRES_USER: postgres + POSTGRES_PASSWORD: postgres + POSTGRES_DB: truthbounty_migration_gate + ports: + - '5432:5432' + options: >- + --health-cmd "pg_isready -U postgres -d truthbounty_migration_gate" + --health-interval 10s + --health-timeout 5s + --health-retries 10 + env: + # The gate targets PostgreSQL because that is the production driver in + # src/config/data-source.ts, whose Postgres branch hardcodes + # `synchronize: false`, so no NODE_ENV override is needed or wanted here. + # NODE_ENV=production in particular must be avoided at job level: it would + # make `npm ci` skip devDependencies, including the ts-node that the + # `migration:*` scripts require. + # Credentials are ephemeral CI service values, not secrets. + DATABASE_URL: postgresql://postgres:postgres@localhost:5432/truthbounty_migration_gate + steps: + - name: Checkout code + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + + - name: Setup Node.js + uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 + with: + node-version: '20' + cache: 'npm' + + - name: Install dependencies + run: npm ci + + # Replaces the previous `npx prisma migrate reset --force` / + # `npx prisma migrate deploy` step, which exercised the legacy Prisma + # migration set and never touched the TypeORM migrations the application + # actually runs. See docs/PRISMA_INVENTORY.md for the full reference list. + - name: Apply all TypeORM migrations to an empty database + run: npm run migration:run + + - name: Fail on schema drift between entities and committed migrations + run: | + set -uo pipefail + # `migration:generate` treats its argument as a path *prefix* and writes + # `-.ts`, so the new file is located with git rather + # than by guessing the name. + output=$(npm run --silent migration:generate -- "src/migrations/ci-drift-check" 2>&1) + status=$? + printf '%s\n' "$output" + + if [[ "$status" -eq 0 ]]; then + echo "::error::Schema drift detected. The committed migrations in src/migrations/ do not reproduce the TypeORM entities, so a fresh database would not match the code." + for file in $(git status --porcelain -- src/migrations | awk '{print $2}'); do + echo "::error::TypeORM generated an uncommitted migration, which is the missing delta: ${file}" + cat "$file" + done + exit 1 + fi + + if ! grep -qi "No changes in database schema were found" <<<"$output"; then + echo "::error::migration:generate exited ${status} for a reason other than 'no changes in database schema'. The drift check could not be evaluated; treat this as a failure, not a pass." + exit 1 + fi + + echo "No schema drift: committed migrations fully cover the TypeORM entities." + + - name: Prove the latest migration is reversible and re-appliable + run: | + set -euo pipefail + npm run migration:revert + npm run migration:run security-scans: name: Security Scans @@ -57,14 +193,37 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout code - uses: actions/checkout@v7 + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Setup Node.js - uses: actions/setup-node@v7 + uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 with: node-version: '20' cache: 'npm' + # Installed from the lockfile so the audit is evaluated against the exact + # tree that ships, and so this job installs deterministically like the rest. + - name: Install dependencies + run: npm ci + + # Canonical dependency scanner. `pipefail` keeps npm's non-zero exit on a + # high-or-critical finding, so this step still fails the workflow. The + # report is written to a file and uploaded below as retained evidence. + - name: Dependency audit (npm audit) + run: | + set -o pipefail + npm audit --audit-level=high 2>&1 | tee npm-audit-report.txt + + # `if: always()` only ensures the evidence is retained when the audit step + # fails; it does not suppress the failure. See docs/DEPENDENCY_SECURITY.md. + - name: Publish dependency audit evidence + if: always() + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 + with: + name: npm-audit-report + path: npm-audit-report.txt + if-no-files-found: error + retention-days: 90 - name: Install locked dependencies run: npm ci @@ -84,34 +243,36 @@ jobs: run: echo "Dependency audit is included in the SBOM gate above." - name: Secret scanning (TruffleHog) - uses: trufflesecurity/trufflehog@main + uses: trufflesecurity/trufflehog@4dd8831c5f12599465d4d45c3c447b4018a34c85 # v3.97.9 with: path: ./ base: ${{ github.event.repository.default_branch }} head: HEAD - name: Initialize CodeQL - uses: github/codeql-action/init@v4 + uses: github/codeql-action/init@2892aa5e19bbd11bc0cff5427e3b750a04d9e3c2 # v4.38.2 with: languages: javascript, typescript - name: Perform CodeQL Analysis - uses: github/codeql-action/analyze@v4 + uses: github/codeql-action/analyze@2892aa5e19bbd11bc0cff5427e3b750a04d9e3c2 # v4.38.2 with: category: "/language:javascript-typescript" container-scan: name: Container Vulnerability Scan + permissions: + contents: read runs-on: ubuntu-latest steps: - name: Checkout code - uses: actions/checkout@v7 + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Build Docker image run: docker build -t truthbounty-api:test . - name: Run Trivy vulnerability scanner - uses: aquasecurity/trivy-action@master + uses: aquasecurity/trivy-action@ed142fd0673e97e23eac54620cfb913e5ce36c25 # v0.36.0 with: image-ref: 'truthbounty-api:test' format: 'table' @@ -122,25 +283,41 @@ jobs: sensitive-changes-check: name: Sensitive Changes Protection + permissions: + contents: read runs-on: ubuntu-latest - if: github.event_name == 'pull_request' steps: - name: Check for sensitive changes - uses: dorny/paths-filter@v4 + uses: dorny/paths-filter@ceb8a2b8f2d89434be7ff52d3de7ec3738c5cc9d # v4.0.3 id: filter with: filters: | sensitive: - 'src/auth/**' - 'src/indexer/**' + - 'src/migrations/**' + - 'src/prisma/**' - 'prisma/**' - 'src/database/**' - '.github/workflows/**' - - name: Prohibit automatic merge - if: steps.filter.outputs.sensitive == 'true' + # Reports on both pull_request and push to main. The job is advisory by + # design and cannot fail; enforcement for the real gates lives in branch + # protection over the job names recorded in docs/CI_GATES.md. + - name: Report sensitive changes + env: + SENSITIVE: ${{ steps.filter.outputs.sensitive }} + EVENT_NAME: ${{ github.event_name }} run: | - echo "Sensitive changes detected in auth, indexer, or database." - echo "Automatic merge is prohibited. Ensure human review is completed." - # Remove auto-merge label if present (pseudo-command for demonstration) - # gh pr edit ${{ github.event.pull_request.number }} --remove-label "auto-merge" + set -euo pipefail + echo "sensitive=${SENSITIVE} event=${EVENT_NAME}" + if [[ "${SENSITIVE}" != "true" ]]; then + echo "No sensitive paths changed." + else + echo "Sensitive paths changed: auth, indexer, TypeORM migrations, Prisma, database, or CI workflows." + if [[ "${EVENT_NAME}" == "pull_request" ]]; then + echo "Automatic merge is prohibited. Ensure human review is completed." + else + echo "Landed on ${GITHUB_REF}. Confirm the required human review record exists." + fi + fi diff --git a/.github/workflows/container-smoke.yml b/.github/workflows/container-smoke.yml new file mode 100644 index 00000000..d66e2221 --- /dev/null +++ b/.github/workflows/container-smoke.yml @@ -0,0 +1,189 @@ +name: Container Smoke Build + +# Companion to the `Container Vulnerability Scan` job in ci.yml. That job asks +# "is the shipped image vulnerable?". This workflow asks the reproducibility +# questions instead: does the image build from a clean checkout with no local +# cache, does the build actually consume the committed lockfile, and does the +# resulting image carry what it needs and nothing it must not. +# +# The two workflows deliberately build separately. Keeping them apart means a +# scan result is never attributed to an image this workflow did not build, and +# vice versa. The cost is one extra image build per run. +# +# Runner-stage hardening (non-root USER, HEALTHCHECK, .dockerignore) is owned by +# issue #498 and is intentionally NOT asserted as a pass/fail gate here. + +on: + push: + branches: + - main + pull_request: + branches: + - main + +permissions: + contents: read + +jobs: + container-smoke: + name: Container Smoke Build + permissions: + contents: read + runs-on: ubuntu-latest + steps: + - name: Checkout code + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + + # Build from a pristine export of the committed tree, with the base image + # re-pulled and the layer cache disabled. This is the "clean checkout, no + # local cache" requirement: no node_modules, no previous build output and + # no uncommitted local file can influence the result. + - name: Build the image from a clean checkout + run: | + set -euo pipefail + context="$(mktemp -d)" + git archive --format=tar HEAD | tar -x -C "$context" + echo "context=$context" + ls -1 "$context" + + docker build \ + --pull \ + --no-cache \ + --tag truthbounty-api:smoke \ + --file "$context/Dockerfile" \ + "$context" + + - name: Assert the builder stage installs from the committed lockfile + run: | + set -euo pipefail + context="$(mktemp -d)" + git archive --format=tar HEAD | tar -x -C "$context" + + # Introduce drift in package.json only, leaving package-lock.json + # untouched. A builder stage that honours the lockfile must refuse to + # install; a builder stage that re-resolves (`npm install`) will happily + # build, which is exactly the drift this gate exists to catch. + node -e ' + const fs = require("fs"); + const path = process.argv[1]; + const pkg = JSON.parse(fs.readFileSync(path, "utf8")); + pkg.dependencies["socket.io"] = "4.8.0"; + fs.writeFileSync(path, JSON.stringify(pkg, null, 2) + "\n"); + ' "$context/package.json" + + output_file="$(mktemp)" + if docker build --pull --no-cache --file "$context/Dockerfile" "$context" >"$output_file" 2>&1; then + echo "::error::The container build succeeded even though package.json and package-lock.json disagree. The builder stage is not consuming the committed lockfile." + exit 1 + fi + + cat "$output_file" + if ! grep -qiE 'out of sync|not in sync|does not satisfy|Invalid: lock file' "$output_file"; then + echo "::error::The build failed, but not for a lockfile-drift reason. This step cannot distinguish drift detection from an unrelated build failure; treat it as an unresolved failure rather than a pass." + exit 1 + fi + + echo "Dependency drift is rejected by the container build, as intended." + + - name: Assert the runtime image carries the expected artifacts + run: | + set -euo pipefail + if ! output=$(docker run --rm --entrypoint sh truthbounty-api:smoke -c ' + for path in /app/dist/main.js /app/node_modules /app/package.json /app/package-lock.json /app/src/generated; do + [ -e "$path" ] || { echo "missing expected runtime artifact: $path"; exit 1; } + done + echo "all expected runtime artifacts present" + ' 2>&1); then + echo "::error::The shipped image is missing at least one expected runtime artifact." + printf '%s\n' "$output" + exit 1 + fi + printf '%s\n' "$output" + + - name: Assert dev-only dependencies and credentials are absent from the shipped image + run: | + set -euo pipefail + # `npm prune --production` in the builder stage must have removed + # devDependencies, including the Prisma CLI. + if ! output=$(docker run --rm --entrypoint sh truthbounty-api:smoke -c ' + for pkg in typescript @nestjs/cli jest prisma; do + [ ! -e "/app/node_modules/$pkg" ] || { echo "dev-only dependency present in the production image: $pkg"; exit 1; } + done + for pkg in typeorm @nestjs/core @prisma/client; do + [ -e "/app/node_modules/$pkg" ] || { echo "runtime dependency missing from the production image: $pkg"; exit 1; } + done + + # .dockerignore keeps .env out of the build context, and the runner + # stage copies only package*.json, node_modules, dist and + # src/generated, so no environment file can reach the shipped image. + found=$(find /app -maxdepth 1 -name ".env*" -print) + [ -z "$found" ] || { echo "environment file baked into the shipped image: $found"; exit 1; } + + echo "no dev-only dependencies or environment files in the shipped image" + ' 2>&1); then + echo "::error::The shipped image carries dev-only dependencies or environment files." + printf '%s\n' "$output" + exit 1 + fi + printf '%s\n' "$output" + + # Known gap, reported rather than failed: `COPY . .` in the builder + # stage still pulls tracked env templates such as .env.docker into an + # intermediate layer. They are not in the shipped image, but they are + # in the builder's layer history. Fixing it means editing .dockerignore + # or the builder COPY, both of which issue #498 owns. Reported here so + # it is not mistaken for a clean bill of health. + docker build --target builder --tag truthbounty-api:builder-smoke . + builder_env=$(docker run --rm --entrypoint sh truthbounty-api:builder-smoke -c \ + 'ls -1a /app | grep -E "^\.env" | paste -sd " " -') + + if [ -n "$builder_env" ]; then + echo "::warning::The builder stage context contains ${builder_env}. These are not in the shipped image, but they are in the builder layer history. Removing them is owned by issue #498." + else + echo "The builder stage context contains no .env* files." + fi + + { + echo "### Builder-stage environment templates" + echo + echo "\`${builder_env:-none}\`" + } >> "$GITHUB_STEP_SUMMARY" + + - name: Report the image's declared health check + run: | + set -euo pipefail + declared=$(docker inspect --format '{{json .Config.Healthcheck}}' truthbounty-api:smoke) + cmd=$(docker inspect --format '{{json .Config.Cmd}}' truthbounty-api:smoke) + user=$(docker inspect --format '{{json .Config.User}}' truthbounty-api:smoke) + + echo "Healthcheck: $declared" + echo "Cmd: $cmd" + echo "User: $user" + + { + echo "### Container runtime declaration" + echo + echo "| Field | Value |" + echo "| --- | --- |" + echo "| \`Healthcheck\` | \`$declared\` |" + echo "| \`Cmd\` | \`$cmd\` |" + echo "| \`User\` | \`$user\` |" + } >> "$GITHUB_STEP_SUMMARY" + + if [ "$declared" = "null" ]; then + echo "::warning::The runner stage declares no HEALTHCHECK. Adding one (and the non-root USER) is owned by issue #498, so this workflow reports the gap instead of gating on it." + fi + + # The liveness route the eventual HEALTHCHECK should target must exist + # in the source. GET /health/live is served by HealthController, which + # is @Public() and requires no dependency, so it is the correct probe. + if ! grep -q "@Get('live')" src/health/health.controller.ts; then + echo "::error::The documented liveness probe route is missing from src/health/health.controller.ts; a container HEALTHCHECK would have no target." + exit 1 + fi + if ! grep -q "@Controller('health')" src/health/health.controller.ts; then + echo "::error::The documented liveness probe is not mounted at /health." + exit 1 + fi + + echo "Documented liveness probe GET /health/live is present in the source." diff --git a/.gitignore b/.gitignore index f2354a39..aa4a1016 100644 --- a/.gitignore +++ b/.gitignore @@ -65,4 +65,3 @@ pids report.[0-9]*.[0-9]*.[0-9]*.[0-9]*.json .qodo -/generated/prisma diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2be58697..de962764 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -9,10 +9,34 @@ Thank you for your interest in contributing! This guide will help you get starte This project is a backend API built with: - **NestJS (TypeScript)** -- **Prisma ORM** -- **SQLite/PostgreSQL** +- **TypeORM** (the persistence layer for the application schema) +- **PostgreSQL** (production) with a **SQLite** fallback for local development - **Jest for testing** +### Persistence and migrations + +TypeORM is the persistence layer for the application schema. Entities live alongside their +feature modules, and migrations live in `src/migrations/`. The data source is +`src/config/data-source.ts`, which selects PostgreSQL when `DATABASE_URL` is set and falls +back to SQLite otherwise. + +```bash +npm run migration:run # Apply pending migrations +npm run migration:revert # Roll back the most recent migration +npm run migration:generate # Generate a migration after changing an entity +``` + +Two things to know before you touch the schema: + +1. **CI enforces the migrations.** The `Schema Migration and Drift Gate` job in `ci.yml` + applies every migration to an empty PostgreSQL database, then reverts and re-applies the + latest one, then fails if the entities do not exactly match the migrated schema. Generate + a migration whenever you change an entity; an unmigrated entity change fails the gate. +2. **A legacy Prisma layer still exists** and is *not* the application schema. It is retained + for a set of identity, analytics, outbox and AI-assistant features. Do not run `prisma + migrate` against it, and do not assume its tables describe the protocol schema. Read + `docs/PRISMA_INVENTORY.md` before adding anything that touches it. + --- ## ⚙️ Setup Instructions @@ -22,3 +46,10 @@ This project is a backend API built with: ```bash git clone https://github.com/DigiNodes/truthbounty-api.git cd truthbounty-api + +# Use Node 20 LTS with npm 10 (see "Supported Runtime and Toolchain" in docs/DEPLOYMENT.md) +nvm use 20 + +# Install exactly what package-lock.json records +npm ci +``` diff --git a/Dockerfile b/Dockerfile index 05661faf..5ceed087 100644 --- a/Dockerfile +++ b/Dockerfile @@ -4,7 +4,10 @@ FROM node:20-alpine AS builder # Set working directory WORKDIR /app -# Install dependencies +# Install dependencies from the committed lockfile. +# `npm ci` makes package-lock.json authoritative: it installs exactly the +# resolved tree and fails the build if package.json and the lockfile disagree. +# `npm install` must not be used here or dependency drift goes undetected. COPY package*.json ./ RUN npm ci diff --git a/docs/CI_GATES.md b/docs/CI_GATES.md new file mode 100644 index 00000000..b9226093 --- /dev/null +++ b/docs/CI_GATES.md @@ -0,0 +1,112 @@ +# CI Gates + +Reference for the required CI gate set, for branch protection configuration. Written for +STAB-BE-005 (issue #519, "Restore the API Required CI Gate Set"). + +## Stable job names + +Branch protection must key on these. The **job id** in the left column is the YAML key and +is what a branch-protection rule matches; the **display name** in the right column is what a +reviewer sees. Both are stable. Renaming either is a breaking change for branch protection +and must update this table in the same pull request. + +| Job id (YAML key) | Display name | Workflow | Blocking? | What it proves | +| --- | --- | --- | --- | --- | +| `build-and-test` | Build, Lint, and Test | `ci.yml` | **Yes** | `npm ci` succeeds against the lockfile, `npm run build` produces no uncommitted artifact drift, ESLint is clean, and the Jest suite passes with coverage. | +| `node-toolchain` | Node/npm Toolchain | `ci.yml` | **Yes** | The runner's `node` and `npm` major versions match `engines`, the lockfile is `lockfileVersion: 3`, and `npm ci` did not modify `package.json` or `package-lock.json`. | +| `schema-migration-gate` | Schema Migration and Drift Gate | `ci.yml` | **Yes** | All TypeORM migrations in `src/migrations/` apply to an empty PostgreSQL database, the latest one is reversible and re-appliable, and the entities do not drift from the migrated schema. | +| `security-scans` | Security Scans | `ci.yml` | **Yes** | `npm audit --audit-level=high`, TruffleHog secret scanning, and CodeQL for JavaScript/TypeScript. Uploads the audit output as the `npm-audit-report` artifact. | +| `container-scan` | Container Vulnerability Scan | `ci.yml` | **Yes** | The image builds, and Trivy reports no unfixed `CRITICAL` or `HIGH` OS or library finding. | +| `container-smoke` | Container Smoke Build | `container-smoke.yml` | **Yes** | The image builds from a clean checkout with no local cache, dependency drift is rejected, the shipped image carries its runtime artifacts and no dev-only dependencies or environment files, and the documented liveness probe exists in the source. | +| `sensitive-changes-check` | Sensitive Changes Protection | `ci.yml` | No (advisory) | Reports whether auth, indexer, TypeORM migration, Prisma, database or CI-workflow paths changed. | + +Recommended branch protection for `main`: require all six blocking jobs above, require them on +`pull_request` to `main` **and** on `push` to `main`, and disallow bypass except for +repository admins with an explicit dismissal reason. + +## Triggers + +Every workflow above runs on both events: + +```yaml +on: + push: + branches: [main] + pull_request: + branches: [main] +``` + +This was already the case for `ci.yml` and is kept. `sensitive-changes-check` previously ran +only on `pull_request`; it now runs on both so that no job is silently absent from a push to +`main`, and it prints push-appropriate guidance instead of PR-only wording. + +## Honesty properties of these gates + +Verified by reading `.github/workflows/ci.yml` and `.github/workflows/container-smoke.yml` as +written. Not verified by executing them. + +| Property | Status | +| --- | --- | +| `continue-on-error` | Not present in either workflow. | +| `\|\| true` | Not present in either workflow. | +| `exit 0` used to mask a failure | Not present in either workflow. `sensitive-changes-check` uses if/else branching rather than an early `exit 0`. | +| Unconditional success | No step ends in unconditional success. The only `if:` on a step is `if: always()` on the audit-evidence upload, which exists so the evidence is retained *when the audit step fails*. It does not suppress that failure: `npm audit` runs with `set -o pipefail` and still fails the job. | +| Conditional skip of a whole job | None. `sensitive-changes-check` used to carry `if: github.event_name == 'pull_request'`; that job-level condition is removed. | +| `permissions:` least privilege | Top level is `permissions: {}` (deny by default). Each job opts in. `security-events: write` is granted only to `security-scans`, which is the only job that uploads CodeQL results. Every other job receives `contents: read` and nothing more, which is the floor required by `actions/checkout`. | +| Deterministic installs | Every job that needs dependencies runs `npm ci`. The container builder stage runs `npm ci`. No job runs `npm install`. | +| Action pinning | Every `uses:` is pinned to a 40-character commit SHA with a `# vX.Y.Z` comment. See below. | + +## Action pins + +All action references are pinned to immutable commit SHAs. The SHAs below were resolved from +the GitHub API against each repository's release tags, not from memory. + +| Action | Reference in workflow | SHA | Version comment | +| --- | --- | --- | --- | +| `actions/checkout` | `3d3c42e5aac5ba805825da76410c181273ba90b1` | tag `v7.0.1` (was `v7`) | `# v7.0.1` | +| `actions/setup-node` | `820762786026740c76f36085b0efc47a31fe5020` | tag `v7.0.0` (was `v7`) | `# v7.0.0` | +| `actions/upload-artifact` | `ea165f8d65b6e75b540449e92b4886f43607fa02` | tag `v4.6.2` | `# v4.6.2` | +| `github/codeql-action` | `2892aa5e19bbd11bc0cff5427e3b750a04d9e3c2` | tag `v4.38.2` (was `v4`) | `# v4.38.2` | +| `dorny/paths-filter` | `ceb8a2b8f2d89434be7ff52d3de7ec3738c5cc9d` | tag `v4.0.3` (was `v4`) | `# v4.0.3` | +| `trufflesecurity/trufflehog` | `4dd8831c5f12599465d4d45c3c447b4018a34c85` | tag `v3.97.9` (was `main`, a branch) | `# v3.97.9` | +| `aquasecurity/trivy-action` | `ed142fd0673e97e23eac54620cfb913e5ce36c25` | tag `v0.36.0` (was `master`, a branch) | `# v0.36.0` | + +No action reference was left as a TODO. Every reference, including the two that were +previously pinned to mutable branches (`trufflehog@main`, `trivy-action@master`), was +resolved to a real release tag. A branch ref is a supply-chain risk: it is mutable, so +upstream can change the code a workflow runs without any change to this repository. + +To re-pin after a Dependabot bump, or to pin something added later, resolve the SHA for real +rather than copying one from this table: + +```bash +gh api repos/OWNER/REPO/git/ref/tags/TAG --jq '.object.sha' +# annotated tags resolve to a tag object; dereference once more: +gh api repos/OWNER/REPO/git/tags/SHA --jq '.object.sha' +``` + +## Relationship to other open work + +This change overlaps existing issues and pull requests. It does not supersede them, and the +maintainer should reconcile rather than merge both. + +| Reference | Overlap | Status here | +| --- | --- | --- | +| Issue **#395** | Also addresses the required CI gate set. | This change implements the gate set and publishes the job-name table. If #395 has in-flight work on the same jobs, prefer one implementation; do not merge both without reconciling. | +| PR **#443** | Touches the same gate set. | Not reviewed. The job names in this table are the ones a branch-protection rule should use; if #443 renames a job, update this table in that pull request. | +| Issue **#497** (memplethee-lab) | Owns `.github/codeql/config.yml` and a scheduled CodeQL workflow. | Deliberately not created or edited here, to keep the two pull requests mergeable. | +| Issue **#498** (memplethee-lab) | Owns the Dockerfile runner stage (`USER`, `HEALTHCHECK`) and `.dockerignore`. | Deliberately not touched. `container-smoke.yml` reports the current `Healthcheck`/`User` values in the step summary and emits a `::warning::` rather than gating on work this repository does not own. | +| Issue **#392** | Mandates Prisma-only persistence convergence, the opposite of issue #517. | Unresolved direction. See `PRISMA_INVENTORY.md`. | +| Issue **#518**, **#516**, **#517** | Landed in the same pull request as this one. | `DEPENDENCY_SECURITY.md`, `DEPLOYMENT.md` and `PRISMA_INVENTORY.md` respectively. | + +## Known failure expected on first run + +`schema-migration-gate` is expected to fail on its first run. +`src/config/data-source.ts` imports `typeorm-naming-strategies`, which is not declared in +`package.json` and not present in `package-lock.json`, so the data source cannot load and +`npm run migration:run` cannot run. See "Known blocker in this gate" in `PRISMA_INVENTORY.md` +for the one-line fix. + +This is the intended outcome of replacing a gate that tested dead code with one that tests +the real schema path. The previous step exercised Prisma and could not have surfaced the +defect. diff --git a/docs/DEPENDENCY_SECURITY.md b/docs/DEPENDENCY_SECURITY.md new file mode 100644 index 00000000..88387f0b --- /dev/null +++ b/docs/DEPENDENCY_SECURITY.md @@ -0,0 +1,207 @@ +# Dependency Security + +Working document for STAB-BE-004 (issue #518, "Remediate API Dependency Vulnerabilities in +Reviewable Batches"). + +> [!IMPORTANT] +> **This is a scaffold with a defined procedure, not a completed audit.** No vulnerability +> scanner was run to produce it. Every finding row below is empty on purpose. Populate it +> from the `npm audit` artifact that CI now publishes, then work the batches below. + +## Why this document exists + +The acceptance criteria for #518 require that no critical vulnerability remains without an +explicit maintainer-approved risk record, that high findings are fixed or documented with +package path, exploitability, owner and follow-up, and that `npm audit` runs in CI and +publishes evidence. Two of those three are process, not code: the table and the record +format have to exist and be used consistently. This document is that process. + +## Method, and its limits + +**What was actually done to build this document.** The declared version ranges in +`package.json` and the resolved versions in `package-lock.json` were read directly. That is +the whole evidence base. Nothing was installed, no scanner was run, and no advisory database +was queried. + +**What that means for the table below.** The table cannot contain severities, advisory +identifiers, CVSS scores or fix versions, because producing any of those would require +querying an advisory database. Writing them from memory would be fabrication, so they are +left blank. The columns that *can* be filled honestly from the repository - package path, +runtime reachability, owner, follow-up - are the ones the acceptance criteria ask for, and +the rows are seeded from the dependency inventory below as a starting structure. + +**What is required to complete it.** Run the scanner and paste the result in. The +`Security Scans` job in `.github/workflows/ci.yml` runs: + +```bash +npm audit --audit-level=high +``` + +with `set -o pipefail`, so a high or critical finding still fails the workflow. The same +output is written to `npm-audit-report.txt` and uploaded as the `npm-audit-report` workflow +artifact with a 90-day retention, so the evidence for a given commit is retrievable without +re-running anything. Download that artifact for the commit under review, and transcribe each +advisory into the table. + +## What is already in place + +These were present before this change and were deliberately kept: + +| Control | Where | Notes | +| --- | --- | --- | +| `npm audit --audit-level=high` | `ci.yml`, `Security Scans` job | The canonical dependency scanner. Already failing the build on a high finding. | +| TruffleHog secret scanning | `ci.yml`, `Security Scans` job | `trufflesecurity/trufflehog`, pinned to a commit SHA. | +| CodeQL for JavaScript/TypeScript | `ci.yml`, `Security Scans` job | The only job with `security-events: write`. | +| Trivy OS and library scan of the built image | `ci.yml`, `Container Vulnerability Scan` job | `exit-code: '1'`, `severity: 'CRITICAL,HIGH'`, `ignore-unfixed: true`. | +| Weekly npm Dependabot updates | `.github/dependabot.yml` | Grouped into production and development dependency PRs, max 5 open. | +| `npm ci` everywhere | `ci.yml`, `Dockerfile` builder stage | Installs the exact lockfile tree; drift fails the build. | + +Added by this change: the `npm audit` output is now retained as an artifact, and +`security-scans` runs `npm ci` so the audit is evaluated against the tree that actually ships +rather than against a lockfile with nothing installed next to it. + +## Declared versus resolved versions + +Read from `package.json` and `package-lock.json`. The `lockfileVersion` is 3 and the supported +npm major is 10 (see the toolchain section of `DEPLOYMENT.md`). + +| Package | Declared range | Resolved | Note | +| --- | --- | --- | --- | +| `@anthropic-ai/sdk` | `^0.115.0` | see lockfile | | +| `@bull-board/api`, `@bull-board/express`, `@bull-board/nestjs` | `^7.1.5` | see lockfile | | +| `@libsql/client` | `^0.17.0` | `0.17.0` | Pinned to an exact resolved version in the lockfile. | +| `@nestjs/*` (common, config, core, jwt, mapped-types, passport, platform-express, schedule, swagger, throttler, typeorm) | `^4.x` - `^11.x` | see lockfile | `@nestjs/mapped-types` is declared as `*`, which is unpinned in `package.json`; the lockfile is the only thing constraining it. | +| `@prisma/adapter-libsql`, `@prisma/client` | `^7.3.0` | `7.4.1` | Retained with reasons in `PRISMA_INVENTORY.md`. | +| `axios` | `^1.18.1` | see lockfile | | +| `bullmq` | `^5.77.6` | see lockfile | | +| `ethers` | `^6.16.0` | see lockfile | | +| `ioredis` | `^5.9.3` | see lockfile | | +| `jsonwebtoken` | `^9.0.3` | see lockfile | | +| `openai` | `^7.1.0` | see lockfile | | +| `passport`, `passport-jwt` | `^0.7.0`, `^4.0.1` | see lockfile | | +| `pg` | `^8.17.2` | see lockfile | Production database driver. | +| `pino`, `pino-http`, `pino-pretty` | `^9.4.0`, `^10.3.0`, `^11.2.0` | see lockfile | | +| `prom-client` | `^15.1.3` | see lockfile | | +| `socket.io` | `4.8.1` (exact) | see lockfile | One of the few exactly-pinned runtime dependencies. | +| `sqlite3` | `^5.1.7` | see lockfile | | +| `typeorm` | `^0.3.28` | see lockfile | Persistence layer. | +| `web3` | `^4.16.0` | see lockfile | Large transitive surface. | + +Development-only: `@nestjs/cli`, `@nestjs/schematics`, `@nestjs/testing`, `prisma`, `jest`, +`ts-jest`, `@swc/jest`, `ts-node`, `ts-loader`, `typescript`, `eslint`, `prettier`, +`supertest`, `tsconfig-paths`, and the `@types/*` set. `npm prune --production` removes all of +these from the shipped image, and the container smoke workflow asserts that +`typescript`, `@nestjs/cli`, `jest` and `prisma` are absent from it. + +Note: the "see lockfile" entries are deliberate. Transcribing the full transitive resolution +graph here would be a large, immediately stale document; the lockfile is the record, and the +`npm audit` artifact is the evidence. + +## Finding table + +Populate one row per advisory from the `npm audit` artifact. **Do not leave a row blank and +do not guess a severity** - if the scanner did not report it, it is not a finding; if it did, +copy the severity verbatim. + +| # | Package path | Advisory | Severity | Exploitability / runtime reachability | Owner | Follow-up | Status | +| --- | --- | --- | --- | --- | --- | --- | --- | +| 1 | _(unpopulated)_ | | | | | | | +| 2 | _(unpopulated)_ | | | | | | | + +Column notes: + +- **Package path** - the `node_modules/...` path from the scanner output, so the row is + unambiguous when a package appears more than once in the tree. +- **Severity** - verbatim from the scanner (`critical`, `high`, `moderate`, `low`). Do not + upgrade or downgrade a severity to make a batch easier to close. +- **Exploitability / runtime reachability** - the judgement a scanner cannot make. Classify + each finding as one of: + - `reachable` - the vulnerable code path can be invoked through an HTTP route, a queue + consumer, or an indexer event handler. + - `dev-only` - only present in `devDependencies`, and confirmed absent from the shipped + image by the container smoke workflow. Cannot be reached in production, but still worth + fixing because it can compromise a build. + - `transitive-unreachable` - reachable only through a dependency path the application does + not use. Record *why* it is unreachable, not just that it is. + - `build-time` - only affects the build. +- **Owner** - the CODEOWNERS entry for the affected path, or the CODEOWNERS entry for + `.github/workflows/` if the finding is a CI action pin. See `.github/CODEOWNERS`. +- **Follow-up** - the batch id from "Batching rule" below, or a link to a risk-acceptance + record. +- **Status** - `open`, `fixed`, `risk-accepted`, or `not-reachable` (with the justification + in the exploitability column). + +## Batching rule + +Remediation lands in **reviewable batches**, defined as follows. + +1. **One logical remediation per batch.** A batch fixes one advisory, one advisory class + across a coherent dependency set (for example "all `tar` advisories in the lockfile"), or + one upstream major-version migration. +2. **No unrelated major migrations bundled.** A major-version bump of an unrelated package + never rides along with a patch. If two majors are both needed, they are two batches, and + the second is stacked on the first rather than merged with it. +3. **One lockfile per batch.** Each batch changes `package.json` and `package-lock.json` and + nothing else, so the diff is reviewable as a dependency change. +4. **Batches are ordered by runtime reachability**, not by severity alone. A `reachable` + `moderate` outranks an unreachable `critical`, because reachability is the part that + actually decides whether the package is exploitable. +5. **Each batch states its verification.** A batch is not done until the maintainer has run + the build and the test suite locally, or the batch explicitly carries that as outstanding. + Nothing in this repository should be merged with an unverified lockfile change; an + unverifiable lockfile edit is the failure mode this issue exists to prevent. + +Suggested initial batch order, to be confirmed against the audit output: + +| Batch | Scope | +| --- | --- | +| B1 | Every `reachable` finding, one advisory per batch | +| B2 | Every `dev-only` finding, one advisory per batch | +| B3 | Runtime dependency major upgrades, one package per batch | +| B4 | Development dependency major upgrades, one package per batch | +| B5 | Residual `transitive-unreachable` and `build-time` findings, closed by reachability rationale or a risk-acceptance record | + +## Risk-acceptance record format + +A **critical** finding may only be closed without a fix by a maintainer filing this record. +No other closure is acceptable, and the record must be in the pull request that closes the +finding, not only in this file. + +``` +## Risk acceptance: in + +- **Finding:** +- **Version:** -> +- **Reachability:** + +- **Impact if hit:** +- **Compensating controls:** +- **Why not fixed now:** +- **Owner:** +- **Approved by:** +- **Approved on:** +- **Expires / re-review on:** +- **Issue to track the fix:** +``` + +Rules for the record: + +- **Every critical finding needs one**, even if the reachability assessment is "not + reachable". The assessment is the value; the approval is the accountability. +- **An expiry date is mandatory.** A record without a re-review date is a permanent silent + waiver, which is what the acceptance criteria are designed to prevent. Choose a date at most + 90 days out. +- **The approver is a maintainer**, not the author of the batch that surfaced the finding. +- **Critical findings with `reachable` classification are not waivable.** Fix them. The + record is for critical findings that are genuinely not exploitable in this deployment. + +## Residual risk in this change + +`package.json` dependency ranges and `package-lock.json` are **unmodified** by this change. +No advisory data was gathered, so no remediation could be justified, and a lockfile edit +that cannot be regenerated and verified locally is precisely the unverifiable change this +issue warns about. `package.json` received one non-dependency edit: `engines.npm` was added +to pin the supported npm major alongside the existing `engines.node`. That field is metadata, +is not resolved as a package, and does not affect the dependency graph. diff --git a/docs/DEPLOYMENT.md b/docs/DEPLOYMENT.md index b014f43e..1a7062e5 100644 --- a/docs/DEPLOYMENT.md +++ b/docs/DEPLOYMENT.md @@ -8,6 +8,33 @@ This document covers the configuration, artifact validation, and deployment oper > The backend acts as a high-performance indexer and interface, but smart contracts remain the ultimate authority. > Do NOT use or commit production secrets in deployment templates or examples. +## Supported Runtime and Toolchain + +The API is built and deployed on exactly one Node major version. The declaration lives in +`package.json` under `engines`, and the `Node/npm Toolchain` CI job asserts that the runner +matches it, so drift between the declaration and the pipeline fails the build. + +| Component | Supported range | Source of truth | +| --- | --- | --- | +| Node.js | `>=20 <21` (Node 20 LTS) | `package.json` → `engines.node` | +| npm | `>=10 <11` (npm 10) | `package.json` → `engines.npm` | +| Lockfile format | `lockfileVersion: 3` | `package-lock.json` | + +npm 10 is the version bundled with Node 20 and is the version that writes `lockfileVersion: 3`. +A different npm major must not be used to install: it can rewrite the lockfile, which turns a +reproducible install into a silent dependency change. + +```bash +# Confirm the local toolchain matches the supported ranges before doing anything else +node --version # expect v20.x +npm --version # expect 10.x +``` + +Install dependencies with `npm ci` only. `npm ci` installs the exact tree recorded in +`package-lock.json` and fails when `package.json` and the lockfile disagree, which is what +makes container builds and CI runs reproducible. Use `npm install` only when intentionally +changing dependencies, and commit the resulting lockfile in the same commit. + ## Pre-Deployment Setup ### Configuration @@ -39,11 +66,22 @@ The CI workflow uploads the CycloneDX SBOM as `dependency-sbom-`. Tr ### 2. Database Migrations Always run database migrations before spinning up the application to ensure schema consistency. Note that the DB is non-authoritative compared to the chain, but must be in sync with the ORM. +TypeORM is the persistence layer for the application schema. Migrations live in `src/migrations/` +and are driven by the data source at `src/config/data-source.ts`, which selects PostgreSQL +when `DATABASE_URL` is set and falls back to SQLite when it is not. + ```bash # Run pending migrations -npx prisma migrate deploy +npm run migration:run + +# Roll back the most recent migration, for example when a bad release shipped one +npm run migration:revert ``` +Do not use `npx prisma migrate deploy` for the application schema. It operates on +`prisma/schema.prisma`, which describes a separate, legacy data set; see +`PRISMA_INVENTORY.md` for the full picture of the two persistence layers. + ### 3. Application Startup Start the application using Docker Compose or your preferred orchestrator (e.g., Kubernetes). diff --git a/docs/PRISMA_INVENTORY.md b/docs/PRISMA_INVENTORY.md new file mode 100644 index 00000000..dc12f131 --- /dev/null +++ b/docs/PRISMA_INVENTORY.md @@ -0,0 +1,264 @@ +# Prisma Inventory + +Search report for every Prisma reference in this repository, produced for STAB-BE-003 +(issue #517, "Remove Prisma Remnants and Prove the TypeORM-Only Baseline"). + +## Summary + +Prisma is **not** inert in this repository. It is a live, load-bearing second persistence +layer used by 7 feature modules. A full removal would break the application, so this +document records what remains, why it remains, and what must happen before it can be removed. + +| Claim | Status | +| --- | --- | +| Prisma is fully removable today | **No.** Blocked; see "Blocker" below. | +| CI exercised dead Prisma code instead of the real schema | **Yes, and now fixed.** See "CI change". | +| Application schema is TypeORM | **Yes.** 17 TypeORM migrations in `src/migrations/`. | +| A repository search report exists | This document. | + +## Method and limits + +The inventory below was produced by reading the repository: a case-insensitive search for +`prisma` across every tracked file, followed by reading each hit to classify it. Findings were +not confirmed by running the code, building the image, or executing the test suite, so every +"retained" classification is a statement about the source as written, not about observed +runtime behaviour. + +Excluded from the search: `package-lock.json` (a generated resolution graph, summarised in +"Declared packages" below) and `src/generated/client/**` (a generated client tree, summarised +separately). Both are described rather than enumerated line by line. + +## Blocker: what stops full removal + +`src/prisma/prisma.service.ts` is imported by **11 production service files across 7 +feature modules**, plus 7 module-wiring files and `src/app.module.ts`. Counted +mechanically with +`grep -rlE "from '.*prisma/prisma\.(service|module)'|@prisma/client" src --include='*.ts'`, +excluding the 20 generated files under `src/generated/client/**` and the 6 `.spec.ts` +files listed separately below. + +| Module | File | What it reads or writes through Prisma | +| --- | --- | --- | +| Identity | `src/identity/identity.service.ts` | `User`, `Wallet`, `$transaction`; also imports `Prisma`, `User`, `Wallet` types straight from `@prisma/client` | +| Identity | `src/identity/worldcoin/worldcoin.service.ts` | `worldIdVerification`, `user` (dual-writes alongside TypeORM) | +| Auth | `src/auth/auth.service.ts` | `wallet` | +| Analytics | `src/analytics/analytics.service.ts` | `user`, `conversation`, `message` | +| Outbox | `src/outbox/outbox.service.ts` | `outboxEvent` | +| Notifications | `src/notifications/services/notifications.service.ts` | `outboxEvent` | +| Sybil resistance | `src/sybil-resistance/sybil-resistance.service.ts` | `user`, `sybilScore` | +| AI assistant | `src/ai-assistant/services/ai-assistant.service.ts` | `conversation`, `message`, `aiUsageMetric` | +| AI assistant | `src/ai-assistant/services/rag.service.ts` | `contextDocument` | +| AI assistant | `src/ai-assistant/ai-assistant.service.ts` | duplicate of the file above; also imports `PrismaService` (see stale references) | +| AI assistant | `src/ai-assistant/rag.service.ts` | duplicate of the `services/` file above; also imports `PrismaService` (see stale references) | +| App wiring | `src/app.module.ts` | imports `PrismaModule` globally | +| Module wiring | `analytics`, `auth`, `identity`, `notifications`, `outbox`, `sybil-resistance`, `ai-assistant` `.module.ts` | each imports `PrismaModule` | + +Six `.spec.ts` files also import `PrismaService` and must move with it: +`src/identity/identity.service.spec.ts`, `src/identity/worldcoin/worldcoin.service.spec.ts`, +`src/outbox/outbox.service.spec.ts`, `src/sybil-resistance/sybil-resistance.service.spec.ts`, +`src/ai-assistant/ai-assistant.service.spec.ts`, and `src/ai-assistant/services/rag.service.spec.ts`. + +`src/notifications/notifications.service.ts` (the top-level file, not the +`services/` one) does **not** import Prisma; only +`src/notifications/services/notifications.service.ts` does. + +`PrismaModule` is `@Global()`, so `PrismaService` constructs and opens a database connection +during application bootstrap. Removing the Prisma packages without removing these call sites +breaks module resolution at startup, not just at query time. + +The acceptance criterion for #517 ("production and test dependency graphs contain no Prisma +runtime/tooling packages") is therefore **not met by this change**, and is recorded as such +rather than claimed. + +## Conflict with issue #392 + +Open issue **#392, "V2-BE-041 - Complete Prisma-Only Persistence Convergence"**, mandates the +opposite end state: it asks for convergence *onto* Prisma, and would keep or expand +`prisma/schema.prisma` and `prisma/migrations/` as the canonical schema. + +This issue (#517) asks for convergence onto TypeORM. Both cannot be true. They are not +reconciled here. + +Recommended reconciliation, for the maintainer to choose: + +1. **Land this change, then close #392 as superseded.** The evidence favours TypeORM: the + protocol schema (17 migrations, the entities, the indexer projections, the CI gate added + here) is TypeORM, and Prisma is the layer that carries identity, analytics, outbox and AI + data. #392 appears to predate that migration of the protocol schema. +2. **Alternatively, land the Prisma migration gate instead** and revert the CI change. That is + a larger, riskier piece of work and is not attempted here. + +Until that is decided, treat the direction as **unresolved** and do not delete either tree. + +## CI change (the part that was actually broken) + +`.github/workflows/ci.yml` previously ran, as its "Run migration tests" step: + +```bash +npx prisma migrate reset --force +npx prisma migrate deploy +``` + +This exercised the legacy Prisma migration set. The TypeORM migrations in `src/migrations/`, +which are the ones the application actually runs, were never applied, rolled back, or +compared against the entities in CI. The gate was green while the real schema path was +untested. + +That step is replaced by a dedicated `schema-migration-gate` job that, against an empty +PostgreSQL service database, using `src/config/data-source.ts`: + +1. `npm run migration:run` - applies all 17 committed TypeORM migrations in order. +2. Runs `typeorm-ts-node-commonjs migration:generate` and inspects the result: + - exit `0` means TypeORM wrote a migration, i.e. the entities and the committed + migrations disagree, i.e. **schema drift**. The job prints the generated migration and + fails. + - a non-zero exit that does **not** report "No changes in database schema were found" is + treated as an unresolved failure, not a pass. This distinction is deliberate: a + generator crash must never be mistaken for a clean schema. +3. `npm run migration:revert` followed by `npm run migration:run` - proves the most recent + migration is reversible and re-appliable. + +### Known blocker in this gate + +`src/config/data-source.ts` line 3 imports `SnakeNamingStrategy` from +`typeorm-naming-strategies`, but that package is **not declared in `package.json` and is not +present in `package-lock.json`**. The import cannot be resolved. + +Consequence: `npm run migration:run` cannot currently load the data source, and the new gate +will fail at that step with a module-resolution error. This is a pre-existing defect in the +TypeORM migration path, not something this change introduces - the previous Prisma-based +step never touched the file, which is why it went unnoticed. + +It was not fixed here because fixing it requires either an install (adding the dependency +regenerates `package-lock.json`, and a lockfile that cannot be regenerated and verified is +exactly the unverifiable change STAB-BE-004 warns against) or a behavioural change to column +naming (dropping the snake_case strategy would change the generated column names that the 17 +committed migrations were written against). + +**One-line fix for the maintainer** (run locally with the supported toolchain, then review +the lockfile diff): + +```bash +npm install --save typeorm-naming-strategies@^4 +``` + +The gate is expected to be red until that lands. That is the gate doing its job: it is +reporting a real defect rather than reporting a green check on untested code. + +## Declared packages + +| Package | Declared as | Resolved in `package-lock.json` | Kept because | +| --- | --- | --- | --- | +| `@prisma/client` | `dependencies`, `^7.3.0` | `7.4.1` | Runtime import in `src/identity/identity.service.ts`; runtime library import (`@prisma/client/runtime/client`) throughout `src/generated/client/` | +| `@prisma/adapter-libsql` | `dependencies`, `^7.3.0` | `7.4.1` | Runtime import in `src/prisma/prisma.service.ts` (`PrismaLibSql`) | +| `prisma` | `devDependencies`, `^7.10.0` | `7.10.0` | CLI. Used by `npx prisma generate` in the `Dockerfile` and by `test/utils/prisma-test-db.helper.ts` (`prisma db push`) | + +The `Dockerfile` still runs `npx prisma generate` in the builder stage. Whether that step +should remain is tracked by the parallel work in this same pull request (STAB-BE-003) and by +open issue **#392**. It is retained here for consistency with those two threads: removing it +would also require deciding the fate of `src/generated/client/`, which the runtime still +imports. It is called out in the commit message so the decision is not lost. + +The container smoke workflow asserts that `prisma` is **absent** from the shipped production +image, which is the truthful current state: `npm prune --production` removes it. + +## Retained references, with reasons + +### Runtime code (load-bearing) + +| Path | Reference | Why retained | +| --- | --- | --- | +| `src/prisma/prisma.service.ts` | `PrismaService` extends `PrismaClient` from `src/generated/client/client`, backed by `@prisma/adapter-libsql` | Injected into 7 feature modules (11 service files). Removing it is the #392/#517 reconciliation, not a cleanup. | +| `src/prisma/prisma.module.ts` | `@Global()` `PrismaModule` | Same. Registered in `src/app.module.ts`. | +| `src/app.module.ts` | imports and registers `PrismaModule` | Bootstrap wiring for the above. | +| `src/identity/identity.service.ts` | `import { Prisma, User, Wallet } from '@prisma/client'` | The **only** direct `@prisma/client` type import in application code. Blocks removing `@prisma/client`. | +| `src/identity/worldcoin/worldcoin.service.ts` | `PrismaService` | Dual-writes verification records to both ORMs; removing the Prisma half is a data-model decision. | +| `src/auth/auth.service.ts` | `PrismaService` | Wallet lookup for authentication. | +| `src/analytics/analytics.service.ts` | `PrismaService` | Contributor, conversation and message aggregates. | +| `src/outbox/outbox.service.ts` | `PrismaService` | Outbox event persistence. | +| `src/notifications/services/notifications.service.ts` | `PrismaService` | Outbox delivery. | +| `src/sybil-resistance/sybil-resistance.service.ts` | `PrismaService` | Sybil score persistence. | +| `src/ai-assistant/services/ai-assistant.service.ts` | `PrismaService` | Conversation, message and usage-metric persistence. | +| `src/ai-assistant/services/rag.service.ts` | `PrismaService` | RAG context-document retrieval. | +| `src/generated/client/**` | Generated Prisma client, committed to the repository | Imported at runtime by `src/prisma/prisma.service.ts` and copied into the image by the `Dockerfile` runner stage. Regenerated by `npx prisma generate`. | + +### Tests (load-bearing, or at minimum not removable without running the suite) + +| Path | Reference | Why retained | +| --- | --- | --- | +| `test/utils/prisma-test-db.helper.ts` | `execSync('npx prisma db push ...')` | Provisions the isolated SQLite database four AI-assistant e2e specs run against. `prisma db push`, not `migrate`, is what makes the throwaway DB match the schema; see `docs/AI_ASSISTANT_OPERATIONS.md` for why. | +| `test/ai-assistant*.e2e-spec.ts` (4 files) | `setupPrismaTestDatabase` | Call the helper above. | +| `test/utils/ai-assistant-auth.helper.ts` | `PrismaService` | Creates `User` + `Wallet` rows with the RBAC `role` column for e2e auth. | +| `test/utils/ai-assistant-test.module.ts` | `PrismaModule` | Trimmed test module wiring. | +| `test/utils/test-helpers.ts` | `PrismaService`, `PrismaModule` | See "Stale" below. | +| `test/fixtures/contracts/fixtures.example.spec.ts` | `PrismaService`, `PrismaModule` | See "Stale" below. | +| `test/outbox-idempotent-delivery.integration.spec.ts` | `jest.mock('../src/prisma/prisma.service')` | Mocks the service precisely to avoid loading the native `@libsql` driver adapter. The mock is the point. | +| `src/auth/auth.service.spec.ts`, `src/auth/guards/roles.guard.spec.ts`, `src/identity/identity.service.spec.ts`, `src/identity/worldcoin/worldcoin.service.spec.ts`, `src/outbox/outbox.service.spec.ts`, `src/sybil-resistance/sybil-resistance.service.spec.ts`, `src/ai-assistant/ai-assistant.service.spec.ts`, `src/ai-assistant/services/rag.service.spec.ts`, `src/jobs/jobs.service.spec.ts` | `PrismaService` test doubles | Injecting the service into the modules under test. `src/jobs/jobs.service.spec.ts` mocks the module outright. | +| `src/dockerfile.spec.ts` | asserts `prisma/schema.prisma` contains `binaryTargets = ["native", "linux-musl"]` | **This test is what pins `prisma/schema.prisma` in place.** Deleting the schema breaks an active unit test. | +| `src/entities/user.entity.spec.ts` | "TypeORM <-> Prisma sync" field-coverage assertions | Active test that compares the two `User` definitions. It is a migration aid, and it is evidence the two layers are intended to converge. | + +### Generated client, environment, and toolchain configuration + +| Path | Reference | Why retained | +| --- | --- | --- | +| `prisma/schema.prisma` | Source of truth for the Prisma client | Generates `src/generated/client/`. Pinned by `src/dockerfile.spec.ts`. | +| `prisma/migrations/**` (4 migrations) | `20260122115647_init`, `20260129_add_sybil_scores`, `20260728000000_add_user_role`, `20260924000000_add_outbox_event`, plus `migration_lock.toml` | **Migration history for data that exists.** `prisma db push` in the e2e helper bypasses it, but the rows created by `prisma migrate`-era deployments are only described here. Not deletable without a data-retention decision. | +| `prisma.config.ts` | `defineConfig` with `schema: "prisma/schema.prisma"` and `migrations.path: "prisma/migrations"` | Required by the `prisma` CLI v7. Removing it breaks `npx prisma generate` in the `Dockerfile`. | +| `Dockerfile` line 14-16 | `npx prisma generate` | See "Declared packages". | +| `.env.example` lines 116-120 | `# Prisma Configuration (SQLite/LibSQL)` / `DATABASE_URL=file:./dev.db` | Required by `PrismaService`, which defaults to `file:./dev.db`. **See the hazard below.** | +| `.env.docker` line 4 | `# PostgreSQL (Prisma)` comment above `DATABASE_URL=postgresql://...` | Comment only. Both the value and the comment are wrong for Prisma; see the hazard below. | +| `.gitignore` line 67 (`/generated/prisma`) | **Removed** | The generator output is pinned to `../src/generated/client` by `prisma/schema.prisma`, so a repo-root `generated/prisma` directory can no longer be produced. The entry was dead. | + +### Environment variable hazard (found, not fixed) + +`DATABASE_URL` is overloaded between the two layers, with incompatible meanings: + +- `src/config/data-source.ts`: any value in `DATABASE_URL` selects **PostgreSQL** (`type: 'postgres', url: DATABASE_URL`). +- `src/prisma/prisma.service.ts`: `DATABASE_URL` is passed to `PrismaLibSql`, which only accepts a **libsql/SQLite** URL. + +`.env.example` sets `DATABASE_URL=file:./dev.db`, which satisfies Prisma and breaks TypeORM. +`.env.docker` sets `DATABASE_URL=postgresql://postgres:postgres@postgres:5432/truthbounty`, +which satisfies TypeORM and breaks Prisma. Neither file is correct for both layers. + +This is not fixable in a doc edit, and renaming the variable would touch 7 feature +modules. It +is recorded here as the concrete technical work that #392's reconciliation requires: the two +layers need distinct connection configuration before either can be removed. + +## Stale references (recorded, not changed) + +These are inaccurate or dead. Each is a documentation or dead-code change with no runtime +effect, but each is listed rather than silently fixed so the reviewer can decide. + +| Path | Line(s) | Problem | Why not changed here | +| --- | --- | --- | --- | +| `src/ai-assistant/ai-assistant.service.ts` | whole file | Duplicate of `src/ai-assistant/services/ai-assistant.service.ts`. The module wires the `services/` version; this one is only imported by `src/ai-assistant/ai-assistant.service.spec.ts`. | Deleting it requires deleting or repointing an active spec. Not verifiable without running tests. | +| `src/ai-assistant/rag.service.ts` | whole file | Duplicate of `src/ai-assistant/services/rag.service.ts`, same situation. | Same. | +| `test/utils/test-helpers.ts` | `clearDatabase`, `seedTestData` | Uses `TRUNCATE ... RESTART IDENTITY CASCADE` (PostgreSQL-only) against a SQLite/libsql database, and seeds `prisma.stake`, `prisma.reward` and `prisma.dispute`, **none of which exist** in `prisma/schema.prisma`. Imported only by `test/fixtures/contracts/fixtures.example.spec.ts`. | Appears to be dead scaffolding whose consumer is itself an example fixture. Confirming it is dead requires running the suite. | +| `test/fixtures/contracts/fixtures.example.spec.ts` | whole file | Depends on the above, so it inherits the same problems. Named `.example.spec.ts` but matches the `*.spec.ts` test regex in `jest.config.js`, so it is collected. | Same. | +| `docs/API_REFERENCE.md` | 493 | "The API is built with NestJS and uses Prisma for database operations." Inaccurate for the protocol schema. | Documentation drift; worth a follow-up with the rest of the doc corrections. | +| `ARCHITECTURE.md` | 341 | Sequence diagram labels a step "Domain Action (Prisma Transaction)". | Inside an ASCII diagram; editing it risks corrupting the layout. | +| `docs/BACKEND_DOCUMENTATION.md` | 26 | ASCII diagram ends a line with "Prisma)". | Inside an ASCII diagram, same reason. | +| `src/COMPONENTS.md` | 779-890 | Documents a `PrismaService` with `prisma.claim` examples; those models do not exist. | Large stale document; a targeted rewrite is its own change. | +| `src/COMPONENTS_QUICK_REFERENCE.md` | 26, 128, 218, 450 | Same class of staleness, and links to `prisma/schema.prisma` as "Database Schema". | Same. | +| `scripts/generate-outbox-migration.js` | whole file | One-off helper that diffs Prisma schemas. Not referenced by any `package.json` script or workflow. | Historical record of how the outbox migration was produced; deleting it removes provenance for `prisma/migrations/20260924000000_add_outbox_event`. | +| `PULL_REQUEST.md` | 33 | Historical PR description mentioning a Prisma-aware `database-profiler.ts`. | Historical artefact, not living documentation. | +| `.github/pull_request_template.md` | 20 | Checkbox asserts "TypeORM/PostgreSQL remains the only persistence architecture". True as a policy for *new* pull requests, not as a description of the tree. | The enforcement for it already exists as a `::warning::` in `v2-policy-advisory.yml`. Rewording it risks weakening the policy while the tree still contains Prisma. | +| `docs/runbooks/outbox-notification-delivery.md` | 5 | "Persistence: Prisma `OutboxEvent` table" | **Accurate** - `src/outbox/outbox.service.ts` really does persist there. Also outside this change's scope. | +| `docs/AI_ASSISTANT_ARCHITECTURE.md`, `docs/AI_ASSISTANT_SECURITY.md`, `docs/AI_ASSISTANT_OPERATIONS.md` | various | Describe the Prisma-backed AI data. | Accurate and deliberately detailed; already state the TypeORM/Prisma split. Keep. | +| `.github/workflows/v2-policy-advisory.yml` | 24, 28 | Uses `|| true` and `exit 0`. | Not masking: the workflow is explicitly advisory, is named "(advisory)", and ends with "Advisory mode: findings do not fail this workflow." A `::warning::` is the intended output. Listed here so the pattern is accounted for rather than overlooked. | + +## Follow-up, in dependency order + +1. Add `typeorm-naming-strategies` to `package.json` and commit the regenerated lockfile. + Unblocks the migration gate. (Blocking, one command.) +2. Resolve #392 vs #517 and record the decision. Everything below depends on it. +3. Give the two persistence layers distinct connection configuration so `DATABASE_URL` is not + overloaded. +4. If TypeORM wins: port `User`/`Wallet` to TypeORM, then work down the module list in + "Blocker" above, deleting each Prisma call site and its test double as you go. Delete + `src/prisma/`, `src/generated/client/`, `prisma/`, `prisma.config.ts`, and the three + Prisma packages last, after `src/dockerfile.spec.ts` is updated. +5. If Prisma wins: revert the `schema-migration-gate` job in `ci.yml` and add an equivalent + Prisma-based gate, including a real `prisma migrate` run against an empty database. +6. Either way, delete `prisma/migrations/` last and only with a data-retention decision. diff --git a/package.json b/package.json index b8666a23..77ecf0a1 100644 --- a/package.json +++ b/package.json @@ -6,7 +6,8 @@ "private": true, "license": "UNLICENSED", "engines": { - "node": ">=20 <21" + "node": ">=20 <21", + "npm": ">=10 <11" }, "scripts": { "build": "nest build",