diff --git a/.dockerignore b/.dockerignore index 0b7988b01..fd65d4dc9 100644 --- a/.dockerignore +++ b/.dockerignore @@ -10,7 +10,6 @@ .gitignore .github/ codecov.yml -.goreleaser.yaml .sourcery.yml .claude/ @@ -155,7 +154,7 @@ docker-compose*.yml docs/implementation/ # ----------------------------------------------------------------------------- -# GoReleaser & dist artifacts +# Build output directories (e.g. frontend/dist/) # ----------------------------------------------------------------------------- dist/ diff --git a/.github/release-drafter.yml b/.github/release-drafter.yml deleted file mode 100644 index 85ff1f0f6..000000000 --- a/.github/release-drafter.yml +++ /dev/null @@ -1,26 +0,0 @@ -name-template: 'v$NEXT_PATCH_VERSION' -tag-template: 'v$NEXT_PATCH_VERSION' -categories: - - title: '๐Ÿš€ Features' - labels: - - 'feature' - - 'feat' - - title: '๐Ÿ› Fixes' - labels: - - 'bug' - - 'fix' - - title: '๐Ÿงฐ Maintenance' - labels: - - 'chore' - - title: '๐Ÿงช Tests' - labels: - - 'test' -change-template: '- $TITLE @$AUTHOR (#$NUMBER)' -template: | - ## What's Changed - - $CHANGES - - ---- - - Full Changelog: https://github.com/${{ github.repository }}/compare/$FROM_TAG...$TO_TAG diff --git a/.github/skills/README.md b/.github/skills/README.md index c6a6ce846..96a02e42e 100644 --- a/.github/skills/README.md +++ b/.github/skills/README.md @@ -69,7 +69,6 @@ Agent Skills are self-documenting, AI-discoverable task definitions that combine | Skill Name | Category | Description | Status | |------------|----------|-------------|--------| -| [utility-version-check](./utility-version-check.SKILL.md) | utility | Validate version matches git tag | โœ… Active | | [utility-clear-go-cache](./utility-clear-go-cache.SKILL.md) | utility | Clear Go build and module caches | โœ… Active | | [utility-bump-beta](./utility-bump-beta.SKILL.md) | utility | Increment beta version number | โœ… Active | | [utility-db-recovery](./utility-db-recovery.SKILL.md) | utility | Database integrity check and recovery | โœ… Active | @@ -264,7 +263,6 @@ python3 .github/skills/scripts/validate-skills.py --single .github/skills/{skill - `test-backend-coverage` - `integration-test-crowdsec` - `security-scan-trivy` - - `utility-version-check` ## Best Practices diff --git a/.github/skills/utility-bump-beta.SKILL.md b/.github/skills/utility-bump-beta.SKILL.md index 34e552cdc..365e155ed 100644 --- a/.github/skills/utility-bump-beta.SKILL.md +++ b/.github/skills/utility-bump-beta.SKILL.md @@ -183,7 +183,6 @@ After running this skill: ## Related Skills -- [utility-version-check](./utility-version-check.SKILL.md) - Validate version matches tags - [build-check-go](../build-check-go.SKILL.md) - Verify build after version bump ## Notes diff --git a/.github/skills/utility-version-check-scripts/run.sh b/.github/skills/utility-version-check-scripts/run.sh deleted file mode 100755 index 3bf38c71d..000000000 --- a/.github/skills/utility-version-check-scripts/run.sh +++ /dev/null @@ -1,22 +0,0 @@ -#!/usr/bin/env bash -set -euo pipefail - -# ============================================================================== -# Utility: Version Check - Execution Script -# ============================================================================== -# This script validates that the .version file matches the latest git tag. -# It wraps the original check-version-match-tag.sh script. -# -# Usage: ./run.sh -# Exit codes: 0 = success, 1 = version mismatch -# ============================================================================== - -# Determine the repository root directory -SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" -REPO_ROOT="$(cd "$SCRIPT_DIR/../../.." && pwd)" - -# Change to repository root -cd "$REPO_ROOT" - -# Execute the version check script -exec scripts/check-version-match-tag.sh "$@" diff --git a/.github/skills/utility-version-check.SKILL.md b/.github/skills/utility-version-check.SKILL.md deleted file mode 100644 index 9f31665da..000000000 --- a/.github/skills/utility-version-check.SKILL.md +++ /dev/null @@ -1,142 +0,0 @@ ---- -name: "utility-version-check" -version: "1.0.0" -description: "Validates that VERSION.md/version file matches the latest git tag for release consistency" -author: "Charon Project" -license: "MIT" -tags: - - "utility" - - "versioning" - - "validation" - - "git" -compatibility: - os: - - "linux" - - "darwin" - shells: - - "bash" -requirements: - - name: "git" - version: ">=2.0" - optional: false -environment_variables: [] -parameters: [] -outputs: - - name: "exit_code" - type: "integer" - description: "0 if version matches, 1 if mismatch or error" -metadata: - category: "utility" - subcategory: "versioning" - execution_time: "short" - risk_level: "low" - ci_cd_safe: true - requires_network: false - idempotent: true ---- - -# Utility: Version Check - -## Overview - -Validates that the version specified in `.version` file matches the latest git tag. This ensures version consistency across the codebase and prevents version drift during releases. The check is used in CI/CD to enforce version tagging discipline. - -## Prerequisites - -- Git repository with tags -- `.version` file in repository root (optional) - -## Usage - -### Basic Usage - -```bash -.github/skills/utility-version-check-scripts/run.sh -``` - -### Via Skill Runner - -```bash -.github/skills/scripts/skill-runner.sh utility-version-check -``` - -### Via VS Code Task - -Use the task: **Utility: Check Version Match Tag** - -## Parameters - -This skill accepts no parameters. - -## Environment Variables - -This skill requires no environment variables. - -## Outputs - -- **Success Exit Code**: 0 - Version matches latest tag or no tags exist -- **Error Exit Codes**: 1 - Version mismatch detected -- **Console Output**: Validation result message - -### Success Output Example - -``` -OK: .version matches latest Git tag v0.3.0-beta.2 -``` - -### Error Output Example - -``` -ERROR: .version (0.3.0-beta.3) does not match latest Git tag (v0.3.0-beta.2) -To sync, either update .version or tag with 'v0.3.0-beta.3' -``` - -## Examples - -### Example 1: Check Version During Release - -```bash -# Before tagging a new release -.github/skills/utility-version-check-scripts/run.sh -``` - -### Example 2: CI/CD Integration - -```yaml -- name: Validate Version - run: .github/skills/scripts/skill-runner.sh utility-version-check -``` - -## Version Normalization - -The skill normalizes both the `.version` file content and git tag by: -- Stripping leading `v` prefix (e.g., `v1.0.0` โ†’ `1.0.0`) -- Removing newline and carriage return characters -- Comparing normalized versions - -This allows flexibility in tagging conventions while ensuring consistency. - -## Error Handling - -- **No .version file**: Exits with 0 (skip check) -- **No git tags**: Exits with 0 (skip check, allows commits before first tag) -- **Version mismatch**: Exits with 1 and provides guidance -- **Git errors**: Script fails with appropriate error message - -## Related Skills - -- [utility-bump-beta](./utility-bump-beta.SKILL.md) - Increment beta version -- [build-check-go](../build-check-go.SKILL.md) - Verify Go build integrity - -## Notes - -- This check is **non-blocking** when no tags exist (allows initial development) -- Version format is flexible (supports semver, beta, alpha suffixes) -- Used in CI/CD to prevent merging PRs with version mismatches -- Part of the release automation workflow - ---- - -**Last Updated**: 2025-12-20 -**Maintained by**: Charon Project -**Source**: `scripts/check-version-match-tag.sh` diff --git a/.github/workflows/auto-changelog.yml b/.github/workflows/auto-changelog.yml deleted file mode 100644 index c57776d0a..000000000 --- a/.github/workflows/auto-changelog.yml +++ /dev/null @@ -1,29 +0,0 @@ -name: Auto Changelog (Release Drafter) - -on: - workflow_run: - workflows: ["Docker Build, Publish & Test"] - types: [completed] - branches: [ main ] - release: - types: [published] - -concurrency: - group: ${{ github.workflow }}-${{ github.event_name }}-${{ github.event.workflow_run.head_branch || github.head_ref || github.ref_name }} - cancel-in-progress: true - -permissions: - contents: write - -jobs: - update-draft: - runs-on: ubuntu-latest - if: ${{ github.event_name != 'workflow_run' || (github.event.workflow_run.conclusion == 'success' && github.event.workflow_run.head_branch == 'main') }} - steps: - - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 - with: - ref: ${{ github.event.workflow_run.head_sha || github.sha }} - - name: Draft Release - uses: release-drafter/release-drafter@34d80673e067bdc0c24568d3af899c216adcfaa9 # v7 - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/auto-versioning.yml b/.github/workflows/auto-versioning.yml deleted file mode 100644 index 5dc57e2e3..000000000 --- a/.github/workflows/auto-versioning.yml +++ /dev/null @@ -1,163 +0,0 @@ -name: Auto Versioning and Release - -# SEMANTIC VERSIONING RULES: -# - PATCH (0.14.1 โ†’ 0.14.2): fix:, perf:, refactor:, docs:, style:, test:, build:, ci: -# - MINOR (0.14.1 โ†’ 0.15.0): feat:, feat(...): -# - MAJOR (0.14.1 โ†’ 1.0.0): MANUAL ONLY - Create git tag manually when ready for 1.0.0 -# -# โš ๏ธ Major version bumps are intentionally disabled in automation to prevent accidents. - -on: - workflow_run: - workflows: ["Docker Build, Publish & Test"] - types: [completed] - branches: [ main ] - -concurrency: - group: ${{ github.workflow }}-${{ github.event.workflow_run.head_branch || github.ref }} - cancel-in-progress: false # Don't cancel in-progress releases - -permissions: - contents: write # Required for creating releases via API (removed unused pull-requests: write) - -jobs: - version: - runs-on: ubuntu-latest - if: ${{ github.event.workflow_run.conclusion == 'success' && github.event.workflow_run.head_branch == 'main' }} - steps: - - name: Checkout - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 - with: - fetch-depth: 0 - ref: ${{ github.event.workflow_run.head_sha || github.sha }} - - - name: Calculate Semantic Version - id: semver - uses: paulhatch/semantic-version@502226b17e3e242e4befd0a45690fcbb27dd1373 # v6.0.3 - with: - # The prefix to use to create tags - tag_prefix: "v" - # Regex pattern for major version bump - DISABLED (manual only) - # Use a pattern that will never match to prevent automated major bumps - major_pattern: "/__MANUAL_MAJOR_BUMP_ONLY__/" - # Regex pattern for minor version bump (new features) - # Matches "feat:" at line start, including "* feat:" bullet lines in squash-merge commit bodies - minor_pattern: "/^(\\* )?feat(\\(.+\\))?:/" - # Patch bumps: All other commits (fix:, chore:, etc.) are treated as patches by default - # Pattern to determine formatting - version_format: "${major}.${minor}.${patch}" - # If no tags are found, this version is used - version_from_branch: "0.0.0" - # This helps it search through history to find the last tag - search_commit_body: true - # Important: This enables the output 'changed' which your other steps rely on - enable_prerelease_mode: false - - - name: Show version - run: | - echo "Next version: ${{ steps.semver.outputs.version }}" - echo "Version changed: ${{ steps.semver.outputs.changed }}" - - - name: Determine tag name - id: determine_tag - run: | - # Normalize the version: remove any leading 'v' so we don't end up with 'vvX.Y.Z' - RAW="${{ steps.semver.outputs.version }}" - VERSION_NO_V="${RAW#v}" - TAG="v${VERSION_NO_V}" - echo "Determined tag: $TAG" - echo "tag=$TAG" >> "$GITHUB_OUTPUT" - - - name: Check for existing GitHub Release - id: check_release - run: | - TAG="${{ steps.determine_tag.outputs.tag }}" - echo "Checking for release for tag: ${TAG}" - STATUS=$(curl -s -o /dev/null -w "%{http_code}" \ - -H "Authorization: token ${GITHUB_TOKEN}" \ - -H "Accept: application/vnd.github+json" \ - "https://api.github.com/repos/${GITHUB_REPOSITORY}/releases/tags/${TAG}") || true - if [ "${STATUS}" = "200" ]; then - echo "exists=true" >> "$GITHUB_OUTPUT" - echo "โ„น๏ธ Release already exists for tag: ${TAG}" - else - echo "exists=false" >> "$GITHUB_OUTPUT" - echo "โœ… No existing release found for tag: ${TAG}" - fi - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - - - name: Generate Release Changelog - id: changelog - if: ${{ steps.semver.outputs.changed == 'true' && steps.check_release.outputs.exists == 'false' }} - run: | - PREV_TAG=$(git describe --tags --abbrev=0 HEAD~1 2>/dev/null || echo "") - NEW_TAG="${{ steps.determine_tag.outputs.tag }}" - REPO="${{ github.repository }}" - - rm -f /tmp/feat.txt /tmp/fix.txt - touch /tmp/feat.txt /tmp/fix.txt - - # Extract feat/fix lines from commit messages, including squash-merge bullet bodies ("* feat: ...") - process_lines() { - while IFS= read -r line; do - clean="${line#\* }" - if echo "$clean" | grep -qE '^feat(\([^)]+\))?!?:'; then - echo "$clean" | sed -E 's/^feat(\([^)]+\))?!?: //' >> /tmp/feat.txt - elif echo "$clean" | grep -qE '^(fix|perf)(\([^)]+\))?!?:'; then - echo "$clean" | sed -E 's/^(fix|perf)(\([^)]+\))?!?: //' >> /tmp/fix.txt - fi - done - } - - if [ -n "$PREV_TAG" ]; then - process_lines < <(git log --format="%B" "${PREV_TAG}..HEAD") - else - process_lines < <(git log --format="%B" -50 HEAD) - fi - - { - echo "## What's Changed" - echo "" - if [ -s /tmp/feat.txt ]; then - echo "### ๐Ÿš€ Features" - sed 's/^/- /' /tmp/feat.txt - echo "" - fi - if [ -s /tmp/fix.txt ]; then - echo "### ๐Ÿ› Bug Fixes & Improvements" - sed 's/^/- /' /tmp/fix.txt - echo "" - fi - if [ ! -s /tmp/feat.txt ] && [ ! -s /tmp/fix.txt ]; then - echo "_Dependency updates and maintenance._" - echo "" - fi - echo "---" - echo "" - if [ -n "$PREV_TAG" ]; then - echo "**Full Changelog**: https://github.com/${REPO}/compare/${PREV_TAG}...${NEW_TAG}" - fi - } > /tmp/release_body.md - - echo "Generated changelog:" - cat /tmp/release_body.md - - - name: Create GitHub Release (creates tag via API) - if: ${{ steps.semver.outputs.changed == 'true' && steps.check_release.outputs.exists == 'false' }} - uses: softprops/action-gh-release@3d0d9888cb7fd7b750713d6e236d1fcb99157228 # v3 - with: - tag_name: ${{ steps.determine_tag.outputs.tag }} - name: Release ${{ steps.determine_tag.outputs.tag }} - body_path: /tmp/release_body.md - make_latest: true - draft: false - prerelease: false - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - - - name: Output release information - if: ${{ steps.semver.outputs.changed == 'true' && steps.check_release.outputs.exists == 'false' }} - run: | - echo "โœ… Successfully created release: ${{ steps.determine_tag.outputs.tag }}" - echo "๐Ÿ“ฆ Release URL: https://github.com/${{ github.repository }}/releases/tag/${{ steps.determine_tag.outputs.tag }}" diff --git a/.github/workflows/docker-build.yml b/.github/workflows/docker-build.yml index 598e3b48f..269447c99 100644 --- a/.github/workflows/docker-build.yml +++ b/.github/workflows/docker-build.yml @@ -35,6 +35,16 @@ on: # See GH #1235 / docs/plans/current_spec.md for the full rationale. branches: [main, development] workflow_dispatch: + # Reviewed as part of a default-branch/workflow_run reliability pass. This + # link is dormant in normal operation: docker-lint.yml's only trigger is + # workflow_dispatch, so nothing in the repo's day-to-day flow ever causes + # Docker Lint to run and chain into this workflow. Left unchanged + # deliberately -- this workflow's real, CI-critical triggers are the + # direct push/pull_request above, which never route through workflow_run + # and are therefore categorically unaffected by that bug class. Making + # Docker Lint auto-trigger would be a functional scope change (hadolint + # running on every push/PR), not a trigger-reliability fix, so it's out + # of scope here. workflow_run: workflows: ["Docker Lint"] types: [completed] diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index f4110bc37..295c8f233 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -4,6 +4,7 @@ on: workflow_run: workflows: ["Docker Build, Publish & Test"] types: [completed] + branches: [main] workflow_dispatch: # Allow manual trigger # Sets permissions to allow deployment to GitHub Pages diff --git a/.github/workflows/e2e-tests-split.yml b/.github/workflows/e2e-tests-split.yml index 3fb9611d6..485fd56dd 100644 --- a/.github/workflows/e2e-tests-split.yml +++ b/.github/workflows/e2e-tests-split.yml @@ -323,13 +323,32 @@ jobs: - name: Install dependencies run: npm ci --ignore-scripts + - name: Harden apt for flaky CI mirrors + run: | + sudo tee /etc/apt/apt.conf.d/99-charon-ci-reliability > /dev/null <<'APTCONF' + Acquire::Retries "3"; + Acquire::http::Timeout "15"; + Acquire::https::Timeout "15"; + APTCONF + - name: Install Playwright Chromium run: | + set -uo pipefail echo "๐Ÿ“ฆ Installing Chromium..." - npx playwright install --with-deps chromium - EXIT_CODE=$? - echo "โœ… Install command completed (exit code: $EXIT_CODE)" - exit "$EXIT_CODE" + for attempt in 1 2 3; do + timeout 10m npx playwright install --with-deps chromium && break + if [ "$attempt" -lt 3 ]; then + echo "Attempt ${attempt}/3 failed; retrying in 15s..." >&2 + echo "Cleaning up any orphaned apt/dpkg processes and stale locks..." >&2 + sudo pkill -9 -f '(^|/)(apt|apt-get|dpkg)( |$)' 2>/dev/null || true + sudo rm -f /var/lib/dpkg/lock-frontend /var/lib/dpkg/lock /var/cache/apt/archives/lock 2>/dev/null || true + sudo dpkg --configure -a > /dev/null 2>&1 || true + sleep 15 + else + echo "ERROR: Playwright chromium install failed after 3 attempts" >&2 + exit 1 + fi + done - name: Run Chromium Security Enforcement Tests run: | @@ -558,21 +577,51 @@ jobs: - name: Install dependencies run: npm ci --ignore-scripts + - name: Harden apt for flaky CI mirrors + run: | + sudo tee /etc/apt/apt.conf.d/99-charon-ci-reliability > /dev/null <<'APTCONF' + Acquire::Retries "3"; + Acquire::http::Timeout "15"; + Acquire::https::Timeout "15"; + APTCONF + - name: Install Playwright Chromium (required by security-tests dependency) run: | + set -uo pipefail echo "๐Ÿ“ฆ Installing Chromium (required by security-tests dependency)..." - npx playwright install --with-deps chromium - EXIT_CODE=$? - echo "โœ… Install command completed (exit code: $EXIT_CODE)" - exit "$EXIT_CODE" + for attempt in 1 2 3; do + timeout 10m npx playwright install --with-deps chromium && break + if [ "$attempt" -lt 3 ]; then + echo "Attempt ${attempt}/3 failed; retrying in 15s..." >&2 + echo "Cleaning up any orphaned apt/dpkg processes and stale locks..." >&2 + sudo pkill -9 -f '(^|/)(apt|apt-get|dpkg)( |$)' 2>/dev/null || true + sudo rm -f /var/lib/dpkg/lock-frontend /var/lib/dpkg/lock /var/cache/apt/archives/lock 2>/dev/null || true + sudo dpkg --configure -a > /dev/null 2>&1 || true + sleep 15 + else + echo "ERROR: Playwright chromium install failed after 3 attempts" >&2 + exit 1 + fi + done - name: Install Playwright Firefox run: | + set -uo pipefail echo "๐Ÿ“ฆ Installing Firefox..." - npx playwright install --with-deps firefox - EXIT_CODE=$? - echo "โœ… Install command completed (exit code: $EXIT_CODE)" - exit "$EXIT_CODE" + for attempt in 1 2 3; do + timeout 10m npx playwright install --with-deps firefox && break + if [ "$attempt" -lt 3 ]; then + echo "Attempt ${attempt}/3 failed; retrying in 15s..." >&2 + echo "Cleaning up any orphaned apt/dpkg processes and stale locks..." >&2 + sudo pkill -9 -f '(^|/)(apt|apt-get|dpkg)( |$)' 2>/dev/null || true + sudo rm -f /var/lib/dpkg/lock-frontend /var/lib/dpkg/lock /var/cache/apt/archives/lock 2>/dev/null || true + sudo dpkg --configure -a > /dev/null 2>&1 || true + sleep 15 + else + echo "ERROR: Playwright firefox install failed after 3 attempts" >&2 + exit 1 + fi + done - name: Run Firefox Security Enforcement Tests run: | @@ -804,21 +853,51 @@ jobs: - name: Install dependencies run: npm ci --ignore-scripts + - name: Harden apt for flaky CI mirrors + run: | + sudo tee /etc/apt/apt.conf.d/99-charon-ci-reliability > /dev/null <<'APTCONF' + Acquire::Retries "3"; + Acquire::http::Timeout "15"; + Acquire::https::Timeout "15"; + APTCONF + - name: Install Playwright Chromium (required by security-tests dependency) run: | + set -uo pipefail echo "๐Ÿ“ฆ Installing Chromium (required by security-tests dependency)..." - npx playwright install --with-deps chromium - EXIT_CODE=$? - echo "โœ… Install command completed (exit code: $EXIT_CODE)" - exit "$EXIT_CODE" + for attempt in 1 2 3; do + timeout 10m npx playwright install --with-deps chromium && break + if [ "$attempt" -lt 3 ]; then + echo "Attempt ${attempt}/3 failed; retrying in 15s..." >&2 + echo "Cleaning up any orphaned apt/dpkg processes and stale locks..." >&2 + sudo pkill -9 -f '(^|/)(apt|apt-get|dpkg)( |$)' 2>/dev/null || true + sudo rm -f /var/lib/dpkg/lock-frontend /var/lib/dpkg/lock /var/cache/apt/archives/lock 2>/dev/null || true + sudo dpkg --configure -a > /dev/null 2>&1 || true + sleep 15 + else + echo "ERROR: Playwright chromium install failed after 3 attempts" >&2 + exit 1 + fi + done - name: Install Playwright WebKit run: | + set -uo pipefail echo "๐Ÿ“ฆ Installing WebKit..." - npx playwright install --with-deps webkit - EXIT_CODE=$? - echo "โœ… Install command completed (exit code: $EXIT_CODE)" - exit "$EXIT_CODE" + for attempt in 1 2 3; do + timeout 10m npx playwright install --with-deps webkit && break + if [ "$attempt" -lt 3 ]; then + echo "Attempt ${attempt}/3 failed; retrying in 15s..." >&2 + echo "Cleaning up any orphaned apt/dpkg processes and stale locks..." >&2 + sudo pkill -9 -f '(^|/)(apt|apt-get|dpkg)( |$)' 2>/dev/null || true + sudo rm -f /var/lib/dpkg/lock-frontend /var/lib/dpkg/lock /var/cache/apt/archives/lock 2>/dev/null || true + sudo dpkg --configure -a > /dev/null 2>&1 || true + sleep 15 + else + echo "ERROR: Playwright webkit install failed after 3 attempts" >&2 + exit 1 + fi + done - name: Run WebKit Security Enforcement Tests run: | @@ -1077,13 +1156,32 @@ jobs: - name: Install dependencies run: npm ci --ignore-scripts + - name: Harden apt for flaky CI mirrors + run: | + sudo tee /etc/apt/apt.conf.d/99-charon-ci-reliability > /dev/null <<'APTCONF' + Acquire::Retries "3"; + Acquire::http::Timeout "15"; + Acquire::https::Timeout "15"; + APTCONF + - name: Install Playwright Chromium run: | + set -uo pipefail echo "๐Ÿ“ฆ Installing Chromium..." - npx playwright install --with-deps chromium - EXIT_CODE=$? - echo "โœ… Install command completed (exit code: $EXIT_CODE)" - exit "$EXIT_CODE" + for attempt in 1 2 3; do + timeout 10m npx playwright install --with-deps chromium && break + if [ "$attempt" -lt 3 ]; then + echo "Attempt ${attempt}/3 failed; retrying in 15s..." >&2 + echo "Cleaning up any orphaned apt/dpkg processes and stale locks..." >&2 + sudo pkill -9 -f '(^|/)(apt|apt-get|dpkg)( |$)' 2>/dev/null || true + sudo rm -f /var/lib/dpkg/lock-frontend /var/lib/dpkg/lock /var/cache/apt/archives/lock 2>/dev/null || true + sudo dpkg --configure -a > /dev/null 2>&1 || true + sleep 15 + else + echo "ERROR: Playwright chromium install failed after 3 attempts" >&2 + exit 1 + fi + done - name: Run Chromium Non-Security Tests (Shard ${{ matrix.shard }}/${{ matrix.total-shards }}) run: | @@ -1315,21 +1413,51 @@ jobs: - name: Install dependencies run: npm ci --ignore-scripts + - name: Harden apt for flaky CI mirrors + run: | + sudo tee /etc/apt/apt.conf.d/99-charon-ci-reliability > /dev/null <<'APTCONF' + Acquire::Retries "3"; + Acquire::http::Timeout "15"; + Acquire::https::Timeout "15"; + APTCONF + - name: Install Playwright Chromium (required by security-tests dependency) run: | + set -uo pipefail echo "๐Ÿ“ฆ Installing Chromium (required by security-tests dependency)..." - npx playwright install --with-deps chromium - EXIT_CODE=$? - echo "โœ… Install command completed (exit code: $EXIT_CODE)" - exit "$EXIT_CODE" + for attempt in 1 2 3; do + timeout 10m npx playwright install --with-deps chromium && break + if [ "$attempt" -lt 3 ]; then + echo "Attempt ${attempt}/3 failed; retrying in 15s..." >&2 + echo "Cleaning up any orphaned apt/dpkg processes and stale locks..." >&2 + sudo pkill -9 -f '(^|/)(apt|apt-get|dpkg)( |$)' 2>/dev/null || true + sudo rm -f /var/lib/dpkg/lock-frontend /var/lib/dpkg/lock /var/cache/apt/archives/lock 2>/dev/null || true + sudo dpkg --configure -a > /dev/null 2>&1 || true + sleep 15 + else + echo "ERROR: Playwright chromium install failed after 3 attempts" >&2 + exit 1 + fi + done - name: Install Playwright Firefox run: | + set -uo pipefail echo "๐Ÿ“ฆ Installing Firefox..." - npx playwright install --with-deps firefox - EXIT_CODE=$? - echo "โœ… Install command completed (exit code: $EXIT_CODE)" - exit "$EXIT_CODE" + for attempt in 1 2 3; do + timeout 10m npx playwright install --with-deps firefox && break + if [ "$attempt" -lt 3 ]; then + echo "Attempt ${attempt}/3 failed; retrying in 15s..." >&2 + echo "Cleaning up any orphaned apt/dpkg processes and stale locks..." >&2 + sudo pkill -9 -f '(^|/)(apt|apt-get|dpkg)( |$)' 2>/dev/null || true + sudo rm -f /var/lib/dpkg/lock-frontend /var/lib/dpkg/lock /var/cache/apt/archives/lock 2>/dev/null || true + sudo dpkg --configure -a > /dev/null 2>&1 || true + sleep 15 + else + echo "ERROR: Playwright firefox install failed after 3 attempts" >&2 + exit 1 + fi + done - name: Run Firefox Non-Security Tests (Shard ${{ matrix.shard }}/${{ matrix.total-shards }}) run: | @@ -1561,21 +1689,51 @@ jobs: - name: Install dependencies run: npm ci --ignore-scripts + - name: Harden apt for flaky CI mirrors + run: | + sudo tee /etc/apt/apt.conf.d/99-charon-ci-reliability > /dev/null <<'APTCONF' + Acquire::Retries "3"; + Acquire::http::Timeout "15"; + Acquire::https::Timeout "15"; + APTCONF + - name: Install Playwright Chromium (required by security-tests dependency) run: | + set -uo pipefail echo "๐Ÿ“ฆ Installing Chromium (required by security-tests dependency)..." - npx playwright install --with-deps chromium - EXIT_CODE=$? - echo "โœ… Install command completed (exit code: $EXIT_CODE)" - exit "$EXIT_CODE" + for attempt in 1 2 3; do + timeout 10m npx playwright install --with-deps chromium && break + if [ "$attempt" -lt 3 ]; then + echo "Attempt ${attempt}/3 failed; retrying in 15s..." >&2 + echo "Cleaning up any orphaned apt/dpkg processes and stale locks..." >&2 + sudo pkill -9 -f '(^|/)(apt|apt-get|dpkg)( |$)' 2>/dev/null || true + sudo rm -f /var/lib/dpkg/lock-frontend /var/lib/dpkg/lock /var/cache/apt/archives/lock 2>/dev/null || true + sudo dpkg --configure -a > /dev/null 2>&1 || true + sleep 15 + else + echo "ERROR: Playwright chromium install failed after 3 attempts" >&2 + exit 1 + fi + done - name: Install Playwright WebKit run: | + set -uo pipefail echo "๐Ÿ“ฆ Installing WebKit..." - npx playwright install --with-deps webkit - EXIT_CODE=$? - echo "โœ… Install command completed (exit code: $EXIT_CODE)" - exit "$EXIT_CODE" + for attempt in 1 2 3; do + timeout 10m npx playwright install --with-deps webkit && break + if [ "$attempt" -lt 3 ]; then + echo "Attempt ${attempt}/3 failed; retrying in 15s..." >&2 + echo "Cleaning up any orphaned apt/dpkg processes and stale locks..." >&2 + sudo pkill -9 -f '(^|/)(apt|apt-get|dpkg)( |$)' 2>/dev/null || true + sudo rm -f /var/lib/dpkg/lock-frontend /var/lib/dpkg/lock /var/cache/apt/archives/lock 2>/dev/null || true + sudo dpkg --configure -a > /dev/null 2>&1 || true + sleep 15 + else + echo "ERROR: Playwright webkit install failed after 3 attempts" >&2 + exit 1 + fi + done - name: Run WebKit Non-Security Tests (Shard ${{ matrix.shard }}/${{ matrix.total-shards }}) run: | diff --git a/.github/workflows/history-rewrite-tests.yml b/.github/workflows/history-rewrite-tests.yml index 0d7e5ca84..7be5dbde3 100644 --- a/.github/workflows/history-rewrite-tests.yml +++ b/.github/workflows/history-rewrite-tests.yml @@ -4,6 +4,16 @@ on: workflow_run: workflows: ["Docker Build, Publish & Test"] types: [completed] + pull_request: + paths: + - 'scripts/history-rewrite/**' + - '.github/workflows/history-rewrite-tests.yml' + push: + branches: [main, development] + paths: + - 'scripts/history-rewrite/**' + - '.github/workflows/history-rewrite-tests.yml' + workflow_dispatch: {} concurrency: group: ${{ github.workflow }}-${{ github.event_name }}-${{ github.event.workflow_run.head_branch || github.head_ref || github.ref_name }} @@ -15,7 +25,7 @@ permissions: jobs: test: runs-on: ubuntu-latest - if: ${{ github.event.workflow_run.conclusion == 'success' }} + if: ${{ github.event_name != 'workflow_run' || github.event.workflow_run.conclusion == 'success' }} steps: - name: Checkout with full history uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 diff --git a/.github/workflows/propagate-changes.yml b/.github/workflows/propagate-changes.yml index 0632db7f6..c7cd1206e 100644 --- a/.github/workflows/propagate-changes.yml +++ b/.github/workflows/propagate-changes.yml @@ -5,6 +5,19 @@ on: workflows: ["Docker Build, Publish & Test"] types: [completed] branches: [ main, development ] + # NOTE for future contributors: do not add a direct `push:` trigger here as + # a workflow_run-reliability fallback without ALSO updating the job-level + # `if:` below. The CURRENT_BRANCH/CURRENT_SHA env fallbacks (`|| github.ref_name` + # / `|| github.sha`) already resolve correctly on a `push` event, but the + # `if:` only recognizes `workflow_dispatch` or a matching `workflow_run` โ€” + # it has no `github.event_name == 'push'` clause, so a bare `push:` addition + # would silently no-op the job on every push (worse than today: it would + # show as a registered trigger in the Actions UI while never actually + # running). workflow_run is also semantically correct here regardless: this + # job should only propagate a commit Docker Build has already validated as + # successful, which a raw `push` event can't guarantee. The existing + # `workflow_dispatch: {}` below (added in 5245858a) is the intended manual + # fallback if workflow_run ever silently fails to fire again. workflow_dispatch: {} concurrency: diff --git a/.github/workflows/release-goreleaser.yml b/.github/workflows/release-goreleaser.yml deleted file mode 100644 index 05cdc55ce..000000000 --- a/.github/workflows/release-goreleaser.yml +++ /dev/null @@ -1,87 +0,0 @@ -name: Release (GoReleaser) - -on: - push: - tags: - - 'v*' - -concurrency: - group: ${{ github.workflow }}-${{ github.ref }} - cancel-in-progress: false - -env: - GO_VERSION: '1.26.6' - NODE_VERSION: '24.19.0' - GOTOOLCHAIN: local - -permissions: - contents: write - packages: write - -jobs: - goreleaser: - if: ${{ !contains(github.ref_name, '-candidate') && !contains(github.ref_name, '-rc') }} - runs-on: ubuntu-latest - env: - # Use the built-in GITHUB_TOKEN by default for GitHub API operations. - # If you need to provide a PAT with elevated permissions, add a GITHUB_TOKEN secret - # at the repo or organization level and update the env here accordingly. - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - steps: - - name: Checkout - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 - with: - fetch-depth: 0 - - - name: Enforce PR-2 release promotion guard - env: - REPO_VARS_JSON: ${{ toJSON(vars) }} - run: | - PR2_GATE_STATUS="$(printf '%s' "$REPO_VARS_JSON" | jq -r '.CHARON_PR2_GATES_PASSED // "false"')" - if [[ "$PR2_GATE_STATUS" != "true" ]]; then - echo "::error::Releasable tag promotion is blocked until PR-2 security/retirement gates pass." - echo "::error::Set repository variable CHARON_PR2_GATES_PASSED=true only after PR-2 approval." - exit 1 - fi - - - name: Set up Go - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7 - with: - go-version-file: backend/go.mod - - cache-dependency-path: backend/go.sum - - - name: Set up Node.js - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7 - with: - node-version: ${{ env.NODE_VERSION }} - - - name: Build Frontend - working-directory: frontend - run: | - # Inject version into frontend build from tag (if present) - VERSION=${GITHUB_REF#refs/tags/} - echo "VITE_APP_VERSION=${VERSION}" >> "$GITHUB_ENV" - npm ci --ignore-scripts - npm run build - - - name: Install Cross-Compilation Tools (Zig) - # Security: Pinned to full SHA for supply chain security - uses: mlugg/setup-zig@d1434d08867e3ee9daa34448df10607b98908d29 # v2.2.1 - with: - version: 0.13.0 - - # GITHUB_TOKEN is set from GITHUB_TOKEN or CHARON_TOKEN (fallback), defaulting to GITHUB_TOKEN - - - name: Generate Changelog Data - run: bash scripts/generate-changelog.sh - - - name: Run GoReleaser - uses: goreleaser/goreleaser-action@f06c13b6b1a9625abc9e6e439d9c05a8f2190e94 # v7 - with: - distribution: goreleaser - version: '~> v2.5' - args: release --clean - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - # CGO settings are handled in .goreleaser.yaml via Zig diff --git a/.github/workflows/release-please.yml b/.github/workflows/release-please.yml new file mode 100644 index 000000000..3524195a7 --- /dev/null +++ b/.github/workflows/release-please.yml @@ -0,0 +1,19 @@ +name: release-please + +on: + push: + branches: [main] + +permissions: + contents: write + pull-requests: write + +jobs: + release-please: + runs-on: ubuntu-latest + steps: + - uses: googleapis/release-please-action@45996ed1f6d02564a971a2fa1b5860e934307cf7 # v5 + with: + config-file: release-please-config.json + manifest-file: .release-please-manifest.json + target-branch: main diff --git a/.gitignore b/.gitignore index 77fe2c3a4..58ec00b71 100644 --- a/.gitignore +++ b/.gitignore @@ -161,11 +161,6 @@ backend/data/caddy/ # ----------------------------------------------------------------------------- docker-compose.override.yml -# ----------------------------------------------------------------------------- -# GoReleaser -# ----------------------------------------------------------------------------- -dist/ - # ----------------------------------------------------------------------------- # Testing & Coverage # ----------------------------------------------------------------------------- diff --git a/.goreleaser.yaml b/.goreleaser.yaml deleted file mode 100644 index 44a0cea34..000000000 --- a/.goreleaser.yaml +++ /dev/null @@ -1,81 +0,0 @@ -version: 2 - -# NOTE: Charon uses a Docker-only deployment model. -# This GoReleaser configuration is used exclusively for changelog generation. -# The builds, archives, and nfpms sections below are kept for potential -# future use but are not currently utilized in the release workflow. -# All distribution happens via Docker images: -# - Docker Hub: docker pull wikid82/charon:latest -# - GHCR: docker pull ghcr.io/wikid82/charon:latest - -project_name: charon - -builds: - - id: linux - dir: backend - main: ./cmd/api - binary: charon - env: - - CGO_ENABLED=0 - goos: - - linux - goarch: - - amd64 - - arm64 - ldflags: - - -s -w - - -X github.com/Wikid82/charon/backend/internal/version.Version={{.Version}} - - -X github.com/Wikid82/charon/backend/internal/version.GitCommit={{.Commit}} - - -X github.com/Wikid82/charon/backend/internal/version.BuildTime={{.Date}} - -archives: - - formats: - - tar.gz - id: linux - ids: - - linux - name_template: >- - {{ .ProjectName }}_ - {{- .Version }}_ - {{- .Os }}_ - {{- .Arch }} - files: - - LICENSE - - README.md - -nfpms: - - id: packages - ids: - - linux - package_name: charon - vendor: Charon - homepage: https://github.com/Wikid82/charon - maintainer: Wikid82 - description: "Charon - A powerful reverse proxy manager" - license: MIT - formats: - - deb - - rpm - contents: - - src: ./backend/data/ - dst: /var/lib/charon/data/ - type: dir - - src: ./frontend/dist/ - dst: /usr/share/charon/frontend/ - type: dir - dependencies: - - libc6 - - ca-certificates - -checksum: - name_template: 'checksums.txt' - -snapshot: - version_template: "{{ .Tag }}-next" - -changelog: - sort: asc - filters: - exclude: - - '^docs:' - - '^test:' diff --git a/.release-please-manifest.json b/.release-please-manifest.json new file mode 100644 index 000000000..18ec56b0d --- /dev/null +++ b/.release-please-manifest.json @@ -0,0 +1,3 @@ +{ + ".": "0.37.1" +} diff --git a/.version b/.version deleted file mode 100644 index 0a8bf80d6..000000000 --- a/.version +++ /dev/null @@ -1 +0,0 @@ -v0.27.0 diff --git a/.vscode/tasks.json b/.vscode/tasks.json index 890fd804a..06f3f5553 100644 --- a/.vscode/tasks.json +++ b/.vscode/tasks.json @@ -690,13 +690,6 @@ "group": "test", "problemMatcher": [] }, - { - "label": "Utility: Check Version Match Tag", - "type": "shell", - "command": ".github/skills/scripts/skill-runner.sh utility-version-check", - "group": "none", - "problemMatcher": [] - }, { "label": "Utility: Clear Go Cache", "type": "shell", diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index f77c29f1d..e9b8bd408 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -1463,20 +1463,45 @@ go test ./integration/... ### Release Workflow -**Automated Release (GitHub Actions):** - -1. **Trigger:** Push tag `v1.2.0` -2. **Build:** Multi-platform Docker images -3. **Test:** Run E2E tests against built image -4. **Security:** Scan for vulnerabilities (block if Critical/High) -5. **SBOM:** Generate Software Bill of Materials (Syft) -6. **Sign:** Cryptographic signature with Cosign -7. **Provenance:** Generate SLSA provenance attestation -8. **Publish:** Push to Docker Hub and GHCR -9. **Release Notes:** Generate changelog from commits -10. **Notify:** Send release notification (Discord, email) - -**In-app changelog data:** Separately from step 9's GitHub release notes, `scripts/generate-changelog.sh` runs during the same `release-goreleaser.yml` workflow to parse conventional-commit history into `backend/internal/changelog/data/changelog.json`, which is `//go:embed`-ed into the binary and powers the in-app "What's New" modal (see "Changelog Subsystem" above). It writes a different file than step 9 and does not affect the GitHub release notes. +Versioning and release publication are handled by +[`googleapis/release-please-action`](https://github.com/googleapis/release-please-action) +(`.github/workflows/release-please.yml`), independently of the Docker +image build pipeline described below. See `VERSION.md` for the full +user-facing walkthrough; summarized here: + +1. **Trigger:** Push to `main` (any commit) +2. **`release-please.yml` runs** (independently of the Docker build): + computes releasable versions from Conventional Commit history and + opens/updates a standing `chore(main): release X.Y.Z` pull request. + No release ships yet at this point. +3. **A human merges that release PR** โ€” this is the only step that + actually cuts a release. release-please then tags the merge commit + `vX.Y.Z` (bare, no component prefix) and creates the GitHub Release. +4. **`orthrus-build.yml` fires on the new `v*` tag** and publishes + semver-tagged Orthrus agent images โ€” the one workflow with a real, + live dependency on the tag release-please creates. + +**Automated Docker Image Build (GitHub Actions, `docker-build.yml`):** + +Triggered independently by every push to `main`/`development` (branch +push, not the release tag): + +1. **Build:** Multi-platform Docker images +2. **Test:** Run E2E tests against built image +3. **Security:** Scan for vulnerabilities (block if Critical/High) +4. **SBOM:** Generate Software Bill of Materials (Syft) +5. **Sign:** Cryptographic signature with Cosign +6. **Provenance:** Generate SLSA provenance attestation +7. **Publish:** Push to Docker Hub and GHCR + +**In-app changelog data:** `scripts/generate-changelog.sh` runs during +`nightly-build.yml` (its one remaining real caller) to parse +conventional-commit history into `backend/internal/changelog/data/changelog.json`, +which is `//go:embed`-ed into the binary and powers the in-app "What's +New" modal (see "Changelog Subsystem" above). It depends only on real +`v*` tags existing in git history โ€” not on release-please's PR/Release +mechanism directly โ€” so it keeps working unchanged by this migration +as long as release-please continues creating bare `v*` tags. **Mandatory rollout gates (sign-off block):** diff --git a/CLAUDE.md b/CLAUDE.md index 151a71ab4..f4e4d84f5 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -258,4 +258,3 @@ Available skills (see `.github/skills/*.SKILL.md` for full docs): | `security-scan-gorm` | Run GORM security scan | | `security-scan-go-vuln` | Run Go vulnerability check | | `integration-test-all` | Run all integration tests | -| `utility-version-check` | Check tool versions | diff --git a/VERSION.md b/VERSION.md index 311c0601c..56dfbc966 100644 --- a/VERSION.md +++ b/VERSION.md @@ -19,47 +19,49 @@ Example: `0.1.0-alpha`, `1.0.0-beta.1`, `2.0.0-rc.2` ## Creating a Release -### Canonical Release Process (Tag-Derived CI) - -1. **Create and push a release tag**: - - ```bash - - git tag -a v1.0.0 -m "Release v1.0.0" - git push origin v1.0.0 - - ``` - -2. **GitHub Actions automatically**: - - Runs release workflow from the pushed tag (`.github/workflows/release-goreleaser.yml`) - - Builds and publishes release artifacts/images through CI (`.github/workflows/docker-build.yml`) - - Creates/updates GitHub Release metadata - -3. **Container tags are published**: +### Canonical Release Process (release-please) + +Charon uses [`googleapis/release-please-action`](https://github.com/googleapis/release-please-action) +to compute versions, maintain a standing release PR, and cut tags + +GitHub Releases from Conventional Commit history. There is no manual +tagging step and no in-repo version manifest to hand-edit โ€” `.release-please-manifest.json` +is the single source of truth for "what version are we at," kept in +sync by release-please itself. + +1. **Commits land on `main`**: every push to `main` runs `.github/workflows/release-please.yml`, + which walks Conventional Commits since the last release and opens + or updates a standing `chore(main): release X.Y.Z` pull request. + If nothing releasable (`feat:`/`fix:`) has landed since the last + release, no PR is opened or updated. +2. **A human merges the release PR**: merging it is what actually + ships a release โ€” nothing goes out automatically before that. +3. **On merge, release-please**: + - Tags the merge commit `vX.Y.Z` (bare, no `charon-` prefix โ€” + pinned via `include-component-in-tag: false` in + `release-please-config.json`) + - Creates the GitHub Release for that tag +4. **Downstream, independently of release-please**: + - `.github/workflows/orthrus-build.yml` triggers on the new `v*` + tag and publishes semver-tagged Orthrus agent images + - The next `nightly-build.yml` run picks up the new tag via + `scripts/generate-changelog.sh`'s `git tag -l 'v*'` scan and + regenerates the in-app "What's New" changelog data + +5. **Container tags are published** (via `.github/workflows/docker-build.yml`, + triggered on the branch push, not the tag): - `v1.0.0` (exact version) - `1.0` (minor version) - `1` (major version) - `latest` (for non-prerelease on main branch) -### Legacy/Optional `.version` Path - -The `.version` file is optional and not the canonical release trigger. - -Use it only when you need local/version-file parity checks: - -1. **Set `.version` locally (optional)**: - - ```bash - echo "1.0.0" > .version - ``` - -1. **Validate `.version` matches the latest tag**: - - ```bash - - bash scripts/check-version-match-tag.sh +**Do not manually push `v*` tags.** A manually-created tag desyncs +`.release-please-manifest.json` (which release-please treats as its +source of truth, not live tag state) from the repo's real tag +history โ€” let release-please create every release tag going forward. - ``` +`release-please-config.json` sets `skip-changelog: true`, so this +hand-curated `CHANGELOG.md` is never touched by release-please's PRs +or Release generation. ### Deterministic Rollout Verification Gates (Mandatory) @@ -71,7 +73,6 @@ Enforcement points: - Release sign-off checklist/process (mandatory): All gates below remain required for release sign-off. - CI-supported checks (current): `.github/workflows/docker-build.yml` and `.github/workflows/supply-chain-verify.yml` enforce the subset currently implemented in workflows. - Manual validation required until CI parity: Validate any not-yet-implemented workflow gates via VS Code tasks `Security: Full Supply Chain Audit`, `Security: Verify SBOM`, `Security: Generate SLSA Provenance`, and `Security: Sign with Cosign`. -- Optional version-file parity check: `Utility: Check Version Match Tag` (script: `scripts/check-version-match-tag.sh`). - [ ] **Digest freshness/parity:** Capture pre-push and post-push index digests for the target tag in GHCR and Docker Hub, confirm expected freshness, @@ -251,7 +252,7 @@ docker build \ ## Changelog Generation -The release workflow automatically generates changelogs from commit messages. Use conventional commit format: +release-please's standing release PR body is generated from commit messages. Use conventional commit format: - `feat:` New features - `fix:` Bug fixes @@ -273,7 +274,5 @@ git commit -m "fix: correct proxy timeout handling" - CI derives the release `Version` from the Git tag (e.g., `v1.2.3`) and embeds this value into the backend binary via Go ldflags; frontend reads the version from the backend's API. This avoids automatic commits to `main`. -- The `.version` file is optional. If present, use the `scripts/check-version-match-tag.sh` script - or the included pre-commit hook to validate that `.version` matches the latest Git tag. -- CI will still generate changelogs automatically using the release-drafter workflow and create - GitHub Releases when tags are pushed. +- release-please creates the tag and the GitHub Release together when its standing release PR is + merged โ€” see "Canonical Release Process (release-please)" above. diff --git a/docs/plans/archive/2026-08-14_semgrep-ci-security-scan-plan.md b/docs/plans/archive/2026-08-14_semgrep-ci-security-scan-plan.md new file mode 100644 index 000000000..7aa3ef37f --- /dev/null +++ b/docs/plans/archive/2026-08-14_semgrep-ci-security-scan-plan.md @@ -0,0 +1,505 @@ +# Semgrep CI Security Scan โ€” Implementation Plan + +Status: Planning complete, revised per Supervisor review (round 2). +Owner for implementation: **devops** agent (CI/CD-only change; no backend-dev or frontend-dev involvement โ€” no application code, no models, no UI). +Branch: current working branch (`development`) per `CLAUDE.md` โ€” no worktree. +PR base branch: `development` (standard feature PR convention observed in this repo; `main` only receives weekly `nightly` promotion merges). + +--- + +## 1. Introduction + +### 1.1 Objective + +Add an independent Semgrep SAST scan to GitHub Actions CI that reproduces, byte-for-byte, the same scan behavior developers already run locally via `scripts/pre-commit-hooks/semgrep-scan.sh` (wired through `lefthook.yml`'s `pre-commit`/`pre-push`/`security-full` targets and `make security-local`). Today, Semgrep coverage exists **only** on the developer's machine โ€” CI has zero Semgrep footprint (confirmed: no match in `.github/workflows/`, `.github/renovate.json`, or any Dockerfile/compose file). This means: + +- A developer who bypasses lefthook (`--no-verify`, an emergency hotfix, a machine without semgrep installed) ships code with no Semgrep signal at all. +- Nobody re-verifies the "clean" local Semgrep run against a controlled, versioned environment โ€” the local binary's version, ruleset revision, and installed registry rules can silently drift per-developer with no CI backstop. + +This plan adds CI-side Semgrep coverage that is authoritative (independent of the developer's local environment) while staying faithful to the existing local invocation. + +### 1.2 Goals + +1. A new CI job runs the **exact same** rule configs, exclusions, and severity/error-gating behavior as `scripts/pre-commit-hooks/semgrep-scan.sh`'s default (no-override) path, scanning the full repo. +2. Semgrep's version is pinned in CI (image tag + digest) โ€” today there is no version pin anywhere in the repo for Semgrep, local or CI. +3. Findings are visible in the GitHub Security tab (SARIF upload), consistent with how CodeQL and Trivy results are surfaced today. +4. A hard-fail gate blocks the PR/branch on ERROR/WARNING findings, mirroring the local script's `--error` behavior โ€” CI is a gate, not just an informational report. +5. `scripts/pre-commit-hooks/semgrep-scan.sh`'s binary/version resolution logic (the `command -v semgrep` check, ยง2.1) is **not touched** โ€” that stays developer-local tooling, per the original brief's explicit scope boundary. The script's rule-config/exclude/severity logic, by contrast, **is** extended with one small, additive, backward-compatible hook (ยง2.7/ยง3.0) so CI can reuse it directly instead of duplicating it โ€” see ยง2.7 for why this is a different constraint than "freeze the whole file," and why the narrower reading is the right one. +6. Documentation (`SECURITY.md` and `ARCHITECTURE.md`) is updated to reflect the new CI coverage. + +### 1.3 Non-goals + +- No change to how the local pre-commit/pre-push semgrep **binary** is discovered, installed, or versioned (the `command -v semgrep` / exit-127 block in `scripts/pre-commit-hooks/semgrep-scan.sh` is untouched). +- No new GitHub Action marketplace dependency requiring npm/JS runtime โ€” Semgrep ships as a self-contained CLI in an official container image, which is used directly. +- No change to `.gitignore`, `.dockerignore`, `.codecov.yml`, or any `Dockerfile` (see ยง2.9 โ€” reviewed explicitly, no changes needed). +- No attempt to unify Trivy's/CodeQL's SARIF-upload plumbing into a shared reusable workflow โ€” out of scope for this feature; each scanner's workflow remains independent, consistent with current repo structure (`codeql.yml`, `security-pr.yml`, `security-weekly-rebuild.yml` are all separate files today). + +--- + +## 2. Research Findings + +### 2.1 Local Semgrep invocation (`scripts/pre-commit-hooks/semgrep-scan.sh`) + +Full script behavior (verified by reading the file): + +- Requires `semgrep` on `PATH`; exits 127 if missing (this resolution logic is untouched by this plan โ€” see ยง1.3). +- Default rule configs (used unless `SEMGREP_CONFIG` env override is set): + ``` + --config p/golang + --config p/javascript + --config p/typescript + --config p/react + --config p/secrets + --config p/dockerfile + ``` +- Targets: staged files if passed as args (lefthook `pre-commit`), else full-repo default `Dockerfile backend frontend/src scripts .github/workflows` (lefthook `security-full` / manual run). +- Exact scan flags (current, pre-change): + ``` + semgrep scan \ + "${SEMGREP_CONFIGS[@]}" \ + --severity ERROR \ + --severity WARNING \ + --error \ + --exclude "frontend/node_modules" \ + --exclude "frontend/coverage" \ + --exclude "frontend/dist" \ + --exclude-rule "go.secrets.gorm.gorm-empty-password.gorm-empty-password" \ + "${TARGETS[@]}" + ``` +- `--error` makes semgrep exit non-zero if any ERROR/WARNING-severity finding exists โ€” this is the local "hard fail" behavior CI must reproduce. + +Wiring confirmed in `lefthook.yml`: +- `pre-commit.semgrep` (line ~113-116): glob-scoped, staged-files-only, blocking. +- `security-full.semgrep` (line ~137-140, manual stage, `lefthook run security-full`): full-repo, no args โ†’ this is the invocation CI should mirror most closely (full-repo, not staged-file-scoped). +- `Makefile:security-local` additionally runs `SEMGREP_CONFIG=p/golang` as a fast pre-push subset โ€” this is a narrower override path, not the target for CI parity (CI should mirror the **full** default ruleset, matching `security-full`). + +### 2.2 Confirmed: zero Semgrep footprint in CI today + +`grep -rn "semgrep" .github/workflows/ .github/renovate.json` (and Dockerfiles/compose) returns no matches. Semgrep is 100% local-only today. (Note: the repo's Renovate config lives at `.github/renovate.json`, not a root-level `renovate.json` โ€” corrected throughout this plan.) + +### 2.3 Existing CI patterns to mirror + +**`.github/workflows/codeql.yml`** (closest pattern for a source-level SAST tool): +- Triggers: `pull_request`/`push` on `[main, nightly, development]`, `workflow_dispatch`, weekly `schedule` cron (`0 3 * * 1`, Mondays 03:00 UTC). +- `concurrency` group keyed on workflow/event/ref, `cancel-in-progress: true`. +- `permissions:` declared at **both** the workflow (top) level and again, identically, at job level (`contents: read`, `security-events: write`, `actions: read`, `pull-requests: read`). +- All third-party actions pinned by commit SHA with a `# vX.Y.Z` trailing comment, e.g. `github/codeql-action/init@ff2f1c621b7f889edc0d3c761ac2e6a3f8cdb0dd # v4`. +- Has a **parity guard** step ("Verify CodeQL parity guard" โ†’ `scripts/ci/check-codeql-parity.sh`) that runs *before* the scan, structurally checking that local pre-commit scripts, `.vscode/tasks.json`, and the CI workflow all agree on query-suite pinning and trigger branches โ€” added specifically because CodeQL's local/CI ruleset previously drifted silently (see `check-codeql-parity.sh` comment referencing a real incident: a suppressed finding rode through PR #1216 unnoticed because local and CI independently duplicated blocking logic). +- Emits results to `$GITHUB_STEP_SUMMARY`, then a **separate, later step** does the actual hard-fail (`Fail on High-Severity Findings`) โ€” reporting and gating are deliberately split into two steps so the summary always renders even on failure. + +**`.github/workflows/security-pr.yml`** (closest pattern for "pinned scanner โ†’ SARIF upload โ†’ hard-fail gate"): +- Runs Trivy via `aquasecurity/trivy-action@ed142fd0673e97e23eac54620cfb913e5ce36c25` (SHA-pinned, `# aquasecurity/trivy-action 0.36.0` comment), with an explicit `version: 'v0.73.0'` input additionally pinning the *scanner* version, not just the action wrapper. +- Runs the scan **twice**: once with `format: 'sarif'` (`continue-on-error: true`, purely for the Security tab), then again with `format: 'table'` + `exit-code: '1'` (no continue-on-error) as the actual blocking gate. It also has an explicit "Check Trivy SARIF output exists" gating step between the SARIF-producing run and the upload step. This two-pass "report, then gate" split, plus the existence check, is the direct template for Semgrep's SARIF-vs-hard-fail split (ยง3.3). +- SARIF uploaded via `github/codeql-action/upload-sarif@ff2f1c621b7f889edc0d3c761ac2e6a3f8cdb0dd # v4.37.7` (same SHA-pinned action already used elsewhere in this repo for SARIF ingestion โ€” no new third-party dependency needed for the upload step). +- Trigger shape is materially more complex than needed here (`workflow_run` chaining off `docker-build.yml`, PR-number resolution, artifact download) because Trivy scans a **built container image**. Semgrep scans **source**, so it needs none of that โ€” it can trigger directly on `push`/`pull_request` like CodeQL, with no dependency on a prior Docker build. + +### 2.4 Repo-wide pinning convention + +Every third-party action in this repo is pinned to an exact commit SHA with a trailing `# vX.Y.Z` comment โ€” never a floating tag, never `@latest`. This is enforced by convention/review, not currently by a lint rule for actions specifically. Any new job must follow this exactly. + +### 2.5 Semgrep version/mechanism research + +Options considered: + +| Option | Assessment | +|---|---| +| `pip install semgrep==` on `ubuntu-latest` | Works, but reintroduces a Python toolchain dependency into a Go+TS repo purely for CI plumbing (`CLAUDE.md`: "No Python โ€” do not introduce Python scripts or requirements"). While this is arguably a tooling install rather than an authored script, it still pulls in `pip`/Python resolution behavior (version solving, transitive dependency drift) that the repo's own conventions steer away from. Rejected. | +| `semgrep/semgrep-action` (formerly `returntocorp/semgrep-action`) marketplace GitHub Action | Semgrep's own current CI docs no longer lead with this as the primary GitHub Actions pattern; it's a thin wrapper around the same official Docker image. Using it would add an extra layer of indirection (an Action wrapping an image) for no behavioral benefit over using the image directly, and re-pinning *that* action's SHA doesn't pin Semgrep's own version any more precisely than pinning the image does. Rejected in favor of the image directly. | +| Official `semgrep/semgrep` Docker image, used as a job-level `container:`, pinned by exact tag **and** digest | Matches this repo's SHA-pinning strictness (a digest is the container-image equivalent of an action's commit SHA โ€” both are content-addressed, immutable references). Gives the CLI directly, with the identical `semgrep scan ...` invocation used locally โ€” maximizes behavioral parity with `semgrep-scan.sh`. **Selected.** | + +Confirmed via the Semgrep GitHub releases API (`api.github.com/repos/semgrep/semgrep/releases/latest`) and PyPI, current stable version at plan time is **`1.173.0`**. Resolved the corresponding Docker Hub manifest digest for `semgrep/semgrep:1.173.0`: + +``` +sha256:67319956da3dcb58baf5b322899c15458e3963e7018a86aeeb5cd224e69cb77a +``` + +Pinned reference to use in the workflow: + +``` +semgrep/semgrep:1.173.0@sha256:67319956da3dcb58baf5b322899c15458e3963e7018a86aeeb5cd224e69cb77a +``` + +**Note for the implementer (devops agent):** re-resolve this digest at implementation time (`docker buildx imagetools inspect semgrep/semgrep:1.173.0` or the registry API) rather than trusting the value transcribed into this plan verbatim, in case the tag's digest has moved between planning and implementation (Docker Hub does not guarantee a tag's digest is immutable the way a Git SHA is โ€” pinning to the *tag+digest pair as observed at merge time* is the achievable guarantee here, and Renovate, already active in this repo via `.github/renovate.json`, will pick up future digest/tag bumps the same way it tracks other pinned SHAs if configured to watch this image โ€” see ยง3.7 edge case). + +**Known gotcha (Semgrep's own docs, `semgrep.dev/docs/kb/semgrep-ci/using-nonroot-docker-image-with-gha`):** running `semgrep/semgrep` as a job-level `container:` against a `actions/checkout`-produced workspace can hit git's "dubious ownership" safety check because the container user doesn't match the checkout's file ownership. Mitigate with an explicit `git config --global --add safe.directory "$GITHUB_WORKSPACE"` step before invoking `semgrep-scan.sh` (ยง3.3). + +**Correction (Supervisor round 2, required change 1):** the container image reference **cannot** be centralized in a workflow-level `env:` var and referenced as `container.image: ${{ env.SEMGREP_IMAGE }}`. GitHub Actions' documented context-availability rules do not expose the `env` context to `jobs..container` โ€” this is a known, currently-true limitation (not something that needs "verification at implementation time"; treating it as an open question in the prior draft was itself the error). The plan now specifies the pinned string **inlined directly** in `container.image` as the only correct form (ยง3.2) โ€” no `env` indirection. + +### 2.6 Placement decision: new file vs. an existing workflow + +*(Per mid-task correction from Management: decide the best location and justify it, rather than defaulting to a new file. Approved as-is by Supervisor round 2 โ€” no changes in this revision.)* + +Three placements were evaluated: + +| Placement | Verdict | +|---|---| +| **New job added to `codeql.yml`** | Rejected. `codeql.yml`'s entire structure is a `strategy.matrix` over CodeQL *languages* (`go`, `javascript-typescript`), with per-language conditional steps (`if: matrix.language == 'go'`) for Go toolchain setup/build and the CodeQL parity guard. Semgrep is not a CodeQL language variant โ€” it's a different tool with a different container, different config format, and a different (single, non-matrixed) invocation. Bolting it in as a third matrix leg would force awkward `if: matrix.language == 'semgrep'` conditionals across steps that don't apply to it (Autobuild, `codeql-action/init`, Go build verification), degrading the readability of a file whose entire premise is "one job, matrixed by CodeQL language." Also couples Semgrep's schedule/trigger lifecycle to CodeQL's, when they are independent tools that should be able to fail, be disabled, or be re-scheduled independently. | +| **New job added to `security-pr.yml`** | Rejected. That workflow's trigger shape and majority of its steps exist *solely* to solve "how do I scan a Docker image that was already built by a separate upstream workflow" โ€” PR-number resolution from `workflow_run` payloads, artifact download/load fallback logic, container extraction of the `charon` binary, a trust-boundary validation step for the `workflow_run` event. None of that applies to Semgrep, which scans source text directly on `push`/`pull_request` with no dependency on `docker-build.yml` having run first. Adding a source-scanning job to an image-scanning workflow would mean either (a) it inherits triggers/conditions built for image scanning that don't fit it (e.g. `workflow_dispatch` inputs are `pr_number`-shaped, meaningless for a source scan), or (b) it needs its own parallel `if:` conditions bolted onto an already condition-heavy file, adding complexity for no shared benefit โ€” the two jobs would share a file but no actual logic. | +| **New file: `.github/workflows/semgrep.yml`** | **Selected.** Semgrep is source-level SAST, triggered directly on `push`/`pull_request`/`schedule`/`workflow_dispatch` โ€” structurally identical in trigger shape to `codeql.yml`, but a distinct tool with its own container, config, and failure/gating semantics. This also matches the repo's existing convention of **one file per scanner**: `codeql.yml` (CodeQL), `security-pr.yml` (Trivy on PR images), `security-weekly-rebuild.yml` (Trivy weekly full scan) are already separate files rather than merged into one "security" workflow, even though they're conceptually related. A dedicated `semgrep.yml` continues that pattern: each scanner is independently triggerable, independently disable-able, and independently readable, at the cost of one more file โ€” a cost the repo has already accepted three times over for its other scanners. | + +### 2.7 "Freeze the whole script" reconsidered โ€” design revision (Supervisor round 2, required change 3) + +**The original brief's non-goal, re-read precisely:** *"Do NOT touch `scripts/pre-commit-hooks/semgrep-scan.sh`'s binary/version resolution logic itself โ€” that's explicitly out of scope, reserved for the user's own local tooling."* This is a constraint about **binary/version discovery** (the `command -v semgrep` / exit-127 block, ยง2.1) โ€” not a blanket freeze on every line of the file. The first draft of this plan over-read it into "never touch this file at all," which forced: + +- A second, hand-written `semgrep scan ...` invocation inline in the workflow YAML, duplicating all six `--config` flags, all three `--exclude` flags, and the `--exclude-rule` value. +- A `check-semgrep-parity.sh` script whose primary job was detecting drift between that duplicated invocation and the real script. +- Pressure to extract shared assertion helpers out of `check-codeql-parity.sh` mainly to support that parity script's config-matching checks. + +That is real, avoidable complexity, not an inherent requirement. **Revised design (adopted โ€” option (a) from Supervisor's feedback):** add one small, additive, backward-compatible hook to `semgrep-scan.sh` itself, leaving the binary/version-resolution logic (the actual thing the non-goal protects) completely untouched: + +```bash +# Existing lines (SEMGREP_CONFIGS / TARGETS construction) unchanged above this point. + +if [ -n "${SEMGREP_SARIF_OUTPUT:-}" ]; then + OUTPUT_FLAGS=(--sarif --output "${SEMGREP_SARIF_OUTPUT}") +else + OUTPUT_FLAGS=(--error) +fi + +semgrep scan \ + "${SEMGREP_CONFIGS[@]}" \ + --severity ERROR \ + --severity WARNING \ + "${OUTPUT_FLAGS[@]}" \ + --exclude "frontend/node_modules" \ + --exclude "frontend/coverage" \ + --exclude "frontend/dist" \ + --exclude-rule "go.secrets.gorm.gorm-empty-password.gorm-empty-password" \ + "${TARGETS[@]}" +``` + +Behavior: +- **`SEMGREP_SARIF_OUTPUT` unset (every existing call site โ€” `pre-commit`, `pre-push`/`security-full`, `make security-local`):** `OUTPUT_FLAGS=(--error)` โ€” byte-identical to today's behavior. Zero change for any existing developer workflow. +- **`SEMGREP_SARIF_OUTPUT=` set (new โ€” CI only):** swaps `--error` for `--sarif --output `, while every `--config`, `--exclude`, and `--exclude-rule` argument stays exactly as-is, sourced from exactly one place. + +This lets CI invoke the **same script** for both the SARIF-producing pass and the hard-fail gate pass (ยง3.3 steps 5 and 8), varying only an env var. Consequences: + +- The duplicated `--config`/`--exclude` list in the workflow YAML is **eliminated entirely** โ€” there is now exactly one place (`semgrep-scan.sh`) that defines what gets scanned, for both local and CI, for both the reporting pass and the gating pass. +- `check-semgrep-parity.sh` shrinks correspondingly (ยง3.4) โ€” it no longer needs to compare two independent config lists (nothing to compare; there's only one). It still has a real, narrower job: confirming the additive hook isn't silently removed, confirming the workflow actually delegates to the script for both passes (rather than a future edit reintroducing an inline duplicate), and confirming the image pin and trigger branches stay correct. This is a smaller, more clearly justified guard than the original draft's. +- The pressure to extract `scripts/ci/lib/workflow-yaml-asserts.sh` out of `check-codeql-parity.sh` is now a plain, optional DRY nicety (the branch-check helper is still needed by both scripts) rather than something load-bearing for the config-parity story โ€” see ยง3.5. + +**Why not stop here and also drop the parity guard entirely?** Because two failure-independent invariants remain worth checking even with zero config duplication: (1) that the additive `SEMGREP_SARIF_OUTPUT` hook stays present in the script (a future refactor of `semgrep-scan.sh` could drop it without realizing CI depends on it), and (2) that the workflow keeps *delegating* to the script for both passes rather than a future edit reintroducing an inline `semgrep scan` call (e.g. someone "simplifying" the SARIF step by hand and accidentally dropping an `--exclude`). Both are cheap, structural, grep-level checks โ€” proportionate, not over-engineering, and much smaller than the original draft's guard (ยง3.4). + +This section supersedes the original ยง2.7 ("Parity guard: warranted, and why") from the first draft. + +### 2.8 Documentation review + +- **`SECURITY.md`** (`## Security Audits & Scanning` โ†’ `### Automated Scanning` table, lines 992-1020): lists Trivy, CodeQL, govulncheck, golangci-lint (gosec), npm audit, and a `### Scanning Workflows` subsection describing each workflow file's purpose (`docker-build.yml`, `supply-chain-verify.yml`, `security-weekly-rebuild.yml`, PR-specific scanning). **This is the primary file to update** โ€” add a `Semgrep` row to the table and a new `**Semgrep SAST Scan**` paragraph under `### Scanning Workflows` describing `.github/workflows/semgrep.yml`. +- **`docs/security.md`**: verified by full-text search (`codeql|trivy|scan|pipeline`, no matches) โ€” this file is entirely about the Cerberus runtime security feature (CrowdSec/WAF/access lists), unrelated to the CI/SAST scanning pipeline. **No change needed here.** +- **`ARCHITECTURE.md` (Supervisor round 2, required change 2 โ€” added to scope):** `CLAUDE.md` requires `ARCHITECTURE.md` updates for changes touching security architecture, and this file already documents the CI security-scanning stack in three places that must be kept current: + - Line 166, tech-stack table: `| **Security Scanning** | Trivy + Grype | Latest | Vulnerability detection |` โ€” append Semgrep, e.g. `Trivy + Grype + Semgrep`. + - Line 1376, CI Jobs list: `3. **Security:** Trivy, CodeQL, Grype, Govulncheck` โ€” append `, Semgrep`. + - Lines 1498-1501, "Container Scanning" components list (`Trivy: ...`, `Grype: ...`, `CodeQL: ...`) โ€” add a fourth line, `Semgrep: Static analysis for security anti-patterns (Go, JS/TS, React, secrets, Dockerfile)`, consistent with the existing one-line-per-tool style. + - This is now part of Commit 3's scope (ยง6) and Acceptance Criteria (ยง5), alongside `SECURITY.md`. + +### 2.9 Ignore-file / build-file review (explicit confirmation per `CLAUDE.md`) + +- **`.gitignore`**: already contains a blanket `*.sarif` ignore (line 189) with a narrow `!scripts/security/testdata/*.sarif` carve-out (line 190). A new `semgrep-results.sarif` file in the repo root matches the existing wildcard โ€” **no change needed**. +- **`.dockerignore`**: already excludes `*.sarif` (line 179) โ€” irrelevant anyway, since this is a CI-only workflow change with no Docker image content change โ€” **no change needed**. +- **`.codecov.yml`**: workflow-only YAML change, produces no coverage-relevant files โ€” **no change needed**. +- **Any `Dockerfile`**: not touched; Semgrep runs in its own CI container, never inside the Charon application image โ€” **no change needed**. + +(Approved as-is by Supervisor round 2 โ€” no changes in this revision.) + +### 2.10 Commit scope: `feat:` vs `feat(security):` + +Per `CLAUDE.md`, `feat:`/`fix:`/`perf:` trigger Docker builds; `chore:` skips them, and `feat(security):`/`fix(security):` is reserved for "genuinely security-relevant... real vulnerability fixes, new protective mechanisms." **Decision:** the workflow-adding commit (Commit 2, ยง6) qualifies as a **new protective mechanism** โ€” it is, definitionally, new automated vulnerability/anti-pattern detection gating merges โ€” so it uses `feat(security):`, not plain `feat:`. Commit 1 (the additive `semgrep-scan.sh` hook + parity guard) also touches genuine security tooling directly and is scoped `feat(security):` for the same reason. Commit 3 (docs) stays `docs:`, matching repo convention for documentation-only changes regardless of what they document. Per `CLAUDE.md`'s vagueness requirement for `(security)` subjects, none of these commit subjects name a vulnerability class or attack vector โ€” they describe the category ("add CI security scanning coverage") only, which is appropriate here since this isn't a vulnerability fix in the first place, just extra coverage. + +**Nuance retained from the original draft:** this change touches zero Docker-build-relevant paths (no `Dockerfile`, no backend/frontend source), so the triggered Docker build (a side effect of `feat`/`feat(security)` prefixes repo-wide) is harmless but expected โ€” not a sign something is wrong with a "just workflow files + one shell script" PR. + +--- + +## 3. Technical Specifications + +### 3.0 Change to `scripts/pre-commit-hooks/semgrep-scan.sh` (additive, in scope per ยง2.7) + +**File:** `scripts/pre-commit-hooks/semgrep-scan.sh` +**Change:** insert the `OUTPUT_FLAGS` branch (ยง2.7) immediately before the existing `semgrep scan \` invocation, and replace the invocation's `--error` line with `"${OUTPUT_FLAGS[@]}"`. No other line in the file changes โ€” the `command -v semgrep` check, the `SEMGREP_CONFIG` override branch, and the `TARGETS` construction are byte-identical to today. +**Backward compatibility:** every existing call site (`lefthook.yml`'s `pre-commit.semgrep`, `security-full.semgrep`, `Makefile`'s `security-local`) never sets `SEMGREP_SARIF_OUTPUT`, so `OUTPUT_FLAGS=(--error)` unconditionally for all of them โ€” identical exit-code and output behavior to the pre-change script. +**New behavior (CI-only):** `SEMGREP_SARIF_OUTPUT= bash scripts/pre-commit-hooks/semgrep-scan.sh [targets...]` scans with the same configs/exclusions but emits SARIF to `` instead of hard-failing on findings. + +### 3.1 New file: `.github/workflows/semgrep.yml` + +No API/DB/frontend surface โ€” this is CI/YAML only. Full structural spec below (devops agent should treat this as the authoritative shape; exact YAML syntax is implementer's to finalize, but every element listed must be present). + +**Workflow name:** `Semgrep - SAST Scan` + +**Triggers:** +```yaml +on: + pull_request: + branches: [main, nightly, development] + push: + branches: [main, nightly, development] + workflow_dispatch: + schedule: + - cron: '0 4 * * 1' # Mondays 04:00 UTC โ€” offset 1h after CodeQL's 03:00 to avoid runner contention +``` + +**Concurrency:** +```yaml +concurrency: + group: ${{ github.workflow }}-${{ github.event_name }}-${{ github.head_ref || github.ref_name }} + cancel-in-progress: true +``` +(Identical pattern to `codeql.yml`.) + +**Permissions โ€” declared at both workflow (top) level and job level, identically, matching `codeql.yml`'s style (Supervisor round 2, non-blocking fix):** +```yaml +permissions: + contents: read + security-events: write + actions: read + pull-requests: read +``` +This exact block appears twice: once at the workflow top level (sibling of `on:`/`concurrency:`), and again inside `jobs.semgrep-scan.permissions` (ยง3.2). + +### 3.2 Job: `semgrep-scan` + +**Correction (Supervisor round 2, required change 1):** the pinned image is inlined directly as a literal string in `container.image` โ€” `jobs..container` does not have access to the `env` context per GitHub Actions' documented context-availability rules, so a workflow-level `env:` indirection (as drafted originally) would not resolve at all. Inlining is the only correct form, not a fallback. + +```yaml +jobs: + semgrep-scan: + name: Semgrep SAST Scan + runs-on: ubuntu-latest + timeout-minutes: 15 + permissions: + contents: read + security-events: write + actions: read + pull-requests: read + container: + image: semgrep/semgrep:1.173.0@sha256:67319956da3dcb58baf5b322899c15458e3963e7018a86aeeb5cd224e69cb77a # semgrep/semgrep 1.173.0 +``` + +### 3.3 Steps + +1. **Checkout repository** + ```yaml + - name: Checkout repository + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 + with: + ref: ${{ github.ref }} + ``` + (Same SHA already pinned and in active use in `codeql.yml` โ€” reuse, don't re-pin a different version.) + +2. **Fix git safe.directory for container user** (mitigates the "dubious ownership" issue documented in Semgrep's own GHA KB article, ยง2.5): + ```yaml + - name: Configure git safe.directory + run: git config --global --add safe.directory "$GITHUB_WORKSPACE" + ``` + +3. **Verify Semgrep parity guard** (ยง3.4): + ```yaml + - name: Verify Semgrep parity guard + run: bash scripts/ci/check-semgrep-parity.sh + ``` + +4. **Print Semgrep version** (cheap sanity check that the pinned image actually resolves to the expected CLI version โ€” catches a bad digest pin immediately and legibly, rather than surfacing as a confusing downstream scan failure): + ```yaml + - name: Verify Semgrep version + run: semgrep --version + ``` + +5. **Run Semgrep (SARIF output, non-blocking)** โ€” calls the real script (ยง2.7/ยง3.0) with the new opt-in var; no duplicated config list. + ```yaml + - name: Run Semgrep (SARIF output) + id: semgrep_sarif + continue-on-error: true + env: + SEMGREP_SARIF_OUTPUT: semgrep-results.sarif + run: bash scripts/pre-commit-hooks/semgrep-scan.sh + ``` + `continue-on-error: true` because this pass must not block the job even if semgrep itself errors โ€” the SARIF file's presence is checked explicitly next (step 6), and the actual gate is step 8, not this step. + +6. **Check Semgrep SARIF output exists** (mirrors `security-pr.yml`'s `Check Trivy SARIF output exists` step โ€” this was an orphaned reference in the first draft of this plan; it is now a real, numbered step): + ```yaml + - name: Check Semgrep SARIF output exists + id: semgrep_sarif_check + if: always() + run: | + if [ -f semgrep-results.sarif ]; then + echo "exists=true" >> "$GITHUB_OUTPUT" + else + echo "exists=false" >> "$GITHUB_OUTPUT" + echo "No Semgrep SARIF output found; skipping SARIF upload" + fi + ``` + +7. **Upload Semgrep SARIF to GitHub Security** (gated on step 6's output rather than blindly attempting the upload): + ```yaml + - name: Upload Semgrep SARIF to GitHub Security + if: always() && steps.semgrep_sarif_check.outputs.exists == 'true' + uses: github/codeql-action/upload-sarif@ff2f1c621b7f889edc0d3c761ac2e6a3f8cdb0dd # v4.37.7 + with: + sarif_file: semgrep-results.sarif + category: semgrep + continue-on-error: true + ``` + (Reuses the exact SHA already pinned for this purpose in `security-pr.yml` โ€” no new pin to introduce or maintain.) + +8. **Run Semgrep (hard-fail gate)** โ€” calls the same script, this time with the default (unset `SEMGREP_SARIF_OUTPUT`) path, i.e. its normal `--error` behavior: + ```yaml + - name: Run Semgrep (hard-fail gate) + run: bash scripts/pre-commit-hooks/semgrep-scan.sh + ``` + This is the literal `security-full` invocation (ยง2.1) โ€” same script, same default full-repo targets, same `--error` flag, run a second time (this time without the SARIF env var) so failure here genuinely gates the job. If this step fails, the job fails, blocking the PR/branch โ€” this is the CI-independent reproduction of the local "green" signal the feature exists to deliver. SARIF upload (step 7) has already completed by this point, so a gate failure here does not suppress the informational upload โ€” order matters and is intentional. + +9. **Upload SARIF artifact** (retention, matches `security-pr.yml`'s `Upload scan artifacts` step): + ```yaml + - name: Upload SARIF artifact + if: always() && steps.semgrep_sarif_check.outputs.exists == 'true' + uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v4.4.3 + with: + name: semgrep-sarif-${{ github.run_id }} + path: semgrep-results.sarif + retention-days: 14 + continue-on-error: true + ``` + +10. **Job summary** + ```yaml + - name: Create job summary + if: always() + run: | + { + echo "## Semgrep SAST Scan Results" + echo "" + echo "**Rulesets**: p/golang, p/javascript, p/typescript, p/react, p/secrets, p/dockerfile" + echo "**Severity Gate**: ERROR, WARNING (--error)" + if [ "${{ job.status }}" == "success" ]; then + echo "PASSED: no blocking Semgrep findings" + else + echo "FAILED: Semgrep reported blocking findings โ€” see step logs and the Security tab" + fi + } >> "$GITHUB_STEP_SUMMARY" + ``` + +### 3.4 New file: `scripts/ci/check-semgrep-parity.sh` + +**Revised, smaller scope (per ยง2.7's design change)** โ€” modeled on `scripts/ci/check-codeql-parity.sh`'s approach (grep/structural assertions, not full YAML parsing), but no longer needs to compare two independent config lists, because ยง3.0's design means there's only one config list in the whole repo (in `semgrep-scan.sh`) and the workflow only ever delegates to it. Exits non-zero with an `::error title=Semgrep parity drift::` annotation on any mismatch. + +**Checks performed:** + +1. Required files exist: `.github/workflows/semgrep.yml`, `scripts/pre-commit-hooks/semgrep-scan.sh`. +2. Assert `scripts/pre-commit-hooks/semgrep-scan.sh` still contains the string `SEMGREP_SARIF_OUTPUT` โ€” the additive CI hook (ยง3.0) must not be silently removed by a future edit to the script that forgets CI depends on it. +3. Assert `.github/workflows/semgrep.yml` contains **two** distinct delegating calls to the real script, not a reimplemented/inlined `semgrep scan ...` invocation: + - a call with `SEMGREP_SARIF_OUTPUT` set (the reporting pass, step 5) โ€” e.g. assert both the literal strings `SEMGREP_SARIF_OUTPUT` and `scripts/pre-commit-hooks/semgrep-scan.sh` appear within the same step block; + - a bare `bash scripts/pre-commit-hooks/semgrep-scan.sh` call with no env override (the gate pass, step 8). + - This is the direct analogue of `check-codeql-parity.sh`'s "shared blocking logic must live in exactly one place" check (that script's lines enforcing `SHARED_GATE_SCRIPT` usage) โ€” applied here to prevent a future edit from "simplifying" either step by inlining `semgrep scan` directly, which would silently reintroduce the duplicated-config problem ยง2.7 eliminated. +4. Assert `.github/workflows/semgrep.yml`'s pinned image reference matches the pattern `semgrep/semgrep:[0-9]+\.[0-9]+\.[0-9]+@sha256:[0-9a-f]{64}` (tag + digest both present โ€” catches an accidental un-pin, e.g. someone changing it to `semgrep/semgrep:latest` during a quick edit). +5. Assert `pull_request`/`push` trigger branches in `semgrep.yml` are `[main, nightly, development]`, reusing `check-codeql-parity.sh`'s existing `ensure_event_branches_semantic` helper pattern (ยง3.5). + +Note what this script **no longer does**, relative to the first draft: it does not enumerate or compare `--config`/`--exclude`/`--exclude-rule` values between two files, because after ยง3.0's change there is only one file that defines them. + +**Where it's invoked:** +- `.github/workflows/semgrep.yml` step 3 (ยง3.3), analogous to `codeql.yml`'s "Verify CodeQL parity guard" step. +- Not wired into `lefthook.yml` in this PR โ€” consistent with existing precedent: `check-codeql-parity.sh` is also CI-only today, invoked directly from `codeql.yml` and not from any lefthook stage. Noted as a possible follow-up, not a gap introduced by this plan. + +### 3.5 Shared helper extraction (optional, DRY nicety โ€” from ยง3.4 item 5) + +`check-semgrep-parity.sh` is now the second script needing the branch-list assertion logic (`ensure_event_branches` / `ensure_event_branches_with_yq` / `ensure_event_branches_semantic`) that currently lives only in `check-codeql-parity.sh`. Recommend extracting it into `scripts/ci/lib/workflow-yaml-asserts.sh`, sourced by both scripts via `source "$(dirname "${BASH_SOURCE[0]}")/lib/workflow-yaml-asserts.sh"`, per `CLAUDE.md`'s "consolidate after second occurrence" DRY guideline. This is a pure refactor of existing, already-tested logic โ€” low risk. Unlike the first draft, this extraction is no longer load-bearing for anything (the config-parity story doesn't depend on it, since there's no config duplication left to compare) โ€” it is a legitimate but strictly optional cleanup. If time-boxed out of this PR, note it as a follow-up rather than skipping silently. + +### 3.6 SARIF category naming + +`category: semgrep` for the `upload-sarif` step (ยง3.3 step 7) โ€” single, flat category since (unlike CodeQL's per-language matrix) there is only one Semgrep job/run per commit, no need for a parameterized category string. + +### 3.7 Error handling / edge cases + +| Scenario | Behavior | +|---|---| +| Pinned image digest becomes invalid/removed from registry (rare, but Docker Hub retention policies exist) | Job fails at container-pull time with a clear GitHub Actions infra error, not a silent skip. Remediation: re-resolve digest, bump the pin โ€” a normal dependency-bump PR, same as any other pinned SHA bump in this repo. | +| SARIF step (step 5) itself crashes (e.g. semgrep internal error, not a rule finding) | `continue-on-error: true` on step 5 means the job continues; step 6 explicitly checks for the SARIF file's existence and sets an output consumed by steps 7 and 9, so a missing file cleanly skips upload rather than `upload-sarif` failing opaquely on a missing path. | +| Hard-fail gate step (step 8) fails legitimately (real findings) | Job fails, PR shows a red check, `$GITHUB_STEP_SUMMARY` still renders (step 10 runs on `if: always()`), SARIF is still uploaded to the Security tab (step 7 already ran before step 8 โ€” order is intentional: SARIF upload must happen *before* the blocking step so a gate failure doesn't skip the informational upload). | +| Renovate later proposes bumping the pinned `semgrep/semgrep` image tag/digest | Handled like any other Renovate-tracked pin, via `.github/renovate.json` โ€” devops agent should confirm at implementation time whether Renovate's Docker-image datasource already picks up `container: image:` refs in workflow YAML by default, or needs an explicit entry added to `.github/renovate.json`; note as a follow-up if configuration is needed, not a blocker for this PR. | +| A future edit to `semgrep-scan.sh` removes the `SEMGREP_SARIF_OUTPUT` hook, or an edit to `semgrep.yml` reintroduces an inline `semgrep scan` call instead of delegating | `check-semgrep-parity.sh` fails CI on the very next PR that makes either change, per ยง3.4 items 2-3. | + +--- + +## 4. Implementation Plan + +This is a CI/DevOps-only change plus one small, additive shell-script change. There is no Playwright/E2E surface (no user-facing behavior changes), no backend implementation, no frontend implementation. The phase structure below is adapted accordingly โ€” **the `devops` agent implements this directly; no handoff to backend-dev, frontend-dev, or playwright-dev is needed.** + +### Phase 1 โ€” Foundation (script hook + parity guard + optional shared lib) +- Apply the additive `SEMGREP_SARIF_OUTPUT` change to `scripts/pre-commit-hooks/semgrep-scan.sh` (ยง3.0). +- Write `scripts/ci/check-semgrep-parity.sh` (ยง3.4). +- Optionally extract `scripts/ci/lib/workflow-yaml-asserts.sh` from `check-codeql-parity.sh` (ยง3.5); if done, refactor `check-codeql-parity.sh` to source it and verify it still passes unchanged. +- Validation gate: run the existing `semgrep` lefthook hooks locally (`lefthook run pre-commit` touching a Go/JS file, or `lefthook run security-full`) to confirm the script's default (`SEMGREP_SARIF_OUTPUT` unset) behavior is byte-identical to pre-change โ€” this is the regression check for ยง3.0's edit. `shellcheck scripts/pre-commit-hooks/semgrep-scan.sh scripts/ci/check-semgrep-parity.sh` (+ `scripts/ci/lib/workflow-yaml-asserts.sh` if extracted). If the shared lib was extracted, `bash scripts/ci/check-codeql-parity.sh` still exits 0 (regression check on that refactor). + +### Phase 2 โ€” Workflow file +- Write `.github/workflows/semgrep.yml` per ยง3.1-ยง3.3 (10 steps, including the SARIF-existence-check as a first-class step, not an orphaned reference). +- Validation gate: `actionlint .github/workflows/semgrep.yml` (tool already required per `lefthook.yml`'s `actionlint` hook, ยง2 header comment listing required tools). `bash scripts/ci/check-semgrep-parity.sh` now passes against the real files. YAML syntax sanity via `yq eval '.' .github/workflows/semgrep.yml >/dev/null` or equivalent. +- **Live GitHub Actions execution cannot be validated locally** โ€” the actual scan run (image pull, semgrep execution against the real repo, SARIF upload, gate pass/fail) is confirmed only once the PR opens and the workflow triggers on `pull_request`. Note this explicitly in the PR description as a manual verification step, not a local DoD gate. + +### Phase 3 โ€” Documentation +- Update `SECURITY.md` per ยง2.8: add Semgrep row to the `### Automated Scanning` table, add a `**Semgrep SAST Scan**` paragraph to `### Scanning Workflows` describing `.github/workflows/semgrep.yml`'s trigger shape and what it covers. +- Update `ARCHITECTURE.md` per ยง2.8: the three call-outs at lines 166, 1376, and 1498-1501. +- Validation gate: `markdownlint SECURITY.md ARCHITECTURE.md` (tool already required per `lefthook.yml` header comment). + +### Phase 4 โ€” Integration validation +- `lefthook run pre-commit` (full local hook suite, including the existing `actionlint` and `semgrep` hooks, to confirm nothing in this change breaks existing local gates). +- `bash scripts/ci/check-codeql-parity.sh` (if refactored) and `bash scripts/ci/check-semgrep-parity.sh` both green. +- Manual review of the rendered `semgrep.yml` against `codeql.yml`/`security-pr.yml` for pinning-comment consistency (every third-party `uses:` has a SHA + version comment; the container image has tag + digest inlined, not via `env`). + +### Phase 5 โ€” PR & CI confirmation +- Open PR; confirm `semgrep.yml` actually triggers on the PR event, pulls the pinned image successfully, produces a SARIF upload visible under the repo's Security โ†’ Code scanning alerts (filtered by tool "Semgrep"), and that the hard-fail gate step correctly reflects the current repo's Semgrep cleanliness (expected: green, since this is the same ruleset the repo already passes locally today). +- If CI surfaces findings the local run didn't (e.g. a stale local semgrep binary/ruleset that had drifted below the pinned CI version), that is itself the feature working as intended โ€” resolve findings on their merits, not by weakening the pin. + +--- + +## 5. Acceptance Criteria + +1. `.github/workflows/semgrep.yml` exists, triggers on `pull_request`/`push` to `[main, nightly, development]`, `workflow_dispatch`, and a weekly `schedule`. +2. Semgrep runs inside a `container:` whose `image:` is the pinned string `semgrep/semgrep:@sha256:` inlined directly (no `env:` indirection) โ€” no floating tag, no `@latest`. +3. `permissions:` is declared identically at both the workflow (top) level and the job level. +4. `scripts/pre-commit-hooks/semgrep-scan.sh` carries exactly the additive `SEMGREP_SARIF_OUTPUT` change described in ยง3.0 โ€” its binary/version-resolution logic and its `--config`/`--exclude`/`--exclude-rule` values are otherwise unchanged, and every existing call site's behavior (`SEMGREP_SARIF_OUTPUT` unset) is byte-identical to pre-change. +5. `.github/workflows/semgrep.yml` invokes `scripts/pre-commit-hooks/semgrep-scan.sh` directly for **both** the SARIF-producing pass (with `SEMGREP_SARIF_OUTPUT` set) and the hard-fail gate pass (unset) โ€” no independent/duplicated `semgrep scan ...` invocation exists anywhere in the workflow YAML. +6. `scripts/ci/check-semgrep-parity.sh` exists, passes against the merged state, and is invoked as a CI step in `semgrep.yml` before the scan runs. +7. SARIF results upload to the GitHub Security tab under category `semgrep`, using the same `github/codeql-action/upload-sarif` SHA pin already used in `security-pr.yml`, gated on an explicit SARIF-existence check step. +8. `SECURITY.md`'s `### Automated Scanning` table and `### Scanning Workflows` section mention Semgrep and `semgrep.yml`. +9. `ARCHITECTURE.md` mentions Semgrep at all three existing security-scanning call-out locations (tech-stack table, CI Jobs list, Container Scanning components list). +10. `.gitignore`, `.dockerignore`, `.codecov.yml`, and all Dockerfiles are confirmed unchanged (per ยง2.9 โ€” no diff expected in this PR). +11. `actionlint`, `markdownlint`, `shellcheck`, and `lefthook run pre-commit` all pass locally on the final diff. +12. `docs/plans/current_spec.md` (this file) reflects the implemented state โ€” no divergence between plan and shipped workflow at PR time (devops agent should update this file if implementation deviates from any spec section above, per standard plan-fidelity practice). + +--- + +## 6. Commit Slicing Strategy + +**Decision:** single PR, one feature ("Semgrep CI coverage"), ordered logical commits. No cross-PR splitting per `CLAUDE.md`'s "One Feature = One PR" rule โ€” this is a small, cohesive, CI-only change (plus one additive shell-script hook); splitting it further would violate that rule for no benefit (there's no independently-shippable sub-feature here โ€” a workflow with no parity guard, or a parity guard with no workflow, are both incomplete on their own). Approved as-is by Supervisor round 2 โ€” shape unchanged, contents updated below for ยง2.7's design revision and the ARCHITECTURE.md addition. + +### Commit 1 โ€” Script hook + parity guard foundation +- **Scope:** Additive-only, no behavior change for any existing call site. Adds the `SEMGREP_SARIF_OUTPUT` hook to `semgrep-scan.sh`, the new (as-yet-unused-by-CI) parity script, and optionally the shared helper extraction. +- **Files:** `scripts/pre-commit-hooks/semgrep-scan.sh` (modified โ€” additive only, per ยง3.0), `scripts/ci/check-semgrep-parity.sh` (new โ€” will fail if run now, since `semgrep.yml` doesn't exist yet; not wired into any workflow in this commit), `scripts/ci/lib/workflow-yaml-asserts.sh` (new, optional) and `scripts/ci/check-codeql-parity.sh` (refactored to source it, optional, no behavioral change) if the extraction from ยง3.5 is included. +- **Dependencies:** none. +- **Validation gate:** `shellcheck` on all touched/new scripts; local `lefthook run pre-commit` / `lefthook run security-full` on a sample file confirms `semgrep-scan.sh`'s default behavior is unchanged; `bash scripts/ci/check-codeql-parity.sh` passes unchanged if the refactor is included (regression check). +- **Commit message:** `feat(security): add opt-in SARIF output mode to local Semgrep script and add CI parity guard` + +### Commit 2 โ€” Semgrep CI workflow +- **Scope:** Adds the new workflow file, delegating both its SARIF and gate passes to the script from Commit 1, and wires the parity guard from Commit 1 into it. +- **Files:** `.github/workflows/semgrep.yml` (new). +- **Dependencies:** Commit 1 (the `SEMGREP_SARIF_OUTPUT` hook and the parity guard must exist for this workflow to reference real, working behavior). +- **Validation gate:** `actionlint .github/workflows/semgrep.yml`; `bash scripts/ci/check-semgrep-parity.sh` now passes (workflow file exists, delegates correctly, image pin format valid); `lefthook run pre-commit` clean on the diff. +- **Commit message:** `feat(security): add pinned Semgrep SAST scan to CI, mirroring local pre-commit/pre-push scan` + +### Commit 3 โ€” Documentation +- **Scope:** `SECURITY.md` and `ARCHITECTURE.md` updates only, per ยง4 Phase 3. +- **Files:** `SECURITY.md`, `ARCHITECTURE.md`. +- **Dependencies:** Commit 2 (documents the workflow file that now exists). +- **Validation gate:** `markdownlint SECURITY.md ARCHITECTURE.md`. +- **Commit message:** `docs: document Semgrep CI scan in SECURITY.md and ARCHITECTURE.md` + +### Commit 4 โ€” Hardening / fixups (conditional) +- **Scope:** Only if Phase 5 (opening the PR and observing the first real workflow run) surfaces something unfixable purely by inspection โ€” e.g. the digest needs re-resolution, `actionlint`/a GitHub Actions schema quirk requires a syntax adjustment not visible from local linting alone, or the container's default shell needs an explicit `shell: bash` on a step. +- **Files:** `.github/workflows/semgrep.yml` and/or `scripts/ci/check-semgrep-parity.sh`, as needed. +- **Dependencies:** Commits 1-3, plus one observed CI run on the PR. +- **Validation gate:** the actual GitHub Actions run on the PR going green. +- **Commit message:** `fix: address Semgrep CI workflow issues found in first live run` (only created if needed โ€” do not pre-author an empty placeholder commit). + +### Rollback / contingency + +- **Rollback:** revert the PR's merge commit. The change is additive-only (new files + one additive, backward-compatible shell-script hook + documentation sections); reverting it removes Semgrep CI coverage cleanly with no residual state โ€” no DB migration, no data written, no schema changed. `git revert -m 1 ` is sufficient. +- **Contingency โ€” pinned image becomes unpullable mid-development-cycle (e.g. registry outage, Docker Hub rate limiting on `ubuntu-latest` runners):** the job fails visibly (container pull failure is unambiguous in the Actions log, distinct from a scan failure), does not block other workflows (independent job, independent file), and does not gate merges any more strictly than any other required-check outage would โ€” same failure mode and same operational response as a transient CodeQL or Trivy Action outage today. +- **Contingency โ€” CI Semgrep surfaces findings that don't reproduce locally:** expected and desired (ยง4 Phase 5) โ€” indicates local environment drift, not a CI bug. Do not suppress via `--exclude-rule` additions without documenting rationale (matching the existing precedent set by the one documented `gorm-empty-password` exclusion already in the script). +- **Contingency โ€” parity guard is judged too strict/noisy after landing** (e.g. flags legitimate divergence that's actually fine): tune the specific assertion in `check-semgrep-parity.sh`, don't delete the guard wholesale โ€” same operating principle already established for `check-codeql-parity.sh`, which has been iterated on rather than removed. diff --git a/docs/plans/current_spec.md b/docs/plans/current_spec.md index 7aa3ef37f..3bea26e97 100644 --- a/docs/plans/current_spec.md +++ b/docs/plans/current_spec.md @@ -1,505 +1,446 @@ -# Semgrep CI Security Scan โ€” Implementation Plan +--- +goal: Migrate Charon's release/versioning pipeline from custom auto-tagging (paulhatch/semantic-version + GoReleaser) to release-please +version: 1.0 +date_created: 2026-08-17 +status: 'Planned' +tags: [chore, infrastructure, migration, ci-cd] +--- -Status: Planning complete, revised per Supervisor review (round 2). -Owner for implementation: **devops** agent (CI/CD-only change; no backend-dev or frontend-dev involvement โ€” no application code, no models, no UI). -Branch: current working branch (`development`) per `CLAUDE.md` โ€” no worktree. -PR base branch: `development` (standard feature PR convention observed in this repo; `main` only receives weekly `nightly` promotion merges). +# Introduction ---- +![Status: Planned](https://img.shields.io/badge/status-Planned-blue) -## 1. Introduction +Charon's release pipeline currently computes the next semantic version with `paulhatch/semantic-version` inside `.github/workflows/auto-versioning.yml`, then hand-rolls a changelog body with shell/grep and publishes a GitHub Release via `softprops/action-gh-release`. A second workflow, `.github/workflows/release-goreleaser.yml`, listens for the resulting `v*` tag push and runs GoReleaser โ€” but this workflow has never once succeeded (every run since `v0.3.0` fails at an "Enforce PR-2 release promotion guard" step because the gating repo variable was never set), so GoReleaser has never built or published any binary/archive/deb/rpm asset for this repo. -### 1.1 Objective +This plan replaces both workflows with `googleapis/release-please-action`, following the pattern already validated in the sibling project `/projects/go_notify_yourself`. Release-please computes versions from Conventional Commits by walking real git history, opens/maintains a standing "release PR," and tags + publishes the GitHub Release itself when that PR is merged โ€” eliminating the hand-rolled changelog script and the custom semver-calculation action in one move. -Add an independent Semgrep SAST scan to GitHub Actions CI that reproduces, byte-for-byte, the same scan behavior developers already run locally via `scripts/pre-commit-hooks/semgrep-scan.sh` (wired through `lefthook.yml`'s `pre-commit`/`pre-push`/`security-full` targets and `make security-local`). Today, Semgrep coverage exists **only** on the developer's machine โ€” CI has zero Semgrep footprint (confirmed: no match in `.github/workflows/`, `.github/renovate.json`, or any Dockerfile/compose file). This means: +This is a standalone, CI/CD-configuration-only chore. It touches no Go or TypeScript application code, no database schema, no API surface, and is entirely unrelated to the in-progress `feature/notifications-engine-extraction` work. It targets a **new branch cut from `main`**, not `development` โ€” see [Branching Note](#branching-note-deviation-from-normal-development-first-flow) below for why. -- A developer who bypasses lefthook (`--no-verify`, an emergency hotfix, a machine without semgrep installed) ships code with no Semgrep signal at all. -- Nobody re-verifies the "clean" local Semgrep run against a controlled, versioned environment โ€” the local binary's version, ruleset revision, and installed registry rules can silently drift per-developer with no CI backstop. +**Recommended branch name**: `chore/release-please-migration` (from `main`). -This plan adds CI-side Semgrep coverage that is authoritative (independent of the developer's local environment) while staying faithful to the existing local invocation. +**Merge strategy for this PR itself**: `chore/release-please-migration` โ†’ `main` should be merged via **squash merge** (the repo's default for ordinary feature/chore PRs; the "merge commit only" rule in CLAUDE.md applies specifically and only to the weekly `nightly` โ†’ `main` promotion PR โ€” see Decision 3). Squashing this PR's six commits into one `chore:`-prefixed commit on `main` is fine either way for release-please's own purposes, since none of this PR's commits are themselves releasable (`feat:`/`fix:`) and release-please's manifest is being explicitly seeded rather than derived from this PR's commit history. -### 1.2 Goals +## User Decisions Required Before Implementation -1. A new CI job runs the **exact same** rule configs, exclusions, and severity/error-gating behavior as `scripts/pre-commit-hooks/semgrep-scan.sh`'s default (no-override) path, scanning the full repo. -2. Semgrep's version is pinned in CI (image tag + digest) โ€” today there is no version pin anywhere in the repo for Semgrep, local or CI. -3. Findings are visible in the GitHub Security tab (SARIF upload), consistent with how CodeQL and Trivy results are surfaced today. -4. A hard-fail gate blocks the PR/branch on ERROR/WARNING findings, mirroring the local script's `--error` behavior โ€” CI is a gate, not just an informational report. -5. `scripts/pre-commit-hooks/semgrep-scan.sh`'s binary/version resolution logic (the `command -v semgrep` check, ยง2.1) is **not touched** โ€” that stays developer-local tooling, per the original brief's explicit scope boundary. The script's rule-config/exclude/severity logic, by contrast, **is** extended with one small, additive, backward-compatible hook (ยง2.7/ยง3.0) so CI can reuse it directly instead of duplicating it โ€” see ยง2.7 for why this is a different constraint than "freeze the whole file," and why the narrower reading is the right one. -6. Documentation (`SECURITY.md` and `ARCHITECTURE.md`) is updated to reflect the new CI coverage. +Two behavior changes were load-bearing enough that they should not be discovered after the fact โ€” surfaced here for explicit sign-off before implementation starts, in addition to their mentions later in Edge Cases / Manual Post-Merge Follow-Ups. **Both have since been resolved by explicit user sign-off; recorded below for the record.** -### 1.3 Non-goals +1. **Release cadence changes from fully-automatic to manually-gated.** Today, any non-`chore:` push to `main` is *immediately* followed by `auto-versioning.yml` cutting a tag and publishing a GitHub Release โ€” no human action required. Under release-please, a push to `main` only updates a *standing draft PR*; nothing ships (no tag, no Release, no `orthrus-build.yml` trigger) until a human explicitly merges that PR. (Release-please does support auto-merging its own PR via a label + a second small workflow, but this plan does not implement that โ€” see Manual Post-Merge Follow-Up #7 โ€” so the default behavior after this migration is manual-gate-by-default.) **RESOLVED โ€” user sign-off: APPROVED, proceed as planned (manual-gate-by-default, no auto-merge configured in this PR).** +2. **Chore-only pushes no longer cut a release**, and **`perf:` commits stop being patch-worthy.** Today, a week where only `chore:`/`docs:`/`test:`/`ci:`/`build:`/`style:`/`refactor:` commits land on `main` still gets a patch-bumped release (since `auto-versioning.yml` treats "anything that isn't `feat:`" as patch-worthy, and its changelog-categorization step explicitly buckets `perf:` alongside `fix:`). Under release-please's default releasable-type set (`feat`, `fix`, `deps`), a chore-only period produces **no** release PR at all, and `perf:` commits โ€” not in that default set โ€” likewise stop triggering a release/changelog entry on their own. **RESOLVED โ€” user sign-off: ACCEPT release-please's defaults as-is. Do not add `perf:` (or any other type) to `changelog-sections`/releasable types as a special-case override; `perf:` commits behave like `chore:`/`docs:`/`test:` under release-please's out-of-the-box config** โ€” no config change proposed for this. This is treated as an accepted, disclosed behavior change (arguably a correctness improvement โ€” no more empty "dependency updates and maintenance" releases), not an oversight. -- No change to how the local pre-commit/pre-push semgrep **binary** is discovered, installed, or versioned (the `command -v semgrep` / exit-127 block in `scripts/pre-commit-hooks/semgrep-scan.sh` is untouched). -- No new GitHub Action marketplace dependency requiring npm/JS runtime โ€” Semgrep ships as a self-contained CLI in an official container image, which is used directly. -- No change to `.gitignore`, `.dockerignore`, `.codecov.yml`, or any `Dockerfile` (see ยง2.9 โ€” reviewed explicitly, no changes needed). -- No attempt to unify Trivy's/CodeQL's SARIF-upload plumbing into a shared reusable workflow โ€” out of scope for this feature; each scanner's workflow remains independent, consistent with current repo structure (`codeql.yml`, `security-pr.yml`, `security-weekly-rebuild.yml` are all separate files today). +## Branching Note: Deviation from Normal Development-First Flow ---- +The repo's normal convention (see the Semgrep CI plan previously in this file, and the "Merge Soak Before Main" project memory) is feature branches off `development`, PR into `development`, then a weekly `nightly` โ†’ `main` promotion carries validated work to `main` roughly a week later. This plan deliberately breaks that pattern: `auto-versioning.yml`, `release-goreleaser.yml`, and the new `release-please.yml` all trigger specifically on `main` (`workflow_run` on the main-branch Docker build, or `push: branches: [main]`), and the live release state (existing `v*` tags, the `CHARON_PR2_GATES_PASSED` variable, the next real release cut) only exists on `main`. A `development`-first path would leave the repo running two parallel, half-migrated release mechanisms for up to a week, which is worse than a direct, carefully-gated `main` PR. **Recommendation for the user**: apply extra manual review scrutiny before merging this PR, since it bypasses the usual nightly soak period by design, not by oversight. + +# Research Findings + +## Existing Architecture Summary + +| File | Role | Status found | +|---|---|---| +| `.github/workflows/auto-versioning.yml` | Computes next semver via `paulhatch/semantic-version@v6.0.3` from `workflow_run` of "Docker Build, Publish & Test" on `main`; hand-greps `feat:`/`fix:`/`perf:` commit-body bullets into a release note; creates tag + GitHub Release via `softprops/action-gh-release@v3`. | Working, but hand-rolled and duplicative of what release-please does natively. | +| `.github/workflows/release-goreleaser.yml` | Triggers on `push: tags: ['v*']`; runs `goreleaser release --clean` (builds linux amd64/arm64 binary, tar.gz archive, deb/rpm via nfpm). | **Never succeeded.** Every run since `v0.3.0` fails at "Enforce PR-2 release promotion guard" (repo variable `CHARON_PR2_GATES_PASSED` has never been set โ€” confirmed via `gh api repos/Wikid82/charon/actions/workflows/release-goreleaser.yml/runs`, `REPO_VARS_JSON: {}` on every run). `gh release view` on `v0.36.3`โ€“`v0.36.5` shows zero attached assets. | +| `.goreleaser.yaml` | GoReleaser config: builds linux/amd64+arm64 binary, tar.gz archive, deb/rpm packages, changelog section. Its own header comment: *"used exclusively for changelog generation... builds/archives/nfpms kept for potential future use but not currently utilized."* | Dead weight โ€” see Decision 5. | +| `.github/workflows/docker-build.yml` | Builds/publishes the actual Charon Docker images. Triggers: `pull_request`, `push: branches: [main, development]`, `workflow_dispatch`, `workflow_run` (Docker Lint). **Does NOT trigger on tag push** (`on:` block has no `tags:` key โ€” confirmed by reading the full trigger block). | This is the sole real distribution channel for Charon (100% Docker). Its `docker/metadata-action` step (line ~334-345) includes `type=semver,pattern={{version}}` tag patterns, but since the workflow never runs on a tag ref, `TRIGGER_REF` is always `refs/heads/*` and those semver patterns never actually resolve to anything โ€” they are inert/vestigial in the current design, not a live tag consumer. | +| `.github/workflows/orthrus-build.yml` | Builds the separate Orthrus agent image. Triggers: `push: branches: [main, development], tags: ['v*']`. Its `docker/metadata-action` step (`type=semver,pattern={{version}}` etc.) **does** fire off tag pushes. | **This is the one real, live downstream consumer of the `v*` tag** that this migration must not break โ€” release-please's created tag must still be a bare `v` (see Decision 1 / tag-naming risk below) for this workflow's `tags: ['v*']` trigger and semver Docker-tag derivation to keep working identically. | +| `.github/workflows/auto-changelog.yml` ("Auto Changelog (Release Drafter)") | Triggers on `workflow_run` (Docker Build success on `main`) and `release: types: [published]`; runs `release-drafter/release-drafter@v7` against `.github/release-drafter.yml`. | **Not mentioned in the task brief but discovered during research โ€” see "Additional Finding" below.** Redundant with release-please's own standing-PR/changelog mechanism; its own tag-template (`v$NEXT_PATCH_VERSION`) always increments patch regardless of PR label, so it's already partially broken (a PR labeled `feature` would still only bump patch). | +| `.github/release-drafter.yml` | release-drafter config: label-based categorization (`feature`/`feat`, `bug`/`fix`, `chore`, `test`), `tag-template: 'v$NEXT_PATCH_VERSION'`. | Retire alongside `auto-changelog.yml` โ€” see Additional Finding. | +| `VERSION.md` | Documents the "canonical" release process. References `.version` as optional/non-canonical (still accurate). Also references a "release-drafter workflow" for changelog generation โ€” **this turned out to be real** (`auto-changelog.yml`), not stale as originally suspected; the doc's inaccuracy is instead that it describes `release-goreleaser.yml`/`docker-build.yml` as if they jointly "build and publish release artifacts/images through CI" from the tag, which is false per the findings above. | Needs a full rewrite (Phase 5 / Commit 6). | +| `.version` | Currently `v0.27.0` (stale โ€” real latest tag is `v0.36.5`). Already documented in `VERSION.md` as "optional... not the canonical release trigger." | Recommend removal โ€” see Decision 4. | +| `scripts/check-version-match-tag.sh` | Compares `.version` to the latest git tag; **already self-deprecating** โ€” prints a warning telling callers to use `.github/skills/scripts/skill-runner.sh utility-version-check` instead, then runs its own logic anyway. `.github/skills/utility-version-check-scripts/run.sh` just `exec`s this same script โ€” the "migration" to the skill runner is circular and never actually happened. | Recommend removal โ€” see Decision 4. | +| `lefthook.yml` (line ~99) | `check-version-match: { glob: ".version", run: "bash scripts/check-version-match-tag.sh" }` | Remove this hook entry alongside `.version` deletion (Commit 5). | +| `backend/internal/version/version.go` | `Version`, `BuildTime`, `GitCommit` vars, defaulted to `"dev"`/`"unknown"`, set via `-ldflags -X ...` at build time. No in-repo manifest file to bump โ€” confirms release-please needs a manifest-less strategy (Decision 1). | Untouched by this migration. | +| `Dockerfile` (lines ~250-286) | Injects `VERSION`/`GIT_COMMIT`/`BUILD_DATE` into the Go binary via `-ldflags -X github.com/Wikid82/charon/backend/internal/version.*=...`, **identically** to what `.goreleaser.yaml`'s `builds.ldflags` does. Also sets `VITE_APP_VERSION` for the frontend build (line ~133-134). | Confirms version injection already happens independently of GoReleaser in the real (Docker) build path โ€” removing GoReleaser does not touch actual version injection at all. | +| `scripts/generate-changelog.sh` | Regenerates `backend/internal/changelog/data/changelog.json` (the in-app "What's New" popup's data source, `go:embed`-ed at build time) by walking `git tag -l 'v*' --sort=v:refname` and categorizing commit subjects between each pair of tags via conventional-commit regex (feat/fix/security-scoped/other). **Depends only on real `v*` tags existing in git history** โ€” it does not read `auto-versioning.yml`'s or `release-goreleaser.yml`'s output, the GitHub Releases API, or any state those workflows produce. | **Fully decoupled from this migration** as long as release-please still creates real, plain-`v*`-prefixed git tags (Decision 1's tag-naming requirement is what makes this true). Called from `nightly-build.yml` (line 225) and the dead `release-goreleaser.yml` (line 77) โ€” **not** from `docker-build.yml`, meaning stable/`latest`-tagged production images currently always ship the placeholder `[]` changelog while nightly images get real data. This is a **pre-existing gap in `docker-build.yml`, out of scope for this CI/CD-versioning-only migration**, flagged here only so it isn't mistaken for something this migration caused or should fix. | +| `backend/internal/services/update_service.go` (line 40) | Self-update checker hits `https://api.github.com/repos/Wikid82/charon/releases/latest` for `tag_name`/`html_url` only โ€” does not need release assets. | Unaffected: release-please still publishes a `releases/latest`-eligible GitHub Release with those fields. | +| `backend/internal/services/orthrus_service.go` (line ~222) | Orthrus agent "Tarball" install method's `curl` command points at `.../releases/latest/download/charon-agent-linux-amd64.tar.gz`. | **This asset has never existed** (GoReleaser never ran successfully, and even if it had, `.goreleaser.yaml` builds `charon`, not `charon-agent`). This is a pre-existing, already-broken feature, unrelated to and unmade-worse by this migration โ€” flagged for the user's awareness only; fixing it is out of scope (would require actually building/publishing an agent binary, a feature-level change). | +| `CHANGELOG.md` (repo root, 600 lines) | Hand-curated, Keep-a-Changelog-format, rich multi-line entries with issue references. Not written by any current CI workflow. | **Conflicts with release-please's default "go" strategy**, which manages `CHANGELOG.md` by prepending auto-generated entries. See Additional Finding / Decision below โ€” resolved via `skip-changelog: true`. | +| `renovate.json` | No references to `goreleaser`/`release-goreleaser`/`release-drafter` by name โ€” Renovate discovers pinned actions generically by scanning workflow YAML. | No Renovate config changes needed; removing `release-goreleaser.yml` and `auto-changelog.yml` simply removes those pinned actions from Renovate's future PRs. | +| `.gitignore` (line ~163-167) | Dedicated `# GoReleaser` section ignoring `dist/`. | Remove alongside `.goreleaser.yaml` (Commit 5). | +| `.dockerignore` (line 13) | Lists `.goreleaser.yaml`. | Remove alongside `.goreleaser.yaml` (Commit 5). | +| `codecov.yml` | No workflow-file or release-artifact references in `ignore:`. | No changes needed. | + +## Reference Implementation (`/projects/go_notify_yourself`) -## 2. Research Findings - -### 2.1 Local Semgrep invocation (`scripts/pre-commit-hooks/semgrep-scan.sh`) - -Full script behavior (verified by reading the file): - -- Requires `semgrep` on `PATH`; exits 127 if missing (this resolution logic is untouched by this plan โ€” see ยง1.3). -- Default rule configs (used unless `SEMGREP_CONFIG` env override is set): - ``` - --config p/golang - --config p/javascript - --config p/typescript - --config p/react - --config p/secrets - --config p/dockerfile - ``` -- Targets: staged files if passed as args (lefthook `pre-commit`), else full-repo default `Dockerfile backend frontend/src scripts .github/workflows` (lefthook `security-full` / manual run). -- Exact scan flags (current, pre-change): - ``` - semgrep scan \ - "${SEMGREP_CONFIGS[@]}" \ - --severity ERROR \ - --severity WARNING \ - --error \ - --exclude "frontend/node_modules" \ - --exclude "frontend/coverage" \ - --exclude "frontend/dist" \ - --exclude-rule "go.secrets.gorm.gorm-empty-password.gorm-empty-password" \ - "${TARGETS[@]}" - ``` -- `--error` makes semgrep exit non-zero if any ERROR/WARNING-severity finding exists โ€” this is the local "hard fail" behavior CI must reproduce. - -Wiring confirmed in `lefthook.yml`: -- `pre-commit.semgrep` (line ~113-116): glob-scoped, staged-files-only, blocking. -- `security-full.semgrep` (line ~137-140, manual stage, `lefthook run security-full`): full-repo, no args โ†’ this is the invocation CI should mirror most closely (full-repo, not staged-file-scoped). -- `Makefile:security-local` additionally runs `SEMGREP_CONFIG=p/golang` as a fast pre-push subset โ€” this is a narrower override path, not the target for CI parity (CI should mirror the **full** default ruleset, matching `security-full`). - -### 2.2 Confirmed: zero Semgrep footprint in CI today - -`grep -rn "semgrep" .github/workflows/ .github/renovate.json` (and Dockerfiles/compose) returns no matches. Semgrep is 100% local-only today. (Note: the repo's Renovate config lives at `.github/renovate.json`, not a root-level `renovate.json` โ€” corrected throughout this plan.) - -### 2.3 Existing CI patterns to mirror - -**`.github/workflows/codeql.yml`** (closest pattern for a source-level SAST tool): -- Triggers: `pull_request`/`push` on `[main, nightly, development]`, `workflow_dispatch`, weekly `schedule` cron (`0 3 * * 1`, Mondays 03:00 UTC). -- `concurrency` group keyed on workflow/event/ref, `cancel-in-progress: true`. -- `permissions:` declared at **both** the workflow (top) level and again, identically, at job level (`contents: read`, `security-events: write`, `actions: read`, `pull-requests: read`). -- All third-party actions pinned by commit SHA with a `# vX.Y.Z` trailing comment, e.g. `github/codeql-action/init@ff2f1c621b7f889edc0d3c761ac2e6a3f8cdb0dd # v4`. -- Has a **parity guard** step ("Verify CodeQL parity guard" โ†’ `scripts/ci/check-codeql-parity.sh`) that runs *before* the scan, structurally checking that local pre-commit scripts, `.vscode/tasks.json`, and the CI workflow all agree on query-suite pinning and trigger branches โ€” added specifically because CodeQL's local/CI ruleset previously drifted silently (see `check-codeql-parity.sh` comment referencing a real incident: a suppressed finding rode through PR #1216 unnoticed because local and CI independently duplicated blocking logic). -- Emits results to `$GITHUB_STEP_SUMMARY`, then a **separate, later step** does the actual hard-fail (`Fail on High-Severity Findings`) โ€” reporting and gating are deliberately split into two steps so the summary always renders even on failure. - -**`.github/workflows/security-pr.yml`** (closest pattern for "pinned scanner โ†’ SARIF upload โ†’ hard-fail gate"): -- Runs Trivy via `aquasecurity/trivy-action@ed142fd0673e97e23eac54620cfb913e5ce36c25` (SHA-pinned, `# aquasecurity/trivy-action 0.36.0` comment), with an explicit `version: 'v0.73.0'` input additionally pinning the *scanner* version, not just the action wrapper. -- Runs the scan **twice**: once with `format: 'sarif'` (`continue-on-error: true`, purely for the Security tab), then again with `format: 'table'` + `exit-code: '1'` (no continue-on-error) as the actual blocking gate. It also has an explicit "Check Trivy SARIF output exists" gating step between the SARIF-producing run and the upload step. This two-pass "report, then gate" split, plus the existence check, is the direct template for Semgrep's SARIF-vs-hard-fail split (ยง3.3). -- SARIF uploaded via `github/codeql-action/upload-sarif@ff2f1c621b7f889edc0d3c761ac2e6a3f8cdb0dd # v4.37.7` (same SHA-pinned action already used elsewhere in this repo for SARIF ingestion โ€” no new third-party dependency needed for the upload step). -- Trigger shape is materially more complex than needed here (`workflow_run` chaining off `docker-build.yml`, PR-number resolution, artifact download) because Trivy scans a **built container image**. Semgrep scans **source**, so it needs none of that โ€” it can trigger directly on `push`/`pull_request` like CodeQL, with no dependency on a prior Docker build. - -### 2.4 Repo-wide pinning convention - -Every third-party action in this repo is pinned to an exact commit SHA with a trailing `# vX.Y.Z` comment โ€” never a floating tag, never `@latest`. This is enforced by convention/review, not currently by a lint rule for actions specifically. Any new job must follow this exactly. - -### 2.5 Semgrep version/mechanism research - -Options considered: - -| Option | Assessment | +- `.release-please-manifest.json`: `{".": "0.2.0"}`. +- `.github/workflows/release-please.yml`: `googleapis/release-please-action@45996ed1f6d02564a971a2fa1b5860e934307cf7 # v5`, `on: push: branches: [main]`, `permissions: contents: write, pull-requests: write`, `config-file`/`manifest-file` inputs. +- `release-please-config.json`: **as of this session, corrected to `"release-type": "go"`** (it was `"node"` earlier in the session, before the sibling repo's own config was independently fixed โ€” this happened to land on the exact same conclusion this plan's research below reaches independently via release-please's own docs, which is reassuring cross-validation, not something to blindly trust as "already proven correct"). The repo has no `package.json`, so `"node"` would have been wrong (release-please would look for a manifest file that doesn't exist). + +# Six Required Decisions + +## Decision 1 โ€” `release-type` for `release-please-config.json` + +**Decision: `"release-type": "go"`.** + +Verified directly against release-please's documented strategy table (`docs/customizing.md` release-type list, fetched this session): + +| release-type | Description (verbatim from docs) | |---|---| -| `pip install semgrep==` on `ubuntu-latest` | Works, but reintroduces a Python toolchain dependency into a Go+TS repo purely for CI plumbing (`CLAUDE.md`: "No Python โ€” do not introduce Python scripts or requirements"). While this is arguably a tooling install rather than an authored script, it still pulls in `pip`/Python resolution behavior (version solving, transitive dependency drift) that the repo's own conventions steer away from. Rejected. | -| `semgrep/semgrep-action` (formerly `returntocorp/semgrep-action`) marketplace GitHub Action | Semgrep's own current CI docs no longer lead with this as the primary GitHub Actions pattern; it's a thin wrapper around the same official Docker image. Using it would add an extra layer of indirection (an Action wrapping an image) for no behavioral benefit over using the image directly, and re-pinning *that* action's SHA doesn't pin Semgrep's own version any more precisely than pinning the image does. Rejected in favor of the image directly. | -| Official `semgrep/semgrep` Docker image, used as a job-level `container:`, pinned by exact tag **and** digest | Matches this repo's SHA-pinning strictness (a digest is the container-image equivalent of an action's commit SHA โ€” both are content-addressed, immutable references). Gives the CLI directly, with the identical `semgrep scan ...` invocation used locally โ€” maximizes behavioral parity with `semgrep-scan.sh`. **Selected.** | +| `go` | "A repository with a CHANGELOG.md" | +| `simple` | "A repository with a version.txt and a CHANGELOG.md" | +| `node` | "A Node.js repository, with a package.json and CHANGELOG.md" | -Confirmed via the Semgrep GitHub releases API (`api.github.com/repos/semgrep/semgrep/releases/latest`) and PyPI, current stable version at plan time is **`1.173.0`**. Resolved the corresponding Docker Hub manifest digest for `semgrep/semgrep:1.173.0`: +`simple` was the task brief's suggested candidate to verify โ€” it is **not** correct: it manages a `version.txt` manifest file, which Charon does not have and does not want (per the pre-verified finding: no in-repo version manifest exists, version comes from git tag โ†’ Docker build-arg โ†’ Go ldflags only). `go` is the only listed strategy requiring **zero** manifest file โ€” exactly Charon's situation, and it still manages `CHANGELOG.md` (mitigated via `skip-changelog: true`, see Additional Finding below) and still creates git tags + GitHub Releases from Conventional Commits. -``` -sha256:67319956da3dcb58baf5b322899c15458e3963e7018a86aeeb5cd224e69cb77a -``` +`node` (the sibling repo's original, now-corrected choice) would have required release-please to manage a `package.json` that doesn't exist in either repo โ€” confirmed by direct filesystem check of `/projects/go_notify_yourself` (no `package.json` present). Not applicable to Charon either. -Pinned reference to use in the workflow: +## Decision 2 โ€” Downstream consumers of the tag/Release that `auto-versioning.yml` currently produces -``` -semgrep/semgrep:1.173.0@sha256:67319956da3dcb58baf5b322899c15458e3963e7018a86aeeb5cd224e69cb77a -``` +Read in full: `docker-build.yml`, `orthrus-build.yml`, `auto-changelog.yml`, `nightly-build.yml`, `update_service.go`, `orthrus_service.go`, `generate-changelog.sh`. + +| Consumer | Trigger | Depends on | +|---|---|---| +| `orthrus-build.yml` | `push: tags: ['v*']` | **Real dependency.** Must keep firing on the same bare `v` tag pattern; its `docker/metadata-action` step derives `type=semver` Docker tags directly from the pushed tag ref. | +| `docker-build.yml` | Branch push only, never tags | **No dependency** โ€” confirmed dead/inert `type=semver` patterns in its own `docker/metadata-action` config (never actually reached because `TRIGGER_REF` is always a branch ref). Nothing to preserve here beyond not breaking branch-push behavior, which this migration doesn't touch. | +| `release-goreleaser.yml` | `push: tags: ['v*']` | **No real dependency** โ€” never succeeded, produces nothing (Decision 5). | +| `auto-changelog.yml` | `workflow_run` + `release: types: [published]` | Redundant, not load-bearing for anything else โ€” see Additional Finding. | +| `update_service.go` | Polls `releases/latest` REST endpoint | Needs *a* GitHub Release to exist with `tag_name`/`html_url` โ€” release-please provides this natively. | +| `generate-changelog.sh` (via `nightly-build.yml`) | Reads `git tag -l 'v*'` directly | Needs real `v*` tags in git history โ€” release-please provides this as long as tag-naming is kept bare (Decision 1's `include-component-in-tag: false`, see risk below). | + +**Conclusion**: the only workflow with a genuine, live functional dependency on the tag is `orthrus-build.yml`, and it only needs the tag to (a) exist, (b) match glob `v*`, (c) parse as semver for the `docker/metadata-action` `type=semver` patterns. Release-please satisfies all three by default, provided tag-naming is pinned explicitly (see risk callout in Technical Specifications). + +## Decision 3 โ€” Interaction with the nightlyโ†’main "merge commit only" rule -**Note for the implementer (devops agent):** re-resolve this digest at implementation time (`docker buildx imagetools inspect semgrep/semgrep:1.173.0` or the registry API) rather than trusting the value transcribed into this plan verbatim, in case the tag's digest has moved between planning and implementation (Docker Hub does not guarantee a tag's digest is immutable the way a Git SHA is โ€” pinning to the *tag+digest pair as observed at merge time* is the achievable guarantee here, and Renovate, already active in this repo via `.github/renovate.json`, will pick up future digest/tag bumps the same way it tracks other pinned SHAs if configured to watch this image โ€” see ยง3.7 edge case). +**Decision: keep the CLAUDE.md rule unchanged. Do not relax it, and do not propose editing CLAUDE.md.** -**Known gotcha (Semgrep's own docs, `semgrep.dev/docs/kb/semgrep-ci/using-nonroot-docker-image-with-gha`):** running `semgrep/semgrep` as a job-level `container:` against a `actions/checkout`-produced workspace can hit git's "dubious ownership" safety check because the container user doesn't match the checkout's file ownership. Mitigate with an explicit `git config --global --add safe.directory "$GITHUB_WORKSPACE"` step before invoking `semgrep-scan.sh` (ยง3.3). +Investigated directly against release-please's docs (`docs/design.md`, GitHub README commit-conventions section, fetched this session): -**Correction (Supervisor round 2, required change 1):** the container image reference **cannot** be centralized in a workflow-level `env:` var and referenced as `container.image: ${{ env.SEMGREP_IMAGE }}`. GitHub Actions' documented context-availability rules do not expose the `env` context to `jobs..container` โ€” this is a known, currently-true limitation (not something that needs "verification at implementation time"; treating it as an open question in the prior draft was itself the error). The plan now specifies the pinned string **inlined directly** in `container.image` as the only correct form (ยง3.2) โ€” no `env` indirection. +- Release-please's own docs state it **"highly recommends"** squash-merging *feature* PRs into a linear history, and that it discovers releasable commits by **iterating backwards through actual git commits** (not by re-parsing PR bodies or bullet-ized squash-commit text) until it hits a known prior release SHA. +- This is superficially the opposite of Charon's current constraint (which exists specifically to keep bullet-per-commit squash bodies parseable by `paulhatch/semantic-version`'s regex). But the underlying *mechanism* that makes the constraint necessary is unchanged: the weekly `nightly` โ†’ `main` promotion PR itself accumulates a full week of already-individually-squashed feature commits. If that promotion PR were **squash-merged** into `main`, all of that week's discrete `feat:`/`fix:` commits would collapse into a single commit on `main` whose own subject line is whatever GitHub picks for the squash (typically the PR title, not necessarily a clean Conventional Commit type) โ€” release-please's per-commit git-log walk would then see **one** commit for the entire week, not one-per-change, and lose the same granularity that currently breaks `paulhatch`. Using **"Create a merge commit"** for the promotion PR preserves each week's individual squashed-per-feature commits as distinct commits in `main`'s history, which is exactly what release-please's commit-by-commit walk needs to correctly attribute each `feat:`/`fix:` to the right release. +- **Conclusion**: the rule's *justification* text in CLAUDE.md ("squash merging collapses all commits into bullet lines that the auto-versioning workflow cannot parse") becomes slightly inaccurate wording once `paulhatch` is gone, but the *rule itself* remains equally necessary under release-please, for an adjacent reason. This plan does **not** propose editing CLAUDE.md's rule or its wording โ€” flagging the wording-vs-mechanism nuance here is for the user's own future reference only, per the task's instruction not to propose CLAUDE.md edits. -### 2.6 Placement decision: new file vs. an existing workflow +## Decision 4 โ€” Fate of `.version` and `scripts/check-version-match-tag.sh` -*(Per mid-task correction from Management: decide the best location and justify it, rather than defaulting to a new file. Approved as-is by Supervisor round 2 โ€” no changes in this revision.)* +**Decision: remove both, plus the `check-version-match` lefthook hook entry (`lefthook.yml` line ~99) and the now-pointless `.github/skills/utility-version-check*` skill wrapper.** -Three placements were evaluated: +Reasoning: +- `.version` is already stale (`v0.27.0` vs. real latest tag `v0.36.5`) and already documented in `VERSION.md` as "optional... not the canonical release trigger" โ€” it carries no functional weight today. +- `scripts/check-version-match-tag.sh` is **already self-deprecated in its own source** (prints a warning telling callers to use the skill-runner instead) โ€” but `.github/skills/utility-version-check-scripts/run.sh` just `exec`s this same script, so the "recommended" migration path is circular dead code, not an actual alternative implementation. +- Release-please replaces the entire concept this check exists for: `.release-please-manifest.json` becomes the single source of truth for "what version are we at," continuously kept in sync with tags by release-please itself. A hand-run parity check between a stale flat-text file and `git tag` adds no safety release-please doesn't already provide, and having *two* "canonical" version records (`.release-please-manifest.json` and `.version`) invites exactly the kind of drift the check script exists to catch. +- The check is non-blocking today (`exit 0` when `.version` is absent, per the script's own logic), so removing the file cannot regress any currently-enforced gate. -| Placement | Verdict | +**Full blast radius** (repo-wide `grep -rln "utility-version-check\|check-version-match-tag"`, re-verified against Supervisor's independent review): + +| Reference | Action | |---|---| -| **New job added to `codeql.yml`** | Rejected. `codeql.yml`'s entire structure is a `strategy.matrix` over CodeQL *languages* (`go`, `javascript-typescript`), with per-language conditional steps (`if: matrix.language == 'go'`) for Go toolchain setup/build and the CodeQL parity guard. Semgrep is not a CodeQL language variant โ€” it's a different tool with a different container, different config format, and a different (single, non-matrixed) invocation. Bolting it in as a third matrix leg would force awkward `if: matrix.language == 'semgrep'` conditionals across steps that don't apply to it (Autobuild, `codeql-action/init`, Go build verification), degrading the readability of a file whose entire premise is "one job, matrixed by CodeQL language." Also couples Semgrep's schedule/trigger lifecycle to CodeQL's, when they are independent tools that should be able to fail, be disabled, or be re-scheduled independently. | -| **New job added to `security-pr.yml`** | Rejected. That workflow's trigger shape and majority of its steps exist *solely* to solve "how do I scan a Docker image that was already built by a separate upstream workflow" โ€” PR-number resolution from `workflow_run` payloads, artifact download/load fallback logic, container extraction of the `charon` binary, a trust-boundary validation step for the `workflow_run` event. None of that applies to Semgrep, which scans source text directly on `push`/`pull_request` with no dependency on `docker-build.yml` having run first. Adding a source-scanning job to an image-scanning workflow would mean either (a) it inherits triggers/conditions built for image scanning that don't fit it (e.g. `workflow_dispatch` inputs are `pr_number`-shaped, meaningless for a source scan), or (b) it needs its own parallel `if:` conditions bolted onto an already condition-heavy file, adding complexity for no shared benefit โ€” the two jobs would share a file but no actual logic. | -| **New file: `.github/workflows/semgrep.yml`** | **Selected.** Semgrep is source-level SAST, triggered directly on `push`/`pull_request`/`schedule`/`workflow_dispatch` โ€” structurally identical in trigger shape to `codeql.yml`, but a distinct tool with its own container, config, and failure/gating semantics. This also matches the repo's existing convention of **one file per scanner**: `codeql.yml` (CodeQL), `security-pr.yml` (Trivy on PR images), `security-weekly-rebuild.yml` (Trivy weekly full scan) are already separate files rather than merged into one "security" workflow, even though they're conceptually related. A dedicated `semgrep.yml` continues that pattern: each scanner is independently triggerable, independently disable-able, and independently readable, at the cost of one more file โ€” a cost the repo has already accepted three times over for its other scanners. | - -### 2.7 "Freeze the whole script" reconsidered โ€” design revision (Supervisor round 2, required change 3) - -**The original brief's non-goal, re-read precisely:** *"Do NOT touch `scripts/pre-commit-hooks/semgrep-scan.sh`'s binary/version resolution logic itself โ€” that's explicitly out of scope, reserved for the user's own local tooling."* This is a constraint about **binary/version discovery** (the `command -v semgrep` / exit-127 block, ยง2.1) โ€” not a blanket freeze on every line of the file. The first draft of this plan over-read it into "never touch this file at all," which forced: - -- A second, hand-written `semgrep scan ...` invocation inline in the workflow YAML, duplicating all six `--config` flags, all three `--exclude` flags, and the `--exclude-rule` value. -- A `check-semgrep-parity.sh` script whose primary job was detecting drift between that duplicated invocation and the real script. -- Pressure to extract shared assertion helpers out of `check-codeql-parity.sh` mainly to support that parity script's config-matching checks. - -That is real, avoidable complexity, not an inherent requirement. **Revised design (adopted โ€” option (a) from Supervisor's feedback):** add one small, additive, backward-compatible hook to `semgrep-scan.sh` itself, leaving the binary/version-resolution logic (the actual thing the non-goal protects) completely untouched: - -```bash -# Existing lines (SEMGREP_CONFIGS / TARGETS construction) unchanged above this point. - -if [ -n "${SEMGREP_SARIF_OUTPUT:-}" ]; then - OUTPUT_FLAGS=(--sarif --output "${SEMGREP_SARIF_OUTPUT}") -else - OUTPUT_FLAGS=(--error) -fi - -semgrep scan \ - "${SEMGREP_CONFIGS[@]}" \ - --severity ERROR \ - --severity WARNING \ - "${OUTPUT_FLAGS[@]}" \ - --exclude "frontend/node_modules" \ - --exclude "frontend/coverage" \ - --exclude "frontend/dist" \ - --exclude-rule "go.secrets.gorm.gorm-empty-password.gorm-empty-password" \ - "${TARGETS[@]}" -``` +| `.github/skills/utility-version-check-scripts/run.sh`, `.github/skills/utility-version-check.SKILL.md`, `scripts/check-version-match-tag.sh`, `.version`, `lefthook.yml`'s `check-version-match` hook | Delete/remove โ€” already in original Deleted/Modified Files scope. | +| `.github/skills/README.md:72` (Utility Skills table row) and `:267` (kebab-case naming example, `utility-version-check` bullet) | **Now added to this plan's scope**: remove both โ€” assigned to Commit 5. | +| `.vscode/tasks.json:694-698` ("Utility: Check Version Match Tag" task, shells out to `skill-runner.sh utility-version-check`) | **Now added to this plan's scope**: remove this task block โ€” assigned to Commit 5. Left in place, it would error every time it's run post-deletion. | +| `.github/skills/utility-bump-beta.SKILL.md:186` ("Related Skills" cross-link to `utility-version-check.SKILL.md`) | **Now added to this plan's scope**: remove the dead link (keep the rest of that skill's "Related Skills" list intact) โ€” assigned to Commit 5. | +| `CLAUDE.md:261` (Skills table row: `utility-version-check \| Check tool versions`) | **Amended after explicit user sign-off**: the user has explicitly authorized editing this governance row as part of this PR ("include the edit in claude.md. no need to make it a follow-up when it can be done now"), satisfying this plan's own constraint against silently touching CLAUDE.md. **Decision: remove the `utility-version-check` row from CLAUDE.md's Skills table (line 261) in this PR**, folded into Commit 5 alongside the rest of this skill's deletion (contingent on that deletion actually happening in this same commit, which it does โ€” see Commit 5 scope). No longer deferred as a Manual Post-Merge Follow-Up. | -Behavior: -- **`SEMGREP_SARIF_OUTPUT` unset (every existing call site โ€” `pre-commit`, `pre-push`/`security-full`, `make security-local`):** `OUTPUT_FLAGS=(--error)` โ€” byte-identical to today's behavior. Zero change for any existing developer workflow. -- **`SEMGREP_SARIF_OUTPUT=` set (new โ€” CI only):** swaps `--error` for `--sarif --output `, while every `--config`, `--exclude`, and `--exclude-rule` argument stays exactly as-is, sourced from exactly one place. +## Decision 5 โ€” Fate of `.goreleaser.yaml` and `release-goreleaser.yml` -This lets CI invoke the **same script** for both the SARIF-producing pass and the hard-fail gate pass (ยง3.3 steps 5 and 8), varying only an env var. Consequences: +**Decision: remove both entirely.** -- The duplicated `--config`/`--exclude` list in the workflow YAML is **eliminated entirely** โ€” there is now exactly one place (`semgrep-scan.sh`) that defines what gets scanned, for both local and CI, for both the reporting pass and the gating pass. -- `check-semgrep-parity.sh` shrinks correspondingly (ยง3.4) โ€” it no longer needs to compare two independent config lists (nothing to compare; there's only one). It still has a real, narrower job: confirming the additive hook isn't silently removed, confirming the workflow actually delegates to the script for both passes (rather than a future edit reintroducing an inline duplicate), and confirming the image pin and trigger branches stay correct. This is a smaller, more clearly justified guard than the original draft's. -- The pressure to extract `scripts/ci/lib/workflow-yaml-asserts.sh` out of `check-codeql-parity.sh` is now a plain, optional DRY nicety (the branch-check helper is still needed by both scripts) rather than something load-bearing for the config-parity story โ€” see ยง3.5. +Confirmed via full reads of `docker-build.yml` and `orthrus-build.yml` (Decision 2) that neither depends on GoReleaser's build/archive/nfpm output โ€” Charon's Docker images are built directly by `docker-build.yml`'s own multi-stage `Dockerfile`, with version injected via ldflags independently and identically to what `.goreleaser.yaml`'s `builds.ldflags` section does (Dockerfile lines ~250-286 vs. `.goreleaser.yaml`'s `builds[0].ldflags`). `.goreleaser.yaml`'s own header comment already self-documents as unused: *"builds, archives, and nfpms... kept for potential future use but are not currently utilized."* -**Why not stop here and also drop the parity guard entirely?** Because two failure-independent invariants remain worth checking even with zero config duplication: (1) that the additive `SEMGREP_SARIF_OUTPUT` hook stays present in the script (a future refactor of `semgrep-scan.sh` could drop it without realizing CI depends on it), and (2) that the workflow keeps *delegating* to the script for both passes rather than a future edit reintroducing an inline `semgrep scan` call (e.g. someone "simplifying" the SARIF step by hand and accidentally dropping an `--exclude`). Both are cheap, structural, grep-level checks โ€” proportionate, not over-engineering, and much smaller than the original draft's guard (ยง3.4). +The only *other* thing `release-goreleaser.yml` does is call `scripts/generate-changelog.sh` (line 77) โ€” but that script is independently invoked by `nightly-build.yml` too, and depends only on git tags, not on GoReleaser itself, so removing the workflow does not remove changelog-generation capability from anywhere it currently actually runs. -This section supersedes the original ยง2.7 ("Parity guard: warranted, and why") from the first draft. +**No nfpm/package-distribution consumer exists** โ€” grep across `docs/`, `.github/workflows/`, and `Dockerfile` for `nfpm`/`.deb`/`.rpm` distribution steps outside `.goreleaser.yaml` itself returns nothing; Charon ships exclusively as Docker images per `ARCHITECTURE.md`'s stated deployment model. -### 2.8 Documentation review +## Decision 6 โ€” Fate of the "PR-2 release promotion guard" -- **`SECURITY.md`** (`## Security Audits & Scanning` โ†’ `### Automated Scanning` table, lines 992-1020): lists Trivy, CodeQL, govulncheck, golangci-lint (gosec), npm audit, and a `### Scanning Workflows` subsection describing each workflow file's purpose (`docker-build.yml`, `supply-chain-verify.yml`, `security-weekly-rebuild.yml`, PR-specific scanning). **This is the primary file to update** โ€” add a `Semgrep` row to the table and a new `**Semgrep SAST Scan**` paragraph under `### Scanning Workflows` describing `.github/workflows/semgrep.yml`. -- **`docs/security.md`**: verified by full-text search (`codeql|trivy|scan|pipeline`, no matches) โ€” this file is entirely about the Cerberus runtime security feature (CrowdSec/WAF/access lists), unrelated to the CI/SAST scanning pipeline. **No change needed here.** -- **`ARCHITECTURE.md` (Supervisor round 2, required change 2 โ€” added to scope):** `CLAUDE.md` requires `ARCHITECTURE.md` updates for changes touching security architecture, and this file already documents the CI security-scanning stack in three places that must be kept current: - - Line 166, tech-stack table: `| **Security Scanning** | Trivy + Grype | Latest | Vulnerability detection |` โ€” append Semgrep, e.g. `Trivy + Grype + Semgrep`. - - Line 1376, CI Jobs list: `3. **Security:** Trivy, CodeQL, Grype, Govulncheck` โ€” append `, Semgrep`. - - Lines 1498-1501, "Container Scanning" components list (`Trivy: ...`, `Grype: ...`, `CodeQL: ...`) โ€” add a fourth line, `Semgrep: Static analysis for security anti-patterns (Go, JS/TS, React, secrets, Dockerfile)`, consistent with the existing one-line-per-tool style. - - This is now part of Commit 3's scope (ยง6) and Acceptance Criteria (ยง5), alongside `SECURITY.md`. +**Decision: the gate is a deliberate, purpose-built temporary safety mechanism (not a misconfigured accident), whose guarded purpose has since been satisfied and whose host workflow is being retired anyway โ€” so it is removed as a natural consequence of Decision 5, not silently dropped on its own merits.** -### 2.9 Ignore-file / build-file review (explicit confirmation per `CLAUDE.md`) +Evidence trail: +- `git log -S"Enforce PR-2 release promotion guard"` traces the gate's introduction to commit `834b27f2` / `45458df1`, `"chore: Add Caddy compatibility gate workflow and related scripts; enhance SMTP settings tests"`, dated 2026-02-23. That same commit also adds `.github/workflows/caddy-pr1-compat.yml` and `docs/reports/caddy-pr1-compatibility-matrix.md`. +- `docs/reports/caddy-security-posture.md`, also dated 2026-02-23, is explicitly titled **"PR-2 Security Patch Posture and Advisory Disposition"** โ€” a Caddy 2.11.x upgrade security review (patch retention/retirement decisions for `expr`, `ipstore`, `nebula`; CVE/GHSA disposition table) that is **unrelated to any numbered pull request in this repo's PR history** โ€” "PR-2" here names a phase of a specific historical security workstream (Caddy version-bump security review), not a generic or accidental label. +- That doc's own closure statement: *"PR-2 posture decisions are review-ready: patch disposition is explicit, admin API assumptions are enforced, and rollback remains deterministic."* โ€” i.e., the gate's guarded condition (finish the Caddy PR-2 security review before letting GoReleaser cut a publishable release) **was satisfied the same day the gate was added.** +- Nobody ever flipped `CHARON_PR2_GATES_PASSED=true` afterward โ€” confirmed via `REPO_VARS_JSON: {}` on every subsequent `release-goreleaser.yml` run. This is most plausibly an oversight (the review closed, but the repo-variable flip was a separate manual step nobody circled back to) rather than a deliberate ongoing hold, since the closure doc gives no indication the gate was meant to stay engaged indefinitely. -- **`.gitignore`**: already contains a blanket `*.sarif` ignore (line 189) with a narrow `!scripts/security/testdata/*.sarif` carve-out (line 190). A new `semgrep-results.sarif` file in the repo root matches the existing wildcard โ€” **no change needed**. -- **`.dockerignore`**: already excludes `*.sarif` (line 179) โ€” irrelevant anyway, since this is a CI-only workflow change with no Docker image content change โ€” **no change needed**. -- **`.codecov.yml`**: workflow-only YAML change, produces no coverage-relevant files โ€” **no change needed**. -- **Any `Dockerfile`**: not touched; Semgrep runs in its own CI container, never inside the Charon application image โ€” **no change needed**. +**Recommendation for the user**: this is not evidence of a security requirement that needs to be re-implemented elsewhere โ€” it was scoped to one specific, already-closed security review. Removing it alongside the rest of `release-goreleaser.yml` is safe. If the user wants a similar "hold releases pending a security sign-off" mechanism for *future* security reviews, that would be a new, forward-looking control to design separately โ€” explicitly flagged here as a possible follow-up, not something this plan implements. -(Approved as-is by Supervisor round 2 โ€” no changes in this revision.) +**A second, independent argument for removal, stronger than the intent-inference above**: even setting aside whether the Caddy-review closure was "meant" to release the gate, Decision 5 establishes that GoReleaser will **never again attempt to publish anything** โ€” the workflow it lives in is being deleted outright, not merely disabled. A gate that guards an action which no longer exists has nothing left to guard, independent of any judgment call about the gate's original intent or whether it was ever properly released. This makes the removal safe on structural grounds alone, not just on the historical-intent grounds argued above. -### 2.10 Commit scope: `feat:` vs `feat(security):` +# Additional Findings Beyond the Six Required Points -Per `CLAUDE.md`, `feat:`/`fix:`/`perf:` trigger Docker builds; `chore:` skips them, and `feat(security):`/`fix(security):` is reserved for "genuinely security-relevant... real vulnerability fixes, new protective mechanisms." **Decision:** the workflow-adding commit (Commit 2, ยง6) qualifies as a **new protective mechanism** โ€” it is, definitionally, new automated vulnerability/anti-pattern detection gating merges โ€” so it uses `feat(security):`, not plain `feat:`. Commit 1 (the additive `semgrep-scan.sh` hook + parity guard) also touches genuine security tooling directly and is scoped `feat(security):` for the same reason. Commit 3 (docs) stays `docs:`, matching repo convention for documentation-only changes regardless of what they document. Per `CLAUDE.md`'s vagueness requirement for `(security)` subjects, none of these commit subjects name a vulnerability class or attack vector โ€” they describe the category ("add CI security scanning coverage") only, which is appropriate here since this isn't a vulnerability fix in the first place, just extra coverage. +These surfaced during the mandated research and materially affect the design, so they're resolved here rather than left implicit. -**Nuance retained from the original draft:** this change touches zero Docker-build-relevant paths (no `Dockerfile`, no backend/frontend source), so the triggered Docker build (a side effect of `feat`/`feat(security)` prefixes repo-wide) is harmless but expected โ€” not a sign something is wrong with a "just workflow files + one shell script" PR. +## A. `auto-changelog.yml` / `.github/release-drafter.yml` are redundant with release-please ---- +Not named in the original task brief, but directly in-scope: it's a `.github/workflows/*` file in the exact pipeline being migrated, and it will actively conflict with release-please if left running (both listen on `release: types: [published]`-adjacent events and both try to own "the changelog for this release"). `release-drafter`'s own tag-template (`v$NEXT_PATCH_VERSION`) already never bumps minor/major regardless of label โ€” a pre-existing bug, further weakening the case for keeping it. -## 3. Technical Specifications +**Decision: remove `.github/workflows/auto-changelog.yml` and `.github/release-drafter.yml` in this PR.** Release-please's standing release PR (with its auto-updated body) replaces the "always-fresh draft changelog" function these two files provide. -### 3.0 Change to `scripts/pre-commit-hooks/semgrep-scan.sh` (additive, in scope per ยง2.7) +## B. `CHANGELOG.md` conflict -**File:** `scripts/pre-commit-hooks/semgrep-scan.sh` -**Change:** insert the `OUTPUT_FLAGS` branch (ยง2.7) immediately before the existing `semgrep scan \` invocation, and replace the invocation's `--error` line with `"${OUTPUT_FLAGS[@]}"`. No other line in the file changes โ€” the `command -v semgrep` check, the `SEMGREP_CONFIG` override branch, and the `TARGETS` construction are byte-identical to today. -**Backward compatibility:** every existing call site (`lefthook.yml`'s `pre-commit.semgrep`, `security-full.semgrep`, `Makefile`'s `security-local`) never sets `SEMGREP_SARIF_OUTPUT`, so `OUTPUT_FLAGS=(--error)` unconditionally for all of them โ€” identical exit-code and output behavior to the pre-change script. -**New behavior (CI-only):** `SEMGREP_SARIF_OUTPUT= bash scripts/pre-commit-hooks/semgrep-scan.sh [targets...]` scans with the same configs/exclusions but emits SARIF to `` instead of hard-failing on findings. +Charon's root `CHANGELOG.md` is hand-curated (Keep a Changelog format, multi-line rich entries, issue cross-references, 600 lines of history). Release-please's `go` release-type, by default, prepends its own auto-generated entries to whatever file `changelog-path` points at (default `CHANGELOG.md`). -### 3.1 New file: `.github/workflows/semgrep.yml` +**Decision**: set `"skip-changelog": true` on the `"."` package in `release-please-config.json`. Per the release-please JSON schema (`schemas/config.json`, fetched this session): *"Skip generating a changelog for this package. Defaults to `false`."* This stops release-please from touching `CHANGELOG.md` at all, preserving the existing hand-curated file untouched. GitHub Release notes generation is understood to be independent of the changelog-file-write path (the Release body is built from the same underlying commit grouping, separately from whether it's also written to a file) โ€” **this exact interaction is not explicitly documented** in the pages fetched this session, so it is flagged in Manual Post-Merge Follow-Ups as something to positively confirm on the first real release-please run (does the created GitHub Release still get a populated body with `skip-changelog: true`?). -No API/DB/frontend surface โ€” this is CI/YAML only. Full structural spec below (devops agent should treat this as the authoritative shape; exact YAML syntax is implementer's to finalize, but every element listed must be present). +## C. Tag-naming default is a real breakage risk โ€” must be pinned explicitly -**Workflow name:** `Semgrep - SAST Scan` +Per the release-please JSON schema (fetched this session): `include-component-in-tag` **defaults to `true`** ("When tagging a release, include the component name as part of the tag"). For a single non-monorepo package at `"."`, this risks producing a tag like `charon-v0.37.0` instead of the bare `v0.37.0` every existing consumer expects (`orthrus-build.yml`'s `tags: ['v*']` trigger, `generate-changelog.sh`'s `git tag -l 'v*'` scan, `update_service.go`'s expectations, and every pre-existing tag in the repo's own history back to `v0.1.0`-style tags). -**Triggers:** -```yaml -on: - pull_request: - branches: [main, nightly, development] - push: - branches: [main, nightly, development] - workflow_dispatch: - schedule: - - cron: '0 4 * * 1' # Mondays 04:00 UTC โ€” offset 1h after CodeQL's 03:00 to avoid runner contention -``` +**Decision**: explicitly set `"include-component-in-tag": false` in `release-please-config.json`. Do not rely on whatever component-name-derivation-for-an-unnamed-root-package default behavior release-please falls back to โ€” pin it. Flagged as a **must-verify-on-first-live-run** item (see Manual Post-Merge Follow-Ups): confirm the first release-please-created tag is exactly `v`, no prefix/suffix. -**Concurrency:** -```yaml -concurrency: - group: ${{ github.workflow }}-${{ github.event_name }}-${{ github.head_ref || github.ref_name }} - cancel-in-progress: true +## D. Pre-1.0 major-version-bump behavior must be pinned explicitly + +`auto-versioning.yml`'s current design deliberately disables automatic major-version bumps ("Major version bumps are intentionally disabled in automation to prevent accidents" โ€” its own header comment; `major_pattern` is set to a regex that can never match). Release-please instead supports major bumps automatically via `!` suffix or `BREAKING CHANGE:` footer conventions, gated pre-1.0 by two schema options (`bump-minor-pre-major`, `bump-patch-for-minor-pre-major`) whose **schema definitions carry no explicit documented default** (confirmed by direct inspection of `schemas/config.json` this session โ€” both properties have a `description` but no `default` key shown). + +**Decision**: set both `"bump-minor-pre-major": true` and `"bump-patch-for-minor-pre-major": true` explicitly in `release-please-config.json`, regardless of what the undocumented actual default turns out to be. This guarantees a `feat!:`/`BREAKING CHANGE:` commit at the current `v0.36.5` bumps to `v0.37.0` (matching the existing "major bumps require a deliberate manual tag, never automatic" philosophy) rather than silently jumping to `v1.0.0`. This is materially safer than trusting an unconfirmed default and is worth the two explicit lines. + +## E. `chore:`-scoped commits will not trigger a Docker build on `main` + +`docker-build.yml`'s `setup` job (lines ~170-173) already skips the real build when the head commit or PR title matches `^chore:` or `^chore\(deps`. Since every commit in this PR is `chore:`-scoped (pure CI/CD config), merging it to `main` will not trigger a Docker build โ€” expected and desired, not a gap to fix. + +# Technical Specifications + +## New Files + +### `release-please-config.json` (repo root) + +```json +{ + "$schema": "https://raw.githubusercontent.com/googleapis/release-please/main/schemas/config.json", + "release-type": "go", + "include-component-in-tag": false, + "bump-minor-pre-major": true, + "bump-patch-for-minor-pre-major": true, + "pull-request-header": "Here's what's new in Charon", + "pull-request-footer": "Merge this PR to cut the release.", + "packages": { + ".": { + "skip-changelog": true + } + } +} ``` -(Identical pattern to `codeql.yml`.) -**Permissions โ€” declared at both workflow (top) level and job level, identically, matching `codeql.yml`'s style (Supervisor round 2, non-blocking fix):** -```yaml -permissions: - contents: read - security-events: write - actions: read - pull-requests: read +### `.release-please-manifest.json` (repo root) + +```json +{ + ".": "0.36.5" +} ``` -This exact block appears twice: once at the workflow top level (sibling of `on:`/`concurrency:`), and again inside `jobs.semgrep-scan.permissions` (ยง3.2). -### 3.2 Job: `semgrep-scan` +Seeded to the real latest tag at plan-authoring time (`git tag --sort=-v:refname | head -1` โ†’ `v0.36.5`). **Implementer note**: re-run that command immediately before implementation and use whatever the actual latest tag is at that time โ€” do not blindly copy `0.36.5` if additional tags have landed on `main` since this plan was written. -**Correction (Supervisor round 2, required change 1):** the pinned image is inlined directly as a literal string in `container.image` โ€” `jobs..container` does not have access to the `env` context per GitHub Actions' documented context-availability rules, so a workflow-level `env:` indirection (as drafted originally) would not resolve at all. Inlining is the only correct form, not a fallback. +### `.github/workflows/release-please.yml` ```yaml +name: release-please + +on: + push: + branches: [main] + +permissions: + contents: write + pull-requests: write + jobs: - semgrep-scan: - name: Semgrep SAST Scan + release-please: runs-on: ubuntu-latest - timeout-minutes: 15 - permissions: - contents: read - security-events: write - actions: read - pull-requests: read - container: - image: semgrep/semgrep:1.173.0@sha256:67319956da3dcb58baf5b322899c15458e3963e7018a86aeeb5cd224e69cb77a # semgrep/semgrep 1.173.0 + steps: + - uses: googleapis/release-please-action@45996ed1f6d02564a971a2fa1b5860e934307cf7 # v5 + with: + config-file: release-please-config.json + manifest-file: .release-please-manifest.json ``` -### 3.3 Steps - -1. **Checkout repository** - ```yaml - - name: Checkout repository - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 - with: - ref: ${{ github.ref }} - ``` - (Same SHA already pinned and in active use in `codeql.yml` โ€” reuse, don't re-pin a different version.) - -2. **Fix git safe.directory for container user** (mitigates the "dubious ownership" issue documented in Semgrep's own GHA KB article, ยง2.5): - ```yaml - - name: Configure git safe.directory - run: git config --global --add safe.directory "$GITHUB_WORKSPACE" - ``` - -3. **Verify Semgrep parity guard** (ยง3.4): - ```yaml - - name: Verify Semgrep parity guard - run: bash scripts/ci/check-semgrep-parity.sh - ``` - -4. **Print Semgrep version** (cheap sanity check that the pinned image actually resolves to the expected CLI version โ€” catches a bad digest pin immediately and legibly, rather than surfacing as a confusing downstream scan failure): - ```yaml - - name: Verify Semgrep version - run: semgrep --version - ``` - -5. **Run Semgrep (SARIF output, non-blocking)** โ€” calls the real script (ยง2.7/ยง3.0) with the new opt-in var; no duplicated config list. - ```yaml - - name: Run Semgrep (SARIF output) - id: semgrep_sarif - continue-on-error: true - env: - SEMGREP_SARIF_OUTPUT: semgrep-results.sarif - run: bash scripts/pre-commit-hooks/semgrep-scan.sh - ``` - `continue-on-error: true` because this pass must not block the job even if semgrep itself errors โ€” the SARIF file's presence is checked explicitly next (step 6), and the actual gate is step 8, not this step. - -6. **Check Semgrep SARIF output exists** (mirrors `security-pr.yml`'s `Check Trivy SARIF output exists` step โ€” this was an orphaned reference in the first draft of this plan; it is now a real, numbered step): - ```yaml - - name: Check Semgrep SARIF output exists - id: semgrep_sarif_check - if: always() - run: | - if [ -f semgrep-results.sarif ]; then - echo "exists=true" >> "$GITHUB_OUTPUT" - else - echo "exists=false" >> "$GITHUB_OUTPUT" - echo "No Semgrep SARIF output found; skipping SARIF upload" - fi - ``` - -7. **Upload Semgrep SARIF to GitHub Security** (gated on step 6's output rather than blindly attempting the upload): - ```yaml - - name: Upload Semgrep SARIF to GitHub Security - if: always() && steps.semgrep_sarif_check.outputs.exists == 'true' - uses: github/codeql-action/upload-sarif@ff2f1c621b7f889edc0d3c761ac2e6a3f8cdb0dd # v4.37.7 - with: - sarif_file: semgrep-results.sarif - category: semgrep - continue-on-error: true - ``` - (Reuses the exact SHA already pinned for this purpose in `security-pr.yml` โ€” no new pin to introduce or maintain.) - -8. **Run Semgrep (hard-fail gate)** โ€” calls the same script, this time with the default (unset `SEMGREP_SARIF_OUTPUT`) path, i.e. its normal `--error` behavior: - ```yaml - - name: Run Semgrep (hard-fail gate) - run: bash scripts/pre-commit-hooks/semgrep-scan.sh - ``` - This is the literal `security-full` invocation (ยง2.1) โ€” same script, same default full-repo targets, same `--error` flag, run a second time (this time without the SARIF env var) so failure here genuinely gates the job. If this step fails, the job fails, blocking the PR/branch โ€” this is the CI-independent reproduction of the local "green" signal the feature exists to deliver. SARIF upload (step 7) has already completed by this point, so a gate failure here does not suppress the informational upload โ€” order matters and is intentional. - -9. **Upload SARIF artifact** (retention, matches `security-pr.yml`'s `Upload scan artifacts` step): - ```yaml - - name: Upload SARIF artifact - if: always() && steps.semgrep_sarif_check.outputs.exists == 'true' - uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v4.4.3 - with: - name: semgrep-sarif-${{ github.run_id }} - path: semgrep-results.sarif - retention-days: 14 - continue-on-error: true - ``` - -10. **Job summary** - ```yaml - - name: Create job summary - if: always() - run: | - { - echo "## Semgrep SAST Scan Results" - echo "" - echo "**Rulesets**: p/golang, p/javascript, p/typescript, p/react, p/secrets, p/dockerfile" - echo "**Severity Gate**: ERROR, WARNING (--error)" - if [ "${{ job.status }}" == "success" ]; then - echo "PASSED: no blocking Semgrep findings" - else - echo "FAILED: Semgrep reported blocking findings โ€” see step logs and the Security tab" - fi - } >> "$GITHUB_STEP_SUMMARY" - ``` - -### 3.4 New file: `scripts/ci/check-semgrep-parity.sh` - -**Revised, smaller scope (per ยง2.7's design change)** โ€” modeled on `scripts/ci/check-codeql-parity.sh`'s approach (grep/structural assertions, not full YAML parsing), but no longer needs to compare two independent config lists, because ยง3.0's design means there's only one config list in the whole repo (in `semgrep-scan.sh`) and the workflow only ever delegates to it. Exits non-zero with an `::error title=Semgrep parity drift::` annotation on any mismatch. - -**Checks performed:** - -1. Required files exist: `.github/workflows/semgrep.yml`, `scripts/pre-commit-hooks/semgrep-scan.sh`. -2. Assert `scripts/pre-commit-hooks/semgrep-scan.sh` still contains the string `SEMGREP_SARIF_OUTPUT` โ€” the additive CI hook (ยง3.0) must not be silently removed by a future edit to the script that forgets CI depends on it. -3. Assert `.github/workflows/semgrep.yml` contains **two** distinct delegating calls to the real script, not a reimplemented/inlined `semgrep scan ...` invocation: - - a call with `SEMGREP_SARIF_OUTPUT` set (the reporting pass, step 5) โ€” e.g. assert both the literal strings `SEMGREP_SARIF_OUTPUT` and `scripts/pre-commit-hooks/semgrep-scan.sh` appear within the same step block; - - a bare `bash scripts/pre-commit-hooks/semgrep-scan.sh` call with no env override (the gate pass, step 8). - - This is the direct analogue of `check-codeql-parity.sh`'s "shared blocking logic must live in exactly one place" check (that script's lines enforcing `SHARED_GATE_SCRIPT` usage) โ€” applied here to prevent a future edit from "simplifying" either step by inlining `semgrep scan` directly, which would silently reintroduce the duplicated-config problem ยง2.7 eliminated. -4. Assert `.github/workflows/semgrep.yml`'s pinned image reference matches the pattern `semgrep/semgrep:[0-9]+\.[0-9]+\.[0-9]+@sha256:[0-9a-f]{64}` (tag + digest both present โ€” catches an accidental un-pin, e.g. someone changing it to `semgrep/semgrep:latest` during a quick edit). -5. Assert `pull_request`/`push` trigger branches in `semgrep.yml` are `[main, nightly, development]`, reusing `check-codeql-parity.sh`'s existing `ensure_event_branches_semantic` helper pattern (ยง3.5). - -Note what this script **no longer does**, relative to the first draft: it does not enumerate or compare `--config`/`--exclude`/`--exclude-rule` values between two files, because after ยง3.0's change there is only one file that defines them. - -**Where it's invoked:** -- `.github/workflows/semgrep.yml` step 3 (ยง3.3), analogous to `codeql.yml`'s "Verify CodeQL parity guard" step. -- Not wired into `lefthook.yml` in this PR โ€” consistent with existing precedent: `check-codeql-parity.sh` is also CI-only today, invoked directly from `codeql.yml` and not from any lefthook stage. Noted as a possible follow-up, not a gap introduced by this plan. - -### 3.5 Shared helper extraction (optional, DRY nicety โ€” from ยง3.4 item 5) - -`check-semgrep-parity.sh` is now the second script needing the branch-list assertion logic (`ensure_event_branches` / `ensure_event_branches_with_yq` / `ensure_event_branches_semantic`) that currently lives only in `check-codeql-parity.sh`. Recommend extracting it into `scripts/ci/lib/workflow-yaml-asserts.sh`, sourced by both scripts via `source "$(dirname "${BASH_SOURCE[0]}")/lib/workflow-yaml-asserts.sh"`, per `CLAUDE.md`'s "consolidate after second occurrence" DRY guideline. This is a pure refactor of existing, already-tested logic โ€” low risk. Unlike the first draft, this extraction is no longer load-bearing for anything (the config-parity story doesn't depend on it, since there's no config duplication left to compare) โ€” it is a legitimate but strictly optional cleanup. If time-boxed out of this PR, note it as a follow-up rather than skipping silently. - -### 3.6 SARIF category naming - -`category: semgrep` for the `upload-sarif` step (ยง3.3 step 7) โ€” single, flat category since (unlike CodeQL's per-language matrix) there is only one Semgrep job/run per commit, no need for a parameterized category string. - -### 3.7 Error handling / edge cases - -| Scenario | Behavior | -|---|---| -| Pinned image digest becomes invalid/removed from registry (rare, but Docker Hub retention policies exist) | Job fails at container-pull time with a clear GitHub Actions infra error, not a silent skip. Remediation: re-resolve digest, bump the pin โ€” a normal dependency-bump PR, same as any other pinned SHA bump in this repo. | -| SARIF step (step 5) itself crashes (e.g. semgrep internal error, not a rule finding) | `continue-on-error: true` on step 5 means the job continues; step 6 explicitly checks for the SARIF file's existence and sets an output consumed by steps 7 and 9, so a missing file cleanly skips upload rather than `upload-sarif` failing opaquely on a missing path. | -| Hard-fail gate step (step 8) fails legitimately (real findings) | Job fails, PR shows a red check, `$GITHUB_STEP_SUMMARY` still renders (step 10 runs on `if: always()`), SARIF is still uploaded to the Security tab (step 7 already ran before step 8 โ€” order is intentional: SARIF upload must happen *before* the blocking step so a gate failure doesn't skip the informational upload). | -| Renovate later proposes bumping the pinned `semgrep/semgrep` image tag/digest | Handled like any other Renovate-tracked pin, via `.github/renovate.json` โ€” devops agent should confirm at implementation time whether Renovate's Docker-image datasource already picks up `container: image:` refs in workflow YAML by default, or needs an explicit entry added to `.github/renovate.json`; note as a follow-up if configuration is needed, not a blocker for this PR. | -| A future edit to `semgrep-scan.sh` removes the `SEMGREP_SARIF_OUTPUT` hook, or an edit to `semgrep.yml` reintroduces an inline `semgrep scan` call instead of delegating | `check-semgrep-parity.sh` fails CI on the very next PR that makes either change, per ยง3.4 items 2-3. | - ---- +Mirrors `/projects/go_notify_yourself/.github/workflows/release-please.yml` exactly, including the same pinned SHA. **Implementer note**: verify this SHA still resolves to `v5` (or a newer tag) immediately before implementation โ€” supply-chain pins should reflect the current best-available release, not be copied stale. -## 4. Implementation Plan +## Modified Files -This is a CI/DevOps-only change plus one small, additive shell-script change. There is no Playwright/E2E surface (no user-facing behavior changes), no backend implementation, no frontend implementation. The phase structure below is adapted accordingly โ€” **the `devops` agent implements this directly; no handoff to backend-dev, frontend-dev, or playwright-dev is needed.** +| File | Change | +|---|---| +| `VERSION.md` | Full rewrite: remove "Canonical Release Process (Tag-Derived CI)" section describing `release-goreleaser.yml`/`docker-build.yml` as jointly building release artifacts from the tag (false, per Decision 2/5). Replace with a description of the release-please PR-based flow: commits land on `main` โ†’ release-please maintains a standing `chore(main): release X.Y.Z` PR โ†’ merging that PR tags + publishes the GitHub Release โ†’ `orthrus-build.yml` picks up the tag independently. Remove the "Legacy/Optional `.version` Path" section (Decision 4). Remove the "release-drafter workflow" changelog-generation mention (Additional Finding A). Keep the Container Image Tags / Nightly Versioning Format sections unchanged (still accurate, untouched by this migration). | +| `lefthook.yml` | Remove the `check-version-match: { glob: ".version", run: "bash scripts/check-version-match-tag.sh" }` hook entry (~line 99). | +| `.gitignore` | Remove the `# GoReleaser` section (`dist/` ignore rule, ~lines 163-167). | +| `.dockerignore` | Remove the `.goreleaser.yaml` line (~line 13). | +| `ARCHITECTURE.md` | **Was missing from the first draft of this plan โ€” added per Supervisor review.** Per CLAUDE.md's own rule ("update `ARCHITECTURE.md` when making changes to... deployment model... integration points"), and because it explicitly names a file this plan deletes: the "Release Workflow" section (~lines 1464-1479) opens with a 10-step "Automated Release (GitHub Actions)" list whose step 1 is `"Trigger: Push tag v1.2.0"` โ€” this describes the tag-push-triggers-everything model this migration retires (steps 2-8 describe `docker-build.yml`'s real, still-accurate build/scan/sign/publish pipeline, which is **not** actually tag-triggered per Decision 2's finding โ€” this section conflates `docker-build.yml`'s branch-push pipeline with a tag-triggered release flow that doesn't exist as described). Line 1479 states `scripts/generate-changelog.sh` "runs during the same `release-goreleaser.yml` workflow" โ€” that workflow is deleted by this PR (the script also independently runs from `nightly-build.yml`, per Research Findings, so this line is doubly inaccurate even before this migration). **Rewrite scope**: replace the "Trigger: Push tag" framing with the release-please push-to-main โ†’ standing-PR โ†’ merge-to-tag flow (mirror the Data Flow diagram below); correct the `generate-changelog.sh` sentence to name `nightly-build.yml` (its one remaining real caller) instead of the deleted workflow; leave steps 2-8's description of `docker-build.yml`'s actual build/scan/sign/publish pipeline untouched (still accurate, just re-anchor which trigger kicks it off). Assigned to Commit 6. | -### Phase 1 โ€” Foundation (script hook + parity guard + optional shared lib) -- Apply the additive `SEMGREP_SARIF_OUTPUT` change to `scripts/pre-commit-hooks/semgrep-scan.sh` (ยง3.0). -- Write `scripts/ci/check-semgrep-parity.sh` (ยง3.4). -- Optionally extract `scripts/ci/lib/workflow-yaml-asserts.sh` from `check-codeql-parity.sh` (ยง3.5); if done, refactor `check-codeql-parity.sh` to source it and verify it still passes unchanged. -- Validation gate: run the existing `semgrep` lefthook hooks locally (`lefthook run pre-commit` touching a Go/JS file, or `lefthook run security-full`) to confirm the script's default (`SEMGREP_SARIF_OUTPUT` unset) behavior is byte-identical to pre-change โ€” this is the regression check for ยง3.0's edit. `shellcheck scripts/pre-commit-hooks/semgrep-scan.sh scripts/ci/check-semgrep-parity.sh` (+ `scripts/ci/lib/workflow-yaml-asserts.sh` if extracted). If the shared lib was extracted, `bash scripts/ci/check-codeql-parity.sh` still exits 0 (regression check on that refactor). +## Deleted Files -### Phase 2 โ€” Workflow file -- Write `.github/workflows/semgrep.yml` per ยง3.1-ยง3.3 (10 steps, including the SARIF-existence-check as a first-class step, not an orphaned reference). -- Validation gate: `actionlint .github/workflows/semgrep.yml` (tool already required per `lefthook.yml`'s `actionlint` hook, ยง2 header comment listing required tools). `bash scripts/ci/check-semgrep-parity.sh` now passes against the real files. YAML syntax sanity via `yq eval '.' .github/workflows/semgrep.yml >/dev/null` or equivalent. -- **Live GitHub Actions execution cannot be validated locally** โ€” the actual scan run (image pull, semgrep execution against the real repo, SARIF upload, gate pass/fail) is confirmed only once the PR opens and the workflow triggers on `pull_request`. Note this explicitly in the PR description as a manual verification step, not a local DoD gate. +| File | Reason | +|---|---| +| `.github/workflows/auto-versioning.yml` | Replaced by `release-please.yml` (Decision 1-3). | +| `.github/workflows/release-goreleaser.yml` | Never succeeded; no downstream consumer (Decision 5). | +| `.goreleaser.yaml` | Unused build/archive/nfpm config; version injection already duplicated in `Dockerfile` directly (Decision 5). | +| `.github/workflows/auto-changelog.yml` | Redundant with release-please's own PR/changelog mechanism (Additional Finding A). | +| `.github/release-drafter.yml` | Config for the file above; removed alongside it. | +| `.version` | Stale, non-canonical, superseded by `.release-please-manifest.json` (Decision 4). | +| `scripts/check-version-match-tag.sh` | Self-deprecated, circularly "replaced" by a wrapper that calls it, superseded by release-please's manifest (Decision 4). | +| `.github/skills/utility-version-check-scripts/run.sh` | Wraps the script above; dead once its target is gone. | +| `.github/skills/utility-version-check.SKILL.md` | Documents the now-removed skill. | + +**Partial edits (file kept, dead reference removed)** โ€” full detail and reasoning in Decision 4 and the Commit Slicing Strategy's Commit 5 row; listed here only as a pointer so this table isn't mistaken for the complete blast radius: + +| File | Edit | +|---|---| +| `.github/skills/README.md` | Remove the `utility-version-check` row (~line 72) and naming-example bullet (~line 267). | +| `.vscode/tasks.json` | Remove the "Utility: Check Version Match Tag" task block (~lines 694-698). | +| `.github/skills/utility-bump-beta.SKILL.md` | Remove the dead `utility-version-check` cross-link (~line 186). | +| `scripts/generate-changelog.sh` | Fix the stale header-comment reference to `release-goreleaser.yml` (line 5) โ€” points at `nightly-build.yml` instead. | +| `CLAUDE.md` | **Edited by this PR, per explicit user authorization** โ€” remove the `utility-version-check` Skills-table row (line 261). Folded into Commit 5. | -### Phase 3 โ€” Documentation -- Update `SECURITY.md` per ยง2.8: add Semgrep row to the `### Automated Scanning` table, add a `**Semgrep SAST Scan**` paragraph to `### Scanning Workflows` describing `.github/workflows/semgrep.yml`'s trigger shape and what it covers. -- Update `ARCHITECTURE.md` per ยง2.8: the three call-outs at lines 166, 1376, and 1498-1501. -- Validation gate: `markdownlint SECURITY.md ARCHITECTURE.md` (tool already required per `lefthook.yml` header comment). +## Data Flow: Before vs. After -### Phase 4 โ€” Integration validation -- `lefthook run pre-commit` (full local hook suite, including the existing `actionlint` and `semgrep` hooks, to confirm nothing in this change breaks existing local gates). -- `bash scripts/ci/check-codeql-parity.sh` (if refactored) and `bash scripts/ci/check-semgrep-parity.sh` both green. -- Manual review of the rendered `semgrep.yml` against `codeql.yml`/`security-pr.yml` for pinning-comment consistency (every third-party `uses:` has a SHA + version comment; the container image has tag + digest inlined, not via `env`). +**Before:** +``` +push to main (non-chore commit) + -> docker-build.yml runs (branch push trigger) + -> auto-versioning.yml runs (workflow_run: on docker-build.yml completion) + -> paulhatch/semantic-version computes next tag + -> shell/grep builds release body from commit messages + -> softprops/action-gh-release creates tag + GitHub Release + -> release-goreleaser.yml runs (tag push trigger) -> FAILS at PR-2 gate, publishes nothing + -> orthrus-build.yml runs (tag push trigger) -> builds + publishes semver-tagged orthrus image + -> auto-changelog.yml runs (release published trigger) -> release-drafter updates a draft release +``` -### Phase 5 โ€” PR & CI confirmation -- Open PR; confirm `semgrep.yml` actually triggers on the PR event, pulls the pinned image successfully, produces a SARIF upload visible under the repo's Security โ†’ Code scanning alerts (filtered by tool "Semgrep"), and that the hard-fail gate step correctly reflects the current repo's Semgrep cleanliness (expected: green, since this is the same ruleset the repo already passes locally today). -- If CI surfaces findings the local run didn't (e.g. a stale local semgrep binary/ruleset that had drifted below the pinned CI version), that is itself the feature working as intended โ€” resolve findings on their merits, not by weakening the pin. +**After:** +``` +push to main (any commit) + -> docker-build.yml runs (branch push trigger, skips build body if chore:) [unchanged] + -> release-please.yml runs (push trigger, independent of docker-build.yml) + -> release-please-action opens/updates a standing "chore(main): release X.Y.Z" PR + (accumulates all releasable commits since the last tag; no PR yet if nothing releasable) + +[separately, whenever a human/bot merges that standing release PR] + -> release-please-action creates the git tag (v, bare per include-component-in-tag:false) + -> release-please-action creates the GitHub Release (skip-changelog:true, so CHANGELOG.md untouched) + -> orthrus-build.yml runs (tag push trigger) -> builds + publishes semver-tagged orthrus image [unchanged] + -> generate-changelog.sh (next nightly-build.yml run) picks up the new tag via `git tag -l 'v*'` [unchanged] +``` ---- +## Error Handling / Edge Cases -## 5. Acceptance Criteria - -1. `.github/workflows/semgrep.yml` exists, triggers on `pull_request`/`push` to `[main, nightly, development]`, `workflow_dispatch`, and a weekly `schedule`. -2. Semgrep runs inside a `container:` whose `image:` is the pinned string `semgrep/semgrep:@sha256:` inlined directly (no `env:` indirection) โ€” no floating tag, no `@latest`. -3. `permissions:` is declared identically at both the workflow (top) level and the job level. -4. `scripts/pre-commit-hooks/semgrep-scan.sh` carries exactly the additive `SEMGREP_SARIF_OUTPUT` change described in ยง3.0 โ€” its binary/version-resolution logic and its `--config`/`--exclude`/`--exclude-rule` values are otherwise unchanged, and every existing call site's behavior (`SEMGREP_SARIF_OUTPUT` unset) is byte-identical to pre-change. -5. `.github/workflows/semgrep.yml` invokes `scripts/pre-commit-hooks/semgrep-scan.sh` directly for **both** the SARIF-producing pass (with `SEMGREP_SARIF_OUTPUT` set) and the hard-fail gate pass (unset) โ€” no independent/duplicated `semgrep scan ...` invocation exists anywhere in the workflow YAML. -6. `scripts/ci/check-semgrep-parity.sh` exists, passes against the merged state, and is invoked as a CI step in `semgrep.yml` before the scan runs. -7. SARIF results upload to the GitHub Security tab under category `semgrep`, using the same `github/codeql-action/upload-sarif` SHA pin already used in `security-pr.yml`, gated on an explicit SARIF-existence check step. -8. `SECURITY.md`'s `### Automated Scanning` table and `### Scanning Workflows` section mention Semgrep and `semgrep.yml`. -9. `ARCHITECTURE.md` mentions Semgrep at all three existing security-scanning call-out locations (tech-stack table, CI Jobs list, Container Scanning components list). -10. `.gitignore`, `.dockerignore`, `.codecov.yml`, and all Dockerfiles are confirmed unchanged (per ยง2.9 โ€” no diff expected in this PR). -11. `actionlint`, `markdownlint`, `shellcheck`, and `lefthook run pre-commit` all pass locally on the final diff. -12. `docs/plans/current_spec.md` (this file) reflects the implemented state โ€” no divergence between plan and shipped workflow at PR time (devops agent should update this file if implementation deviates from any spec section above, per standard plan-fidelity practice). +| Scenario | Behavior | +|---|---| +| No releasable commits since last tag (only `chore:`/`docs:`/`test:`/`ci:`/`build:`/`style:`/`refactor:` land on `main`) | Release-please does not open/update a release PR at all. **Behavior change from today**: `auto-versioning.yml` currently bumps patch for *any* non-`feat:` commit, so today a `chore:`-only week still cuts a release; under release-please, it won't. This is a deliberate, disclosed change (arguably a correctness improvement โ€” no more "no-op" patch releases), not an oversight. Flagged for user sign-off. | +| `feat!:`/`BREAKING CHANGE:` commit lands pre-1.0 | Bumps minor, not major, per Decision/Additional-Finding D's explicit config. | +| Release PR sits open for a long time while more commits land | Release-please updates the existing PR's body/diff in place (its documented standard behavior) โ€” no duplicate PRs. | +| Someone merges the release PR via squash instead of the "Create a merge commit"/default GitHub merge release-please expects | Not explicitly tested in this plan (CI-config-only, no live GitHub run possible locally) โ€” flagged in Manual Post-Merge Follow-Ups as the first thing to verify by watching the first real release PR merge. | +| `.release-please-manifest.json` drifts from the real latest tag (e.g., someone force-pushes a tag manually) | Release-please reads the manifest as its source of truth for "last released version," not live tag state โ€” a manual tag push outside release-please's flow would desync them. Document this in `VERSION.md`'s rewrite as "don't manually tag `v*` releases going forward; let release-please do it." | + +# Implementation Plan + +This is a CI/CD-configuration-only change: no Go code, no TypeScript/React code, no database migrations, no API surface. Phases below are adapted accordingly from the standard template. + +## Phase 1: Playwright Tests (spec behavior) โ€” N/A + +No user-facing behavior changes; nothing to spec as `test.fixme`. Explicitly out of scope โ€” see "CLAUDE.md Definition-of-Done Applicability" below. + +## Phase 2: Backend Implementation โ€” N/A + +No `backend/` changes. + +## Phase 3: Frontend Implementation โ€” N/A + +No `frontend/` changes. ---- +## Phase 4: CI/CD Configuration Changes (replaces "Integration and Testing" for this chore) + +- GOAL-001: Stand up the release-please config/manifest/workflow and validate every JSON/YAML file for syntactic correctness and internal consistency (manifest version matches real latest tag; config's package key matches manifest's package key). + +| Task | Description | Completed | Date | +|------|-------------|-----------|------| +| TASK-001 | Create `release-please-config.json` per Technical Specifications, with `include-component-in-tag: false`, `bump-minor-pre-major: true`, `bump-patch-for-minor-pre-major: true`, `skip-changelog: true` all set explicitly. | | | +| TASK-002 | Create `.release-please-manifest.json`, seeded to the actual latest `v*` tag at implementation time (re-verify, don't copy `0.36.5` blindly). | | | +| TASK-003 | Create `.github/workflows/release-please.yml`, pinned-SHA `googleapis/release-please-action`, `push: branches: [main]` trigger, `contents: write` + `pull-requests: write` permissions. | | | +| TASK-004 | Validate all three new/changed files with `jq empty` (JSON) / a YAML parser (`yamllint` or `python -c "import yaml,sys; yaml.safe_load(open(sys.argv[1]))"`) before commit. | | | + +## Phase 5: Retirement + Documentation + +- GOAL-002: Remove the superseded workflows/config/scripts and rewrite `VERSION.md` to describe the new flow accurately. + +| Task | Description | Completed | Date | +|------|-------------|-----------|------| +| TASK-005 | Delete `.github/workflows/auto-versioning.yml`, `.github/workflows/release-goreleaser.yml`, `.goreleaser.yaml`. | | | +| TASK-006 | Delete `.github/workflows/auto-changelog.yml`, `.github/release-drafter.yml`. | | | +| TASK-007 | Delete `.version`, `scripts/check-version-match-tag.sh`, `.github/skills/utility-version-check-scripts/run.sh`, `.github/skills/utility-version-check.SKILL.md`; remove the `check-version-match` entry from `lefthook.yml`. | | | +| TASK-008 | Remove the `# GoReleaser` section from `.gitignore`; remove the `.goreleaser.yaml` line from `.dockerignore`. | | | +| TASK-009 | Rewrite `VERSION.md` per Technical Specifications. | | | +| TASK-010 | Run `lefthook run pre-commit` to confirm no hook references a now-deleted file/glob and everything still passes. | | | + +# CLAUDE.md Definition-of-Done Applicability + +This PR is CI-config/YAML/JSON/Markdown-only. Mapped explicitly against the standard DoD: + +| DoD Item | Applies? | Notes | +|---|---|---| +| 1. Playwright E2E Tests | **N/A** | No user-facing behavior; no frontend/backend code path changes. | +| 1.5. GORM Security Scan | **N/A** | No `backend/internal/models/**`, no GORM queries/migrations touched. | +| 2. Local Patch Coverage Preflight (`scripts/local-patch-report.sh`) | **Run it anyway** | It's mandatory regardless of change type per CLAUDE.md; expect it to report ~0% "patch" surface since no `.go`/`.ts`/`.tsx` lines are touched โ€” confirm it doesn't error out on a code-less diff rather than skip it. | +| 3. Security Scans (CodeQL/Trivy) | **Defer to CI** | This is a `chore:`-scoped change with no new application code path โ€” per CLAUDE.md's own rule ("Defer to CI for fix/test/chore/refactor-scoped changes with no new feature surface"), do not run these locally; CI runs both unconditionally regardless. **Caveat**: neither CodeQL nor Trivy actually provides coverage for this PR's one genuinely new risk surface โ€” `googleapis/release-please-action` is a new third-party Action granted `contents: write` + `pull-requests: write` on this repo. CodeQL scans Go/JS source; Trivy scans container images/dependencies; neither evaluates GitHub Actions permission scopes or third-party Action supply-chain trust. "Defer to CI" is accurate for what those two tools actually check, but should not be read as "this PR's permissions posture is covered" โ€” the mitigation here is the existing pinned-SHA convention (matching every other Action reference in this repo) plus the fact that the sibling repo already runs the identical Action/permissions combination without incident, not CodeQL/Trivy. | +| 4. Lefthook Triage | **Applies** | Run `lefthook run pre-commit` โ€” should no-op past YAML/JSON formatting-class hooks (no `.go`/`.ts` glob matches), but must still pass cleanly, especially after removing the `check-version-match` hook entry (verify lefthook config itself is still valid YAML). | +| 5. Staticcheck | **N/A** (no-op) | No `.go` files touched; hook glob won't match anything. | +| 6. Coverage Testing (85% backend/frontend) | **N/A** | No code touched; nothing for `go-test-coverage.sh`/`frontend-test-coverage.sh` to measure against this diff. | +| 7. Type Safety (`npm run type-check`) | **N/A** | No `.ts`/`.tsx` files touched. | +| 8. Verify Build (`go build`, `npm run build`) | **Recommended as a sanity check, not a real gate** | Neither build path is touched by this diff; running them just confirms the repo wasn't already broken. Not blocking for this PR specifically. | +| 9. Fixed/New Code Testing | **N/A** | No unit-testable code changed. | +| 10. Clean Up (debug prints, dead code) | **Applies in spirit** | The primary output of this PR *is* dead-code removal (Decisions 4-6, Additional Finding A) โ€” this is effectively the main content of the PR, not a final pass. | + +**The real gates for this PR** (called out explicitly per the task brief, since the standard DoD doesn't fit a CI-config change well): +- Every new/modified JSON file parses (`jq empty release-please-config.json .release-please-manifest.json`). +- Every new/modified YAML file parses (`.github/workflows/release-please.yml`, and re-validate `lefthook.yml` after the hook-entry removal). +- `lefthook run pre-commit` passes cleanly end-to-end. +- **GitHub Actions workflow behavior (does `release-please.yml` actually open a correct PR, does merging it actually tag+release correctly, does `orthrus-build.yml` actually still fire on that tag) can only be fully verified by a live run on GitHub after merge โ€” not locally, not in this plan.** This is the single biggest residual-risk category for this PR and is why the Manual Post-Merge Follow-Ups section below exists. + +# Manual Post-Merge Follow-Ups (cannot be validated without a live push to GitHub) + +1. **Confirm repo-level permissions**: verify `pull-requests: write` is actually honored for the `GITHUB_TOKEN` used by Actions in this repo (Settings โ†’ Actions โ†’ General โ†’ Workflow permissions). If the repo/org default is read-only, the `release-please.yml` workflow's explicit `permissions:` block should override it, but confirm on the first run rather than assume. +2. **Watch the first `release-please.yml` run on `main`** after this PR merges: confirm it either (a) opens a `chore(main): release X.Y.Z` PR if there are releasable commits since `v0.36.5`, or (b) does nothing cleanly if there aren't โ€” don't assume silence means broken. +3. **Verify tag format on the first real release**: confirm the tag release-please creates is exactly `v` (no `charon-` prefix) โ€” this is the `include-component-in-tag: false` risk called out in Additional Finding C. If it's wrong, `orthrus-build.yml`'s `tags: ['v*']` trigger and `generate-changelog.sh`'s `git tag -l 'v*'` scan both silently stop matching. +4. **Verify `CHANGELOG.md` is untouched** by the first release-please PR (confirms `skip-changelog: true` behaves as expected) **and** verify the resulting GitHub Release still gets a populated body (confirms release-notes generation is independent of the changelog-file write path โ€” this exact interaction wasn't found explicitly documented during this session's research). +5. **Verify `orthrus-build.yml` still fires** on the first release-please-created tag and produces the expected `type=semver` Docker tags. +6. **Branch protection interaction**: check whether `main`'s branch protection rules (required reviews, required status checks) block release-please's own bot-authored release PR from being merged, and if so, decide whether to exempt it or just merge manually each time (release-please doesn't require any special exemption to function โ€” it just opens a normal PR). +7. **Decide who merges the release PR and how** (manual click each time vs. some auto-merge label) โ€” this plan does not configure auto-merge; that's a deliberate choice left to the user, since it directly controls when a real release goes out. This is the operational follow-through on the "User Decisions Required Before Implementation" #1 go/no-go above: if the user decides fully-automatic cadence is a hard requirement, a follow-up PR adding a release-please auto-merge label + workflow would be needed โ€” not something this plan implements. +8. **Clean up historical run records** (optional): the deleted workflows' historical Action run logs remain visible under "Actions" until manually deleted/archived if desired โ€” cosmetic only, not required. +9. **Sibling-repo note**: `/projects/go_notify_yourself/release-please-config.json` was independently corrected to `"release-type": "go"` during this session (see Reference Implementation section) โ€” no action needed here, noted only so the user isn't surprised by the diff if they look at that repo later. + +# Acceptance Criteria + +- [ ] `release-please-config.json`, `.release-please-manifest.json`, and `.github/workflows/release-please.yml` exist, are valid JSON/YAML, and match the Technical Specifications section (including the explicit `include-component-in-tag`, `bump-minor-pre-major`, `bump-patch-for-minor-pre-major`, `skip-changelog` settings). +- [ ] `.github/workflows/auto-versioning.yml`, `.github/workflows/release-goreleaser.yml`, `.goreleaser.yaml`, `.github/workflows/auto-changelog.yml`, `.github/release-drafter.yml`, `.version`, `scripts/check-version-match-tag.sh`, and the two `utility-version-check` skill files are all deleted. +- [ ] `.github/skills/README.md`, `.vscode/tasks.json`, and `.github/skills/utility-bump-beta.SKILL.md` no longer reference the deleted `utility-version-check` skill (Decision 4 blast-radius items). +- [ ] `CLAUDE.md:261`'s `utility-version-check` Skills-table row is removed in this PR (Commit 5), per the user's explicit authorization to edit CLAUDE.md as part of this migration. +- [ ] `lefthook.yml` no longer references `check-version-match-tag.sh`; `lefthook run pre-commit` passes. +- [ ] `.gitignore` and `.dockerignore` no longer reference GoReleaser artifacts/config. +- [ ] `scripts/generate-changelog.sh`'s header comment no longer references the deleted `release-goreleaser.yml`. +- [ ] `VERSION.md` accurately describes the release-please PR-based flow and no longer references `release-goreleaser.yml`, `.version` as a release trigger, or the release-drafter workflow. +- [ ] `ARCHITECTURE.md`'s "Release Workflow" section accurately describes the release-please push-to-main โ†’ standing-PR โ†’ merge-to-tag flow and no longer references `release-goreleaser.yml` or a tag-push trigger as step 1. +- [ ] `orthrus-build.yml` and `docker-build.yml` are **not modified** by this PR (both confirmed to need no changes per Decision 2). +- [ ] No Go, TypeScript, or database-schema files are touched. +- [ ] PR is opened from `chore/release-please-migration` against `main` (not `development`), with the branching deviation explicitly called out in the PR description per the Branching Note above, and merged via squash merge (per the Merge Strategy note in the Introduction). +- [ ] All commits use `chore:` (or `chore(ci):`) Conventional Commit prefixes, matching CLAUDE.md's CI-trigger convention (so this PR's own merge does not trigger a Docker build). +- [x] The user has explicitly signed off on both items in "User Decisions Required Before Implementation" (manual release-cadence gating: APPROVED; chore-only-weeks/`perf:`-no-longer-releasable: ACCEPT release-please defaults, no special-case override) โ€” see that section for the recorded decisions. +- [x] The user has explicitly authorized editing `CLAUDE.md` as part of this PR to remove the stale `utility-version-check` Skills-table row (line 261), folded into Commit 5 โ€” see Decision 4's blast-radius table. + +# Commit Slicing Strategy + +**Decision: single PR, `chore/release-please-migration` โ†’ `main`, with ordered logical commits.** Per CLAUDE.md's "One Feature = One PR" rule โ€” this is one cohesive infrastructure change and must not be split across multiple PRs (e.g., "add release-please" in one PR and "remove old workflows" in another would leave the repo running two competing release mechanisms simultaneously in the gap between merges, which is strictly worse than doing it atomically). -## 6. Commit Slicing Strategy - -**Decision:** single PR, one feature ("Semgrep CI coverage"), ordered logical commits. No cross-PR splitting per `CLAUDE.md`'s "One Feature = One PR" rule โ€” this is a small, cohesive, CI-only change (plus one additive shell-script hook); splitting it further would violate that rule for no benefit (there's no independently-shippable sub-feature here โ€” a workflow with no parity guard, or a parity guard with no workflow, are both incomplete on their own). Approved as-is by Supervisor round 2 โ€” shape unchanged, contents updated below for ยง2.7's design revision and the ARCHITECTURE.md addition. - -### Commit 1 โ€” Script hook + parity guard foundation -- **Scope:** Additive-only, no behavior change for any existing call site. Adds the `SEMGREP_SARIF_OUTPUT` hook to `semgrep-scan.sh`, the new (as-yet-unused-by-CI) parity script, and optionally the shared helper extraction. -- **Files:** `scripts/pre-commit-hooks/semgrep-scan.sh` (modified โ€” additive only, per ยง3.0), `scripts/ci/check-semgrep-parity.sh` (new โ€” will fail if run now, since `semgrep.yml` doesn't exist yet; not wired into any workflow in this commit), `scripts/ci/lib/workflow-yaml-asserts.sh` (new, optional) and `scripts/ci/check-codeql-parity.sh` (refactored to source it, optional, no behavioral change) if the extraction from ยง3.5 is included. -- **Dependencies:** none. -- **Validation gate:** `shellcheck` on all touched/new scripts; local `lefthook run pre-commit` / `lefthook run security-full` on a sample file confirms `semgrep-scan.sh`'s default behavior is unchanged; `bash scripts/ci/check-codeql-parity.sh` passes unchanged if the refactor is included (regression check). -- **Commit message:** `feat(security): add opt-in SARIF output mode to local Semgrep script and add CI parity guard` - -### Commit 2 โ€” Semgrep CI workflow -- **Scope:** Adds the new workflow file, delegating both its SARIF and gate passes to the script from Commit 1, and wires the parity guard from Commit 1 into it. -- **Files:** `.github/workflows/semgrep.yml` (new). -- **Dependencies:** Commit 1 (the `SEMGREP_SARIF_OUTPUT` hook and the parity guard must exist for this workflow to reference real, working behavior). -- **Validation gate:** `actionlint .github/workflows/semgrep.yml`; `bash scripts/ci/check-semgrep-parity.sh` now passes (workflow file exists, delegates correctly, image pin format valid); `lefthook run pre-commit` clean on the diff. -- **Commit message:** `feat(security): add pinned Semgrep SAST scan to CI, mirroring local pre-commit/pre-push scan` - -### Commit 3 โ€” Documentation -- **Scope:** `SECURITY.md` and `ARCHITECTURE.md` updates only, per ยง4 Phase 3. -- **Files:** `SECURITY.md`, `ARCHITECTURE.md`. -- **Dependencies:** Commit 2 (documents the workflow file that now exists). -- **Validation gate:** `markdownlint SECURITY.md ARCHITECTURE.md`. -- **Commit message:** `docs: document Semgrep CI scan in SECURITY.md and ARCHITECTURE.md` - -### Commit 4 โ€” Hardening / fixups (conditional) -- **Scope:** Only if Phase 5 (opening the PR and observing the first real workflow run) surfaces something unfixable purely by inspection โ€” e.g. the digest needs re-resolution, `actionlint`/a GitHub Actions schema quirk requires a syntax adjustment not visible from local linting alone, or the container's default shell needs an explicit `shell: bash` on a step. -- **Files:** `.github/workflows/semgrep.yml` and/or `scripts/ci/check-semgrep-parity.sh`, as needed. -- **Dependencies:** Commits 1-3, plus one observed CI run on the PR. -- **Validation gate:** the actual GitHub Actions run on the PR going green. -- **Commit message:** `fix: address Semgrep CI workflow issues found in first live run` (only created if needed โ€” do not pre-author an empty placeholder commit). - -### Rollback / contingency - -- **Rollback:** revert the PR's merge commit. The change is additive-only (new files + one additive, backward-compatible shell-script hook + documentation sections); reverting it removes Semgrep CI coverage cleanly with no residual state โ€” no DB migration, no data written, no schema changed. `git revert -m 1 ` is sufficient. -- **Contingency โ€” pinned image becomes unpullable mid-development-cycle (e.g. registry outage, Docker Hub rate limiting on `ubuntu-latest` runners):** the job fails visibly (container pull failure is unambiguous in the Actions log, distinct from a scan failure), does not block other workflows (independent job, independent file), and does not gate merges any more strictly than any other required-check outage would โ€” same failure mode and same operational response as a transient CodeQL or Trivy Action outage today. -- **Contingency โ€” CI Semgrep surfaces findings that don't reproduce locally:** expected and desired (ยง4 Phase 5) โ€” indicates local environment drift, not a CI bug. Do not suppress via `--exclude-rule` additions without documenting rationale (matching the existing precedent set by the one documented `gorm-empty-password` exclusion already in the script). -- **Contingency โ€” parity guard is judged too strict/noisy after landing** (e.g. flags legitimate divergence that's actually fine): tune the specific assertion in `check-semgrep-parity.sh`, don't delete the guard wholesale โ€” same operating principle already established for `check-codeql-parity.sh`, which has been iterated on rather than removed. +| Commit | Scope | Files | Depends on | Validation gate | +|---|---|---|---|---| +| **1** | Add release-please config + manifest (no workflow yet โ€” inert until Commit 2) | `release-please-config.json`, `.release-please-manifest.json` | โ€” | `jq empty release-please-config.json .release-please-manifest.json`; manually diff the manifest's seed version against `git tag --sort=-v:refname \| head -1` to confirm it's current. | +| **2** | Add the release-please workflow | `.github/workflows/release-please.yml` | Commit 1 | YAML parses; `lefthook run pre-commit` passes; manual read-through confirming the pinned action SHA/tag comment matches the sibling repo's convention and is a real, current release. | +| **3** | Retire the superseded auto-versioning/GoReleaser pipeline | `.github/workflows/auto-versioning.yml` (delete), `.github/workflows/release-goreleaser.yml` (delete), `.goreleaser.yaml` (delete), `.gitignore` (remove GoReleaser section), `.dockerignore` (remove `.goreleaser.yaml` line), `scripts/generate-changelog.sh` (fix stale header-comment reference at line 5 from `release-goreleaser.yml` to `nightly-build.yml`, its one remaining real caller) | Commits 1-2 (don't remove the old path until the new one exists) | `lefthook run pre-commit` passes; confirm no remaining reference to `.goreleaser.yaml` anywhere (`grep -rn goreleaser --include=*.yml --include=*.md .` minus this plan file itself); confirm `generate-changelog.sh` still runs correctly after the comment-only edit (no functional change, but re-run it locally against a small tag range as a sanity check). | +| **4** | Retire the redundant release-drafter changelog automation | `.github/workflows/auto-changelog.yml` (delete), `.github/release-drafter.yml` (delete) | Commit 2 (release-please must exist as the replacement before removing this) | `lefthook run pre-commit` passes. | +| **5** | Retire the legacy `.version` parity check and its full reference surface | `.version` (delete), `scripts/check-version-match-tag.sh` (delete), `.github/skills/utility-version-check-scripts/run.sh` (delete), `.github/skills/utility-version-check.SKILL.md` (delete), `lefthook.yml` (remove `check-version-match` hook entry), `.github/skills/README.md` (remove the `utility-version-check` row from the Utility Skills table at ~line 72, and the `utility-version-check` bullet from the naming-convention examples at ~line 267), `.vscode/tasks.json` (remove the "Utility: Check Version Match Tag" task block, ~lines 694-698), `.github/skills/utility-bump-beta.SKILL.md` (remove the dead `utility-version-check` cross-link from its "Related Skills" section, ~line 186, keep the rest of that list), `CLAUDE.md` (remove the `utility-version-check` row from the Skills table at ~line 261 โ€” per explicit user authorization to edit CLAUDE.md as part of this PR) | Commit 1 (manifest is the intended replacement source of truth) | `lefthook run pre-commit` passes with no dangling glob/hook referencing a deleted script; confirm `lefthook.yml` and `.vscode/tasks.json` are still valid YAML/JSON respectively; confirm `CLAUDE.md` remains valid Markdown with only the one table row removed, no other governance text touched; `grep -rn "utility-version-check\|check-version-match-tag" .` (excluding this plan file and `.git/`) returns **zero** hits anywhere, including `CLAUDE.md`. | +| **6** | Documentation rewrite | `VERSION.md` (full rewrite per Technical Specifications), `ARCHITECTURE.md` (rewrite the "Release Workflow" section per Modified Files above: replace tag-push-triggers-everything framing with the release-please flow, fix the `generate-changelog.sh`/`release-goreleaser.yml` reference) | Commits 1-5 (must describe the end state, not the transition) | Manual proofread against the final state of every file above; confirm no reference to any deleted file/workflow remains in either doc. | + +**Rollback / contingency for the PR as a whole**: since this is entirely additive-then-subtractive CI configuration with no code or schema changes, rollback is a plain `git revert` of the merge commit (or of the whole PR range) โ€” no data migrations, no forward-only state changes are introduced. The one piece of *external* (not-in-git) state this PR's downstream behavior touches is the standing release-please PR itself and any tag it creates after merge; if the migration needs to be rolled back after a real release-please release has already gone out, `.release-please-manifest.json` should be re-seeded to match whatever the real latest tag is at rollback time (not blindly reverted to the pre-migration value), and the old `auto-versioning.yml`/`release-goreleaser.yml` files restored via revert will resume exactly their prior (partially broken) behavior with no additional cleanup needed, since neither of them depended on anything release-please would have introduced. + +# Dependencies + +- **DEP-001**: `googleapis/release-please-action` (pinned by SHA, `# v5` comment) โ€” new external GitHub Action dependency, matching the pattern already trusted in `/projects/go_notify_yourself`. +- **DEP-002**: No new npm/Go module dependencies. No `package.json`/`go.mod` changes. + +# Risks & Assumptions + +- **RISK-001**: `include-component-in-tag` default-vs-explicit-`false` mismatch could produce a wrongly-prefixed tag on the first real release, silently breaking `orthrus-build.yml`'s trigger and `generate-changelog.sh`'s tag scan until noticed. Mitigated by explicit config (Additional Finding C) and flagged as the #1 manual-verification item post-merge. +- **RISK-002**: `skip-changelog: true`'s interaction with GitHub Release notes generation is not fully documented in the sources available this session โ€” small chance the Release body comes out empty rather than independently populated. Flagged in Manual Post-Merge Follow-Ups. +- **RISK-003**: Branch protection on `main` could block release-please's bot-authored release PR from merging cleanly (required reviewers, required status checks that don't apply to a docs/manifest-only PR). Flagged in Manual Post-Merge Follow-Ups; no code change can pre-empt this, it must be checked live. +- **RISK-004**: This PR targets `main` directly, bypassing the normal `development` โ†’ `nightly` โ†’ `main` soak cycle by design (see Branching Note). Slightly higher blast-radius-per-mistake than the repo's usual flow, mitigated by the fact that the change is inert until the *next* real release-worthy commit lands on `main` (release-please won't retroactively do anything to already-tagged history). +- **ASSUMPTION-001**: The latest tag at plan-authoring time (`v0.36.5`) is still the latest tag at implementation time. Re-verify before seeding `.release-please-manifest.json` (explicitly called out as an implementer task, not assumed). +- **ASSUMPTION-002** (revised โ€” the original wording overclaimed completeness; corrected per Supervisor's independent re-run of the same grep, which surfaced two more references this plan now accounts for rather than leaves implicit): a repo-wide grep for `auto-versioning`, `release-goreleaser`, `CHARON_PR2_GATES_PASSED`, `softprops/action-gh-release`, and `paulhatch` does **not** guarantee full coverage of every reference to the files this plan deletes โ€” it only checked those specific literal strings, and it missed `ARCHITECTURE.md` (now added to Modified Files above) and a stale header comment in `scripts/generate-changelog.sh:5` (`"see .github/workflows/release-goreleaser.yml"` โ€” functionally harmless, since the script's actual behavior doesn't depend on GoReleaser, but a dead pointer once that workflow is deleted; corrected as part of Commit 3, since it's tied directly to the GoReleaser removal). Grep-based "nothing else references this" claims in this plan should be read as "no hits for the specific strings searched," not as an exhaustive guarantee โ€” the actual assumption being made is that the Research Findings section's enumerated consumer list is complete, which was cross-checked by Supervisor's independent review and found to need these two additions plus the Decision 4 blast-radius additions above, and no others. + +# Related Specifications / Further Reading + +- `/projects/go_notify_yourself/release-please-config.json`, `.release-please-manifest.json`, `.github/workflows/release-please.yml` โ€” reference implementation. +- `docs/reports/caddy-security-posture.md` โ€” origin/closure evidence for the "PR-2" gate (Decision 6). +- release-please documentation: `docs/customizing.md`, `docs/design.md`, `schemas/config.json` in `googleapis/release-please` (all fetched and cited directly in this plan). diff --git a/docs/reports/archive/2026-08-14_semgrep-ci-security-scan-qa_report.md b/docs/reports/archive/2026-08-14_semgrep-ci-security-scan-qa_report.md new file mode 100644 index 000000000..46f5606ec --- /dev/null +++ b/docs/reports/archive/2026-08-14_semgrep-ci-security-scan-qa_report.md @@ -0,0 +1,108 @@ +# QA Report โ€” Semgrep CI Security Scan (Independent Verification) + +**Branch**: `development` +**Commits reviewed**: `6bf066f8`, `2fbecf07`, `7c6fb04f` +**Reviewed by**: qa-security agent +**Date**: 2026-08-14 +**Scope**: CI/CD-only feature โ€” no application code, models, or frontend/UI surface touched. +**Prior review**: Supervisor code review โ€” approved, no blocking issues. +**Purpose**: Independent functional/security verification per Phase 6 of the management pipeline, ahead of a final "done" determination. + +## Summary Verdict: **PASS** โ€” no blocking defects found. + +The Semgrep CI gate is functionally real (confirmed to fail on findings and pass when clean, via a positive-control test), the parity guard genuinely detects drift (confirmed via four separate intentional-break tests), the container image is correctly digest-pinned, and all local DoD-relevant checks scoped to a CI/shell-script-only change are clean. Two pre-existing environment/documentation gaps were identified and are explicitly **not** attributed to this feature (see ยง4 and ยง6). + +--- + +## 1. Functional Correctness of the Scan (PASS) + +Installed Semgrep 1.173.0 into a throwaway venv (`/tmp/.../scratchpad/semgrep-venv`, exact version match to the pinned CI image) and ran the actual wrapper script `scripts/pre-commit-hooks/semgrep-scan.sh` exactly as CI invokes it. + +| Check | Result | +|---|---| +| `SEMGREP_SARIF_OUTPUT= bash scripts/pre-commit-hooks/semgrep-scan.sh` (full repo, no targets) | Exit 0. Produced a valid SARIF file (`version`, `runs`, `results`, `$schema` present; parsed cleanly as JSON). | +| `bash scripts/pre-commit-hooks/semgrep-scan.sh` (no env var, full repo) | Exit 0. `--error` semantics confirmed live (see ยง1.1). | +| Repo clean under full scan | Reproduced: 974 files tracked by git, 160 rules run, **0 findings** โ€” matches both prior QA/DevOps reports exactly. Two suppressed (`nosemgrep`-annotated) `websocket-missing-origin-check` findings appear in the SARIF's `results` array with `suppressions: [{kind: inSource}]` โ€” this is correct SARIF behavior (audit trail for suppressed findings) and does not affect the "0 findings / 0 blocking" scan summary or exit code. | +| Runtime | ~45โ€“48s per full-repo pass locally (single-threaded venv on this sandbox; CI's dedicated `semgrep/semgrep` container should be comparable or faster). | + +### 1.1 Positive-control test: does the gate actually gate? (Most important check โ€” PASS) + +Constructed a minimal Go file containing an unguarded `websocket.Upgrader{}.Upgrade()` call (the same rule ID, `go.gorilla.security.audit.websocket-missing-origin-check`, that appears โ€” suppressed โ€” in the real codebase), and ran it through the **actual, unmodified** wrapper script with a single-file target: + +``` +SEMGREP_SARIF_OUTPUT=out.sarif bash scripts/pre-commit-hooks/semgrep-scan.sh + โ†’ Findings: 1 (1 blocking) โ†’ exit 0 (SARIF mode does not hard-fail) + +bash scripts/pre-commit-hooks/semgrep-scan.sh + โ†’ Findings: 1 (1 blocking) โ†’ exit 1 (--error mode hard-fails) +``` + +This is the critical distinction the task flagged as the top risk: a gate that always exits 0 regardless of findings would be a silent no-op. **Confirmed not the case.** The `SEMGREP_SARIF_OUTPUT` toggle in `scripts/pre-commit-hooks/semgrep-scan.sh:42-46` genuinely swaps `--error` for `--sarif --output `, and only the `--error` invocation (the CI workflow's "hard-fail gate" step, `semgrep.yml:75-76`) enforces blocking. The SARIF-producing pass (`semgrep.yml:49-54`) is additionally wrapped in `continue-on-error: true` at the workflow level, which is defense-in-depth on top of the script's own non-blocking `--sarif` exit code โ€” belt and suspenders, not a substitute for the real gate. + +--- + +## 2. Workflow YAML Structural Validity (PASS) + +- `actionlint .github/workflows/semgrep.yml` (installed via `go install github.com/rhysd/actionlint@latest` into a throwaway `GOBIN`): **0 findings, exit 0.** +- Container image resolution: `docker buildx imagetools inspect semgrep/semgrep:1.173.0@sha256:67319956da3dcb58baf5b322899c15458e3963e7018a86aeeb5cd224e69cb77a` (the exact digest read fresh from the committed file, `semgrep.yml:33`) resolved successfully against the registry, returning a multi-platform manifest list whose index digest matches the pinned digest exactly. The pin is real and correct, not a stale/copy-pasted digest. + +--- + +## 3. Parity Guards (PASS, and confirmed non-trivial) + +- `bash scripts/ci/check-semgrep-parity.sh` โ€” exit 0. +- `bash scripts/ci/check-codeql-parity.sh` โ€” exit 0 (unaffected by the refactor that extracted `scripts/ci/lib/workflow-yaml-asserts.sh`; this file was verified as a correct behavior-preserving extraction, not a modification of the CodeQL guard's assertions). + +**Adversarial drift tests** (performed against throwaway copies in `/tmp/.../scratchpad/parity-break-test/`, never against the real repo files; all discarded after each test, working tree confirmed clean of these edits afterward): + +| Simulated drift | Guard result | +|---|---| +| Inline `semgrep scan --config p/golang` reintroduced in place of `bash scripts/pre-commit-hooks/semgrep-scan.sh` delegation | **Caught.** `must delegate to scripts/pre-commit-hooks/semgrep-scan.sh instead of reimplementing the semgrep scan invocation inline` โ€” exit 1. | +| `SEMGREP_SARIF_OUTPUT` hook deleted from the local script | **Caught.** `must retain the SEMGREP_SARIF_OUTPUT hook so CI can produce SARIF via the same script` โ€” exit 1. | +| Image pin degraded from `semgrep/semgrep:1.173.0@sha256:...` to `semgrep/semgrep:latest` | **Caught.** `must pin the semgrep/semgrep image with both an exact tag and a sha256 digest` โ€” exit 1. | +| `pull_request` branch list narrowed from `[main, nightly, development]` to `[main, nightly]` | **Caught.** `pull_request branches must be [main, nightly, development]` โ€” exit 1. | + +All four drift classes are detected. The guard is a real structural check, not a no-op that always passes. + +--- + +## 4. Local DoD-Relevant Checks (PASS, with one noted pre-existing environment gap) + +Scoped per the task's guidance: no Playwright E2E (no user-facing behavior), no GORM scan (confirmed zero files under `backend/internal/models/**` or any `.go` files touched โ€” `git diff --name-only c510085f 7c6fb04f` shows only workflow/doc/shell files), no frontend type-check/build (zero `frontend/` files touched). + +- **shellcheck** (installed a static v0.10.0 binary into a throwaway location, no sudo/apt available) on all touched/new shell scripts (`semgrep-scan.sh`, `check-semgrep-parity.sh`, `workflow-yaml-asserts.sh`, `check-codeql-parity.sh`), using the project's actual severity threshold from `lefthook.yml` (`shellcheck --severity=error`): **0 findings.** (Default-severity mode surfaces two SC1091 "info"-level "not following sourced file" notices caused by the scripts' dynamic `SCRIPT_DIR` resolution pattern โ€” expected and filtered out by the repo's own configured threshold, not a defect.) +- **`lefthook run pre-commit`**, targeted against this feature's exact changed-file set (`--file <7 files>`, since nothing was staged in this session): all hooks pass โ€” `trailing-whitespace`, `end-of-file-fixer`, `actionlint`, `check-lfs-large-files`, `block-codeql-db`, `block-data-backups`, `semgrep` (0 findings, `semgrep` installed into PATH via the throwaway venv for this run only). One hook, **`check-yaml`, failed** with `ModuleNotFoundError: No module named 'yaml'` โ€” **confirmed pre-existing/environmental, not introduced by this feature**: the hook shells out to system `python3 -c "import yaml..."` (`lefthook.yml:54`), and this sandbox's system Python lacks PyYAML. Verified by installing PyYAML into the throwaway venv and re-running the identical parse command directly against `.github/workflows/semgrep.yml` โ€” it parsed cleanly (exit 0), proving the YAML itself is valid and the failure is purely a missing sandbox dependency, structurally identical to the already-documented `gitleaks`-unavailable gap below. +- **`gitleaks`**: confirmed absent from PATH (`which gitleaks` โ†’ exit 1) in this sandbox, matching the DevOps report. This predates the Semgrep feature entirely (a secrets-scanning tool unrelated to Semgrep) and is not something this feature could plausibly mask โ€” the feature adds zero new secret-bearing surface (see ยง5). + +--- + +## 5. Security-Specific Checks (PASS) + +- **`permissions:`** โ€” both the workflow-level and job-level blocks are exactly `contents: read`, `security-events: write`, `actions: read`, `pull-requests: read` โ€” least-privilege, no broader scope (no `write` on `contents`, no `id-token`, no `packages`, etc.). +- **Triggers** โ€” `pull_request` (not `pull_request_target`) confirmed at `semgrep.yml:4`. No fork-PR privilege-escalation risk. +- **Secrets/tokens** โ€” `grep -in "secrets\.\|token\|GITHUB_TOKEN"` across `semgrep.yml`, `semgrep-scan.sh`, `check-semgrep-parity.sh`, `workflow-yaml-asserts.sh` returns no matches (the only "token" hits were unrelated word fragments in comments, none present). The SARIF-upload step uses `github/codeql-action/upload-sarif`, which relies on the workflow's implicit default `GITHUB_TOKEN` scoped by the `permissions:` block above โ€” no custom secret is declared or required anywhere in this feature. +- **Container image pin** โ€” digest-pinned (`semgrep/semgrep:1.173.0@sha256:67319956...`), confirmed resolvable and matching the registry (ยง2). Not a floating tag. + +--- + +## 6. Documentation Lint (PASS โ€” no regression introduced) + +Ran `markdownlint-cli2@0.23.2` (the exact tool/version pinned in `package.json`, not the unrelated `markdownlint-cli` package) against `SECURITY.md` and `ARCHITECTURE.md`. + +- **SECURITY.md**: 114 findings (MD036 emphasis-as-heading, MD060 table-column-style, one MD034 bare-URL) โ€” **identical count before and after** commit `7c6fb04f` (verified via `git show 7c6fb04f~1:SECURITY.md` piped through the same linter: 114 findings on the pre-commit version too). All findings are pre-existing formatting debt scattered across unrelated CVE-entry sections (lines 407โ€“813); zero findings land in the new Semgrep paragraph or table row this PR added (~line 1000โ€“1032). +- **ARCHITECTURE.md**: 93 findings, likewise identical before/after `7c6fb04f`. + +This PR's doc changes introduce zero new lint findings. Pre-existing lint debt is a separate, out-of-scope cleanup item and not this feature's responsibility. + +--- + +## Blocking Issues + +**None.** + +## Non-Blocking Observations (informational only, no action required for this PR) + +1. `check-yaml` and `gitleaks` are unavailable in this local sandbox due to missing system dependencies (PyYAML, gitleaks binary). Both are pre-existing environment gaps unrelated to this feature; CI's environment has these tools installed and is authoritative. No masking risk identified โ€” the underlying YAML was independently verified valid, and this feature introduces no new secret-bearing surface for `gitleaks` to have caught. +2. SECURITY.md/ARCHITECTURE.md carry substantial pre-existing markdownlint debt (207 combined findings) unrelated to this PR. Worth a future standalone cleanup pass, but explicitly out of scope here per this feature's CI/shell-script-only mandate. + +## Final Overall Verdict: **PASS โ€” ready to be marked done.** diff --git a/docs/reports/qa_report.md b/docs/reports/qa_report.md index 46f5606ec..5c6a23fd8 100644 --- a/docs/reports/qa_report.md +++ b/docs/reports/qa_report.md @@ -1,108 +1,167 @@ -# QA Report โ€” Semgrep CI Security Scan (Independent Verification) +# QA Report โ€” Issue #619 Test-Infrastructure Debt Closeout -**Branch**: `development` -**Commits reviewed**: `6bf066f8`, `2fbecf07`, `7c6fb04f` +**Branch**: `test/issue-619-test-infra-debt` +**Commits reviewed this pass**: `dac267f3`..`52bdc675` (19 commits โ€” 6 parallel dev-agent fix rounds closing out the 52 E2E failures documented in the prior pass) **Reviewed by**: qa-security agent -**Date**: 2026-08-14 -**Scope**: CI/CD-only feature โ€” no application code, models, or frontend/UI surface touched. -**Prior review**: Supervisor code review โ€” approved, no blocking issues. -**Purpose**: Independent functional/security verification per Phase 6 of the management pipeline, ahead of a final "done" determination. +**Date**: 2026-08-08 +**Plan reference**: `docs/plans/current_spec.md` -## Summary Verdict: **PASS** โ€” no blocking defects found. +## FINAL CLOSEOUT (added by Management after this report, same day) -The Semgrep CI gate is functionally real (confirmed to fail on findings and pass when clean, via a positive-control test), the parity guard genuinely detects drift (confirmed via four separate intentional-break tests), the container image is correctly digest-pinned, and all local DoD-relevant checks scoped to a CI/shell-script-only change are clean. Two pre-existing environment/documentation gaps were identified and are explicitly **not** attributed to this feature (see ยง4 and ยง6). +Per the codified policy change (`75c63696`), no further local full-suite/multi-browser Playwright runs were performed. CI's next run on the PR is the authoritative full cross-browser confirmation. All other, non-Playwright Definition of Done gates were completed locally after this report (branch then gained one more commit, `716e26b2`, fixing the 2 new a11y findings in ยง1 below): ---- +| Gate | Result | +|---|---| +| Backend coverage (`go-test-coverage.sh`) | โœ… 89.2% line coverage (min 87%) | +| Frontend coverage (`frontend-test-coverage.sh`) | โœ… 90.84% line coverage (min 87%); Statements 89.63%, Branches 82.8%, Functions 87.27% | +| Local patch coverage (`local-patch-report.sh`) | โœ… 100% overall (18/18 changed lines), vs. 90% minimum | +| Lefthook `pre-commit` (targeted run against all 51 files changed vs. `development`, since the `actionlint` job hung on an environment issue unrelated to this PR โ€” zero `.github/workflows/*.yml` files are touched by this PR, confirmed via `git diff --stat`, so `actionlint` has nothing to check here regardless) | โœ… go-vet, golangci-lint-fast, dockerfile-check, frontend-type-check, frontend-lint all pass. `shellcheck` re-run directly (bypassing a `{staged_files}` templating artifact) โ€” 0 errors. `semgrep` โ€” 424 rules / 973 files / 0 findings. | +| Lefthook `codeql` (Go + JS) | โœ… Go: 1 result, suppressed (pre-existing, documented in `codeql-suppressions.yml`, unrelated to this PR). JS: 0 findings. Parity check passed. | +| `gitleaks` (not part of the mandatory `pre-commit` stage, run directly for defense-in-depth per the "double-check for secrets" guidance) | โœ… 514 pre-existing findings across the broader repo (test fixtures/mock credentials, none introduced by this PR) โ€” **0 findings in any of this PR's 51 changed files**, confirmed by cross-referencing the gitleaks JSON report against the changed-file list. | +| Trivy container/dependency scan | โžก๏ธ Carried forward, not re-run โ€” confirmed via `git diff --stat` that this PR changes zero `go.mod`/`go.sum`/`package.json`/`package-lock.json`/`Dockerfile*` files across all 30 commits vs. `development`, so the earlier confirmed result (0 new Critical/High, 1 pre-existing tracked HIGH documented in `SECURITY.md`) cannot have drifted. | +| Backend build (`go build ./...`) | โœ… Clean | +| Frontend build (`npm run build`) | โœ… Clean, `โœ“ built in 2.40s` | +| GORM security scan | N/A, correctly skipped โ€” zero `.go` files changed anywhere in this PR | -## 1. Functional Correctness of the Scan (PASS) +**This is now a complete Definition of Done sweep for everything except full cross-browser/full-suite Playwright confirmation, which is CI's job per policy.** Recommend: push, open/update the PR, let CI run the full 3-browser matrix, and treat any CI-reported failure as a new finding to triage rather than assuming the local partial runs already covered it. -Installed Semgrep 1.173.0 into a throwaway venv (`/tmp/.../scratchpad/semgrep-venv`, exact version match to the pinned CI image) and ran the actual wrapper script `scripts/pre-commit-hooks/semgrep-scan.sh` exactly as CI invokes it. +## STATUS: Partial re-validation โ€” stopped mid-run per updated process guidance. Not a final merge verdict. -| Check | Result | -|---|---| -| `SEMGREP_SARIF_OUTPUT= bash scripts/pre-commit-hooks/semgrep-scan.sh` (full repo, no targets) | Exit 0. Produced a valid SARIF file (`version`, `runs`, `results`, `$schema` present; parsed cleanly as JSON). | -| `bash scripts/pre-commit-hooks/semgrep-scan.sh` (no env var, full repo) | Exit 0. `--error` semantics confirmed live (see ยง1.1). | -| Repo clean under full scan | Reproduced: 974 files tracked by git, 160 rules run, **0 findings** โ€” matches both prior QA/DevOps reports exactly. Two suppressed (`nosemgrep`-annotated) `websocket-missing-origin-check` findings appear in the SARIF's `results` array with `suppressions: [{kind: inSource}]` โ€” this is correct SARIF behavior (audit trail for suppressed findings) and does not affect the "0 findings / 0 blocking" scan summary or exit code. | -| Runtime | ~45โ€“48s per full-repo pass locally (single-threaded venv on this sandbox; CI's dedicated `semgrep/semgrep` container should be comparable or faster). | +This pass was launched as a full Definition-of-Done re-validation (clean rebuild + all 3 browsers + backend/frontend coverage + lefthook + Trivy). **Mid-run, Management issued a process change, since codified in commit `75c63696`** (`CLAUDE.md`, `.claude/agents/qa-security.md`, `management.md`, `playwright-dev.md`): full-suite and multi-browser (`chromium`+`firefox`+`webkit` together) Playwright runs are **CI-only**, never local โ€” not even as a "final" or "consolidated" validation pass. Locally, E2E scope is limited to targeted specs under a single browser (`--project=firefox`). This report captures exactly what was verified before that instruction fully landed, stops all further local full-suite/multi-browser E2E work in compliance with the now-codified policy, and hands back to Management for commit/push so CI can authoritatively confirm cross-browser health. **Do not treat the gates below as a complete DoD sweep** โ€” several were not (re-)run this pass and are marked accordingly, distinct from gates that were actually executed with evidence in this session. No further local full-suite Playwright runs will follow this report. -### 1.1 Positive-control test: does the gate actually gate? (Most important check โ€” PASS) +--- -Constructed a minimal Go file containing an unguarded `websocket.Upgrader{}.Upgrade()` call (the same rule ID, `go.gorilla.security.audit.websocket-missing-origin-check`, that appears โ€” suppressed โ€” in the real codebase), and ran it through the **actual, unmodified** wrapper script with a single-file target: +## Gate-by-Gate Status (this pass) + +| # | Gate | Status | Detail | +|---|------|--------|--------| +| 1 | E2E Docker image clean rebuild | โœ… **VERIFIED** | 3 separate clean rebuilds run this pass (one per browser cycle, matching prior methodology). Changelog-fixture injection confirmed working (`FIXTURE Injecting E2E changelog fixture` โ†’ build โ†’ `FIXTURE Reverting changelog fixture overwrite`, working tree left clean). Caddy proxy port auto-sync confirmed (`PLAYWRIGHT_CADDY_PROXY_PORT already set to 8180 in .env` on every rebuild). | +| 2 | Playwright โ€” chromium + security-tests | โœ… **COMPLETED** (see note) | **1354 passed, 2 failed, 42 skipped** (fresh clean-rebuilt container). The 2 failures are **new findings**, not part of the previously-documented 52 โ€” see ยง1 below. Neither is caused by the 19 commits in scope. **Note**: this was a full-suite run under one browser, executed before the CI-only policy (`75c63696`) was fully in effect for this session. Retained here as useful evidence since it already ran to completion, but this is the **last** local full-suite run this pass โ€” not to be repeated. | +| 3 | Playwright โ€” firefox | โš ๏ธ **STOPPED MID-RUN**, then **halted entirely per codified CI-only policy** | 673 of ~955 tests completed, **0 failures observed** before stop (all โœ“ or expected skips). Includes `tests/settings/ntfy-notification-provider.spec.ts:564` (one of the two explicitly flagged uncertain items) โ€” **passed**. Suite was killed cleanly (process group terminated, no orphaned processes left running) partway through `tests/settings/pushover-notification-provider.spec.ts`, once the process-change instruction landed. Not a completed, authoritative run โ€” do not read "0 failures so far" as a clean bill of health for the untested remainder. **This run will not be resumed or repeated locally** โ€” full-suite/multi-browser confirmation is CI's job per `75c63696`. | +| 4 | Playwright โ€” webkit | โ›” **NOT RUN, and will not be run locally** | Never started, per the now-codified CI-only policy for full-suite runs. `tests/core/caddy-import/caddy-import-webkit.spec.ts:173` (flagged, no root cause found/no fix applied) and the ImportSession per-user-scope race question remain **unverified**; CI's webkit job is now the only path to confirming or refuting them. | +| 5 | Backend coverage (`go-test-coverage.sh`) | โ›” **NOT RUN this pass** | `backend/coverage.txt` on disk is stale (timestamped before this session). No fresh number to report. | +| 6 | Frontend coverage (`frontend-test-coverage.sh`) | โ›” **NOT RUN this pass** | `frontend/coverage/lcov.info` absent. No fresh number to report. | +| 7 | Local patch coverage (`local-patch-report.sh`) | โ›” **NOT RUN this pass** | `test-results/local-patch-report.{md,json}` absent. | +| 8 | Lefthook pre-commit (staticcheck, CodeQL Go+JS, semgrep) | โ›” **NOT RUN this pass** | | +| 9 | Trivy scan | โ›” **NOT RUN this pass** | Prior pass reported 0 new Critical/High with 1 pre-existing tracked HIGH (`CVE-2026-32286`, documented in `SECURITY.md`); not re-confirmed this session. No dependency changes in the 19-commit range (all `fix(test)`/`fix` commits touching `tests/`, `frontend/src/App.tsx`, `frontend/src/pages/Login.tsx`, `.env`/rebuild scripts โ€” no `go.mod`/`go.sum`/`package.json` changes), so risk of drift is low but **not independently re-confirmed**. | +| 10 | `git diff dac267f3..HEAD --stat -- backend/ \| grep '\.go$'` (GORM scan applicability) | โœ… **VERIFIED** | Empty. Confirmed no `.go` files changed in the 19-commit range โ€” GORM scan correctly skippable. | +| 11 | Frontend type-check / build / backend build | โ›” **NOT RUN this pass** | | +| 12 | Full `npx vitest run` | โ›” **NOT RUN this pass** | | +| 13 | Tautology grep (`grep -rn "\|\| true" tests/ --include=*.spec.ts`) | โœ… **VERIFIED** | 0 matches. Cheap, non-Playwright check โ€” run to completion. | -``` -SEMGREP_SARIF_OUTPUT=out.sarif bash scripts/pre-commit-hooks/semgrep-scan.sh - โ†’ Findings: 1 (1 blocking) โ†’ exit 0 (SARIF mode does not hard-fail) +--- -bash scripts/pre-commit-hooks/semgrep-scan.sh - โ†’ Findings: 1 (1 blocking) โ†’ exit 1 (--error mode hard-fails) -``` +## 1. New finding: 2 accessibility failures in chromium run (not part of the original 52, not caused by the 19 commits) + +**`tests/a11y/dns-providers.a11y.spec.ts:8`** and **`tests/a11y/security.a11y.spec.ts:59`** failed in the full chromium+security-tests run with genuine axe-core violations: + +- **DNS Providers page** โ€” `[CRITICAL] button-name`: 2 icon-only delete buttons (`Trash2` icon, no text/`aria-label`) have no accessible name. Root cause: `frontend/src/components/DNSProviderCard.tsx:187-191` โ€” `` with no `aria-label`. +- **Security dashboard page** โ€” `[CRITICAL] select-name`: 2 `` elements have no associated `