Add blocking Semgrep SAST gate

.github/workflows/sast.yml

@@ -0,0 +1,69 @@
+name: SAST
+
+on:
+  push:
+    branches: [main, release/*]
+  pull_request:
+    branches: [main, release/*]
+  # Allow manual trigger
+  workflow_dispatch:
+
+permissions:
+  contents: read
+  security-events: write
+
+jobs:
+  semgrep:
+    name: semgrep
+    runs-on: ubuntu-latest
+
+    steps:
+      - name: Harden the runner (Audit all outbound calls)
+        uses: step-security/harden-runner@9af89fc71515a100421586dfdb3dc9c984fbf411 # v2.19.4
+        with:
+          egress-policy: audit
+
+      - name: Checkout code
+        uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v6.2.0
+
+      - name: Setup Python
+        uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5.6.0
+        with:
+          python-version: "3.12"
+
+      - name: Install Semgrep
+        run: pip install semgrep
+
+      # Blocking policy and suppression process documented in
+      # docs/VULNERABILITY_MANAGEMENT.md (OSPS-VM-06.02). Scan is run
+      # unfiltered (all severities) so WARNING/INFO findings still reach the
+      # Code Scanning tab; the ERROR-only blocking gate is enforced as a
+      # separate step below rather than by filtering the scan itself, so it
+      # doesn't strip lower-severity findings from the SARIF upload.
+      - name: Run Semgrep SAST scan
+        run: |
+          semgrep scan \
+            --config p/security-audit \
+            --config p/secrets \
+            --config p/owasp-top-ten \
+            --config p/javascript \
+            --config p/typescript \
+            --config p/react \
+            --json-output semgrep.json \
+            --sarif-output semgrep.sarif \
+            .
+
+      - name: Upload SARIF to code scanning
+        if: ${{ !cancelled() }}
+        uses: github/codeql-action/upload-sarif@8aad20d150bbac5944a9f9d289da16a4b0d87c1e # v4.36.2
+        with:
+          sarif_file: semgrep.sarif
+
+      - name: Enforce blocking policy (ERROR-severity findings fail the build)
+        run: |
+          error_count=$(jq '[.results[] | select(.extra.severity == "ERROR")] | length' semgrep.json)
+          echo "ERROR-severity findings: $error_count"
+          if [ "$error_count" -gt 0 ]; then
+            jq -r '.results[] | select(.extra.severity == "ERROR") | "\(.path):\(.start.line)  \(.check_id)"' semgrep.json
+            exit 1
+          fi

Dockerfile

@@ -58,7 +58,14 @@ RUN chmod +x /entrypoint.sh
 
 EXPOSE 5000
 
+# No USER instruction here by design: the container must start as root so
+# entrypoint.sh can chown/usermod /app and /app/data to the host-provided
+# PUID/PGID (LinuxServer.io convention for bind-mounted volumes), then it
+# drops privileges itself via `su-exec questarr` before exec'ing CMD (see
+# entrypoint.sh's final line).
+# nosemgrep: dockerfile.security.missing-user-entrypoint.missing-user-entrypoint -- see comment above
 ENTRYPOINT ["/entrypoint.sh"]
+# nosemgrep: dockerfile.security.missing-user.missing-user -- entrypoint.sh drops to the unprivileged questarr user via su-exec before this CMD ever runs
 CMD ["npm", "run", "start"]
 
 LABEL org.opencontainers.image.title="Questarr"

docs/VULNERABILITY_MANAGEMENT.md

@@ -122,20 +122,30 @@ SAST covers vulnerabilities in Questarr's own source code (`client/`,
 
 ### 2.1 Identification
 
-[CodeQL](https://codeql.github.com/) analysis runs via GitHub's **default
-setup** (Settings → Code security → Code scanning), covering JavaScript/
-TypeScript (client + server) sources on every pull request, push to `main`,
-and a periodic background schedule GitHub manages automatically.
-
-Default setup is deliberately used instead of a custom `codeql.yml`
-workflow: GitHub does not allow a repository to run both at once (SARIF
-uploads from a custom/"advanced" workflow are rejected outright while
-default setup is enabled — see the note in
-[`docs/SECURITY_ASSESSMENT.md`](/docs/SECURITY_ASSESSMENT.md)), and default
-setup requires no workflow-file maintenance as CodeQL/query versions evolve.
-
-Results are uploaded to the repository's Security → Code scanning alerts
-tab, which is the source of truth for open SAST findings.
+Two SAST layers run, deliberately kept separate rather than merged into one
+tool/workflow:
+
+- [CodeQL](https://codeql.github.com/) analysis runs via GitHub's **default
+  setup** (Settings → Code security → Code scanning), covering JavaScript/
+  TypeScript (client + server) sources on every pull request, push to
+  `main`, and a periodic background schedule GitHub manages automatically.
+  Default setup is deliberately used instead of a custom `codeql.yml`
+  workflow: GitHub does not allow a repository to run both at once (SARIF
+  uploads from a custom/"advanced" CodeQL workflow are rejected outright
+  while default setup is enabled — see the note in
+  [`docs/SECURITY_ASSESSMENT.md`](/docs/SECURITY_ASSESSMENT.md)), and
+  default setup requires no workflow-file maintenance as CodeQL/query
+  versions evolve.
+- [Semgrep](https://semgrep.dev/) runs as the `semgrep` job in
+  [`.github/workflows/sast.yml`](/.github/workflows/sast.yml) on every push
+  and pull request targeting `main`/`release/*`. This is a separate SARIF
+  producer (tool name `Semgrep`, not `CodeQL`), so it doesn't collide with
+  CodeQL default setup, and it exists specifically to give SAST a per-PR
+  pass/fail signal (§2.3) that default setup alone cannot provide.
+
+Both upload their SARIF output to the repository's Security → Code scanning
+alerts tab, which is the source of truth for open SAST findings regardless
+of which tool produced them.
 
 ### 2.2 Prioritization & remediation thresholds
 
@@ -165,12 +175,38 @@ Remediation process, per finding:
    A pattern of dismissals in the same area should prompt a query-suppression
    review rather than repeated one-off dismissals.
 
-### 2.3 Scope note
-
-Unlike the SCA gate (§1.3), SAST results are not wired as a merge-blocking
-status check, because CodeQL alert triage requires human judgment about
-exploitability that a hard gate would short-circuit, and because default
-setup (§2.1) does not expose a per-PR pass/fail signal the way a custom
-workflow job would. New alerts are enforced via the SLA table above and
-periodic maintainer review of the Security tab, not by blocking individual
-PRs at scan time.
+### 2.3 Pre-merge gate
+
+CodeQL's default setup (§2.1) intentionally has no per-PR blocking signal —
+alert triage there follows the SLA table above and periodic maintainer
+review, since it requires human judgment about exploitability that a hard
+gate would short-circuit.
+
+Semgrep closes that gap with an actual merge-blocking check:
+
+- The `semgrep` job ([`.github/workflows/sast.yml`](/.github/workflows/sast.yml))
+  runs `semgrep scan` with the `p/security-audit`, `p/secrets`,
+  `p/owasp-top-ten`, `p/javascript`, `p/typescript`, and `p/react` rulesets,
+  unfiltered by severity, writing both a JSON and a SARIF copy of every
+  finding. The SARIF copy is uploaded to the Code scanning tab as-is, so
+  `WARNING`/`INFO` findings stay visible there (matching the Medium/Low rows
+  in §2.2) instead of being dropped from the scan entirely. A separate step
+  then reads the JSON output and fails the job — blocking merge — if any
+  `ERROR`-severity finding is present; this dedicated step is the required
+  status check for OSPS-VM-06.02, kept independent of the scan step so the
+  blocking condition can never silently filter what reaches Code Scanning.
+- **Suppression (declaring a finding non-exploitable):** a blocking finding
+  may be suppressed only via an inline `// nosemgrep: <rule-id>` comment on
+  the line immediately preceding the flagged code (or on the same line —
+  Semgrep does not recognize the directive anywhere else), with a mandatory
+  trailing justification, e.g.:
+
+  ```ts
+  // nosemgrep: javascript.lang.security.audit.path-traversal -- path is validated against an allow-list in server/middleware.ts:293 before this line
+  ```
+
+  A suppression without a stated reason is not acceptable and should be
+  rejected in review. Suppressions are visible in the diff, so they go
+  through the same PR review as any other change — no separate approval
+  channel exists for dismissing a finding, unlike CodeQL's Code Scanning UI
+  dismissal path (§2.2, item 3).

server/credential-crypto.ts

@@ -70,7 +70,7 @@ export async function getCredentialsEncryptionKey(): Promise<Buffer> {
 
 function encryptWithKey(plaintext: string, key: Buffer): string {
   const iv = crypto.randomBytes(IV_LENGTH);
-  const cipher = crypto.createCipheriv(ALGORITHM, key, iv);
+  const cipher = crypto.createCipheriv(ALGORITHM, key, iv, { authTagLength: AUTH_TAG_LENGTH });
   const ciphertext = Buffer.concat([cipher.update(plaintext, "utf8"), cipher.final()]);
   const authTag = cipher.getAuthTag();
   return ENCRYPTED_PREFIX + Buffer.concat([iv, authTag, ciphertext]).toString("base64");
@@ -81,7 +81,7 @@ function decryptWithKey(value: string, key: Buffer): string {
   const iv = raw.subarray(0, IV_LENGTH);
   const authTag = raw.subarray(IV_LENGTH, IV_LENGTH + AUTH_TAG_LENGTH);
   const ciphertext = raw.subarray(IV_LENGTH + AUTH_TAG_LENGTH);
-  const decipher = crypto.createDecipheriv(ALGORITHM, key, iv);
+  const decipher = crypto.createDecipheriv(ALGORITHM, key, iv, { authTagLength: AUTH_TAG_LENGTH });
   decipher.setAuthTag(authTag);
   return Buffer.concat([decipher.update(ciphertext), decipher.final()]).toString("utf8");
 }