From 2ac68c43481063916ab5e430488b357c5ef9c12b Mon Sep 17 00:00:00 2001 From: chodeus Date: Tue, 11 Aug 2026 14:37:59 +0800 Subject: [PATCH 1/8] =?UTF-8?q?ci:=20harden=20pipeline=20=E2=80=94=20guard?= =?UTF-8?q?=20rewrite,=20develop-invariant=20job,=20least-privilege=20perm?= =?UTF-8?q?s=20(#501)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Branch filters '*'->'**', branch-isolation-guard rewrite (icase + poster_self_heal + fonts), new develop-invariant-guard, least-privilege permissions, Python 3.13, on-branch-delete injection + latent-skip-bug fix, on-branch-create timeout/concurrency, dep-audit Dockerfile-trigger drop, sync-develop @coderabbitai ignore, and a JSON-asset parse test. --- .github/workflows/codeql-lint.yml | 83 ++++++++++++++++++++++---- .github/workflows/dep-audit.yml | 3 +- .github/workflows/on-branch-create.yml | 6 ++ .github/workflows/on-branch-delete.yml | 24 ++++---- .github/workflows/sync-develop.yml | 10 +++- README.md | 2 +- pyproject.toml | 7 ++- tests/test_json_assets.py | 36 +++++++++++ 8 files changed, 141 insertions(+), 30 deletions(-) create mode 100644 tests/test_json_assets.py diff --git a/.github/workflows/codeql-lint.yml b/.github/workflows/codeql-lint.yml index 5773a4de..d6319fd5 100644 --- a/.github/workflows/codeql-lint.yml +++ b/.github/workflows/codeql-lint.yml @@ -4,7 +4,7 @@ on: workflow_dispatch: push: branches: - - '*' + - '**' tags: - 'v*' paths: @@ -21,13 +21,14 @@ on: - 'deploy/docker/compose.yaml' - 'frontend/package-lock.json' - 'deploy/docker/Dockerfile' + - 'deploy/docker/**' - '.dockerignore' - 'scripts/start.sh' - '.release-please-manifest.json' - '.github/workflows/codeql-lint.yml' pull_request: branches: - - '*' + - '**' paths: - '**.py' - '**.js' @@ -42,6 +43,7 @@ on: - 'deploy/docker/compose.yaml' - 'frontend/package-lock.json' - 'deploy/docker/Dockerfile' + - 'deploy/docker/**' - '.dockerignore' - 'scripts/start.sh' - '.release-please-manifest.json' @@ -50,9 +52,7 @@ on: - cron: '25 4 * * 1' # Weekly on Monday at 04:25 UTC permissions: - security-events: write contents: read - packages: write concurrency: group: codeql-lint-${{ github.ref }} @@ -63,6 +63,9 @@ jobs: codeql-python: name: CodeQL - Python runs-on: ubuntu-latest + permissions: + security-events: write + contents: read steps: - name: Checkout code @@ -82,6 +85,9 @@ jobs: codeql-javascript: name: CodeQL - JavaScript runs-on: ubuntu-latest + permissions: + security-events: write + contents: read steps: - name: Checkout code @@ -106,11 +112,13 @@ jobs: steps: - name: Checkout code uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 + with: + persist-credentials: false - name: Setup Python uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7 with: - python-version: '3.11' + python-version: '3.13' cache: 'pip' - name: Install linting tools @@ -127,11 +135,13 @@ jobs: steps: - name: Checkout code uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 + with: + persist-credentials: false - name: Setup Python uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7 with: - python-version: '3.11' + python-version: '3.13' cache: 'pip' - name: Install dependencies @@ -222,9 +232,14 @@ jobs: echo "Not a main-bound ref — branch-isolation check not applicable." exit 0 fi - leaks=$(git ls-files -- '*cl2k*' \ + # :(icase) — git globs are case-sensitive (else Cl2kMakerPage.jsx slips + # through); cover both extensions dirs, poster_self_heal, and the fonts. + leaks=$(git ls-files -- \ + ':(icase)*cl2k*' \ + ':(icase)*poster_self_heal*' ':(icase)*posterselfheal*' ':(icase)*posterheal*' \ 'backend/extensions/*' ':!backend/extensions/__init__.py' \ - 'frontend/src/extensions/*' ':!frontend/src/extensions/index.js') + 'frontend/src/extensions/*' ':!frontend/src/extensions/index.js' \ + 'deploy/docker/fonts/*') if [ -n "$leaks" ]; then echo "::error::Develop-only extension files found on a main-bound ref:" echo "$leaks" @@ -232,6 +247,49 @@ jobs: fi echo "OK: no develop-only extension code present." + # ---- Develop invariant guard ---- + # Mirror of branch-isolation-guard for the develop half: develop may differ from + # main ONLY by added files plus an append-only deploy/docker/Dockerfile. + develop-invariant-guard: + name: Develop Invariant Guard + # Always runs (no job-level if) so it stays a satisfiable needs on every ref; + # the step no-ops on non-develop-bound refs. + runs-on: ubuntu-latest + permissions: + contents: read + + steps: + - name: Checkout code + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 + with: + fetch-depth: 0 + + - name: Assert develop only adds files (shared files byte-identical to main) + run: | + set -eu + # Only develop-bound refs carry the extension delta; pass on any other ref. + if [ "${GITHUB_REF}" != "refs/heads/develop" ] && [ "${GITHUB_BASE_REF:-}" != "develop" ]; then + echo "Not a develop-bound ref — develop-invariant check not applicable." + exit 0 + fi + git fetch --quiet origin main + # Merge-base diff (origin/main...HEAD): main being ahead of an unsynced + # develop must not false-positive; only develop's own delta is inspected. + bad=$(git diff origin/main...HEAD --name-status \ + | grep -Ev '^A[[:space:]]' \ + | grep -Ev '^M[[:space:]]+deploy/docker/Dockerfile$' || true) + if [ -n "$bad" ]; then + echo "::error::develop diverges from main beyond added files + an append-only Dockerfile:" + echo "$bad" + exit 1 + fi + # The one allowed modification must be append-only: no main lines removed. + if git diff origin/main...HEAD -- deploy/docker/Dockerfile | grep -q '^-[^-]'; then + echo "::error::deploy/docker/Dockerfile removes lines present on main; develop must only append CL2K layers." + exit 1 + fi + echo "OK: develop differs from main only by added files + append-only Dockerfile." + # ---- Docker Build (gated by all quality checks) ---- docker-validate: name: Docker Validate (PR) @@ -240,7 +298,7 @@ jobs: # needs-failure SKIPS this job — which GitHub reports as Success for a # required check, turning a hard gate into a free pass. Promote # "Frontend Tests" in branch protection to make it block instead. - needs: [codeql-python, codeql-javascript, backend-lint, backend-smoke, frontend-lint, branch-isolation-guard] + needs: [codeql-python, codeql-javascript, backend-lint, backend-smoke, frontend-lint, branch-isolation-guard, develop-invariant-guard] runs-on: ubuntu-latest steps: @@ -277,9 +335,12 @@ jobs: docker-push: name: Docker Build & Push if: github.event_name == 'push' || github.event_name == 'workflow_dispatch' - needs: [codeql-python, codeql-javascript, backend-lint, backend-smoke, frontend-lint, frontend-tests, branch-isolation-guard] + needs: [codeql-python, codeql-javascript, backend-lint, backend-smoke, frontend-lint, frontend-tests, branch-isolation-guard, develop-invariant-guard] runs-on: ubuntu-latest timeout-minutes: 45 # see release-please.yml docker-version + permissions: + contents: read + packages: write # Serialize with the release-please docker-version job so concurrent pushes # to the same :latest tag don't race on GHCR auth. cancel-in-progress is # false because both jobs publish artifacts we want to keep — they just @@ -341,7 +402,7 @@ jobs: labels: ${{ steps.meta.outputs.labels }} notify-failure: - needs: [codeql-python, codeql-javascript, backend-lint, backend-smoke, frontend-lint, frontend-tests, branch-isolation-guard, docker-push] + needs: [codeql-python, codeql-javascript, backend-lint, backend-smoke, frontend-lint, frontend-tests, branch-isolation-guard, develop-invariant-guard, docker-push] if: failure() && (github.event_name == 'push' || github.event_name == 'workflow_dispatch') uses: chodeus/chodeus-ops/.github/workflows/notify-discord.yml@aae195c19ec7069d2e91aca88107220ce807ef7e # main with: diff --git a/.github/workflows/dep-audit.yml b/.github/workflows/dep-audit.yml index 43ef8b6c..26a6fa11 100644 --- a/.github/workflows/dep-audit.yml +++ b/.github/workflows/dep-audit.yml @@ -10,10 +10,11 @@ on: schedule: - cron: "43 2 * * 1" pull_request: + # No Dockerfile here: this job scans lockfiles (below), not container base + # images, so triggering on it would imply base-image coverage it lacks. paths: - "requirements.txt" - "frontend/package-lock.json" - - "deploy/docker/Dockerfile" - ".github/workflows/dep-audit.yml" permissions: {} diff --git a/.github/workflows/on-branch-create.yml b/.github/workflows/on-branch-create.yml index 7a4aa16d..6513dc94 100755 --- a/.github/workflows/on-branch-create.yml +++ b/.github/workflows/on-branch-create.yml @@ -11,6 +11,12 @@ permissions: jobs: docker-tag: runs-on: ubuntu-latest + timeout-minutes: 45 + # Shares codeql-lint/release-please's ghcr-push group so same-ref builds take + # turns; caps a wedged arm64 build at 45m instead of the 6h default. + concurrency: + group: ghcr-push-${{ github.ref }} + cancel-in-progress: false # Run on `create` events for branches, and on manual dispatch (so a # CI fix can be re-tested against an existing branch without having # to delete + recreate it). diff --git a/.github/workflows/on-branch-delete.yml b/.github/workflows/on-branch-delete.yml index 551c5998..36ed4c8b 100755 --- a/.github/workflows/on-branch-delete.yml +++ b/.github/workflows/on-branch-delete.yml @@ -10,33 +10,33 @@ jobs: ghcr-delete-tag: runs-on: ubuntu-latest if: github.event.ref_type == 'branch' + # REF via env (not ${{ }} inlined into run:) so a crafted branch name can't inject shell. + env: + REF: ${{ github.event.ref }} steps: - - name: Get the deleted branch name - id: get_branch - run: echo "BRANCH_NAME=${{ github.event.ref }}" >> $GITHUB_OUTPUT - - name: Skip protected branches + id: guard run: | - BRANCH="${{ steps.get_branch.outputs.BRANCH_NAME }}" - if [[ "$BRANCH" == "main" || "$BRANCH" == "master" || "$BRANCH" == "experimental" ]]; then - echo "Skipping deletion for protected branch: $BRANCH" - exit 0 + if [[ "$REF" == "main" || "$REF" == "master" || "$REF" == "experimental" || "$REF" == "develop" ]]; then + echo "Skipping deletion for protected branch: $REF" + echo "protected=true" >> "$GITHUB_OUTPUT" fi - name: Delete tag from GHCR + if: steps.guard.outputs.protected != 'true' env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - TAG_NAME: ${{ steps.get_branch.outputs.BRANCH_NAME }} run: | REPO="${{ github.repository_owner }}/chub" DIGEST=$(curl -s -H "Authorization: Bearer $GH_TOKEN" \ - "https://ghcr.io/v2/${REPO}/manifests/${TAG_NAME}" \ + "https://ghcr.io/v2/${REPO}/manifests/${REF}" \ -I | grep -i 'docker-content-digest:' | awk '{print $2}' | tr -d '\r') if [[ -z "$DIGEST" ]]; then echo "Tag not found on GHCR" exit 0 fi - curl -s -X DELETE -H "Authorization: Bearer $GH_TOKEN" \ + # -f so an HTTP error is a real failure, and no `|| echo` mask so it fails the step. + curl -sf -X DELETE -H "Authorization: Bearer $GH_TOKEN" \ "https://ghcr.io/v2/${REPO}/manifests/${DIGEST}" \ - && echo "Deleted GHCR tag: ${TAG_NAME}" || echo "Failed to delete GHCR tag: ${TAG_NAME}" + && echo "Deleted GHCR tag: $REF" diff --git a/.github/workflows/sync-develop.yml b/.github/workflows/sync-develop.yml index 68cbcfb4..9661422c 100644 --- a/.github/workflows/sync-develop.yml +++ b/.github/workflows/sync-develop.yml @@ -3,8 +3,10 @@ name: Sync develop from main # Open a back-merge PR from main into develop on every push to main, so develop # (the public app + develop-only extensions) keeps receiving everything that lands # on main — releases, fixes, dependency bumps. The branch model's sync direction -# is main -> develop and shared files are byte-identical on both, so the merge -# should never conflict while the invariant holds. An already-open base:develop / +# is main -> develop and shared files are byte-identical on both EXCEPT +# deploy/docker/Dockerfile, which develop extends with CL2K layers — so that file +# is the one EXPECTED merge conflict; resolve it by taking main's FROM/base lines +# and keeping develop's appended CL2K layers. An already-open base:develop / # head:main PR auto-tracks main's tip, so we only create one when none is open. # # The PR is a NOTIFICATION that develop has drifted, not a mergeable PR: develop @@ -55,7 +57,9 @@ jobs: --base develop \ --head main \ --title "chore: sync develop with main" \ - --body "\`develop\` has drifted behind \`main\` (releases, fixes, dependency bumps). **Do not merge this PR** — it reports the drift, it does not fix it. + --body "@coderabbitai ignore + + \`develop\` has drifted behind \`main\` (releases, fixes, dependency bumps). **Do not merge this PR** — it reports the drift, it does not fix it. \`develop\` requires branches be up to date, and \`head:main\` can never satisfy that without pulling develop's extension files into main, which the branch invariant forbids. Squash or rebase would also leave \`main\` unreachable from \`develop\`, so this workflow would just open another PR next push. diff --git a/README.md b/README.md index 9837bb1e..230f29c8 100755 --- a/README.md +++ b/README.md @@ -9,7 +9,7 @@ A self-hosted, all-in-one media asset manager for your Plex/ARR stack. [![MIT License](https://img.shields.io/badge/license-MIT-463fbc?style=flat-square)](https://opensource.org/licenses/MIT) -[![Python](https://img.shields.io/badge/python-3.8%2B-1992f3?style=flat-square)](https://www.python.org/) +[![Python](https://img.shields.io/badge/python-3.13%2B-1992f3?style=flat-square)](https://www.python.org/) [![Docker Image](https://img.shields.io/badge/ghcr.io-chodeus%2Fchub-463fbc?style=flat-square&logo=docker&logoColor=white)](https://github.com/chodeus/chub/pkgs/container/chub) [![GitHub Issues](https://img.shields.io/github/issues/chodeus/chub?color=463fbc&style=flat-square)](https://github.com/chodeus/chub/issues) [![GitHub Stars](https://img.shields.io/github/stars/chodeus/chub?color=53e8f0&style=flat-square)](https://github.com/chodeus/chub/stargazers) diff --git a/pyproject.toml b/pyproject.toml index c8c7bb4d..c647037b 100755 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,3 +1,6 @@ +[project] +requires-python = ">=3.13" + [tool.isort] profile = "black" multi_line_output = 3 @@ -9,13 +12,13 @@ line_length = 88 # For Black, set target-version to the lowest Python version you support, e.g., "py39", "py310", "py311" [tool.black] line-length = 88 -target-version = ["py39"] +target-version = ["py313"] skip-string-normalization = false skip-magic-trailing-comma = false [tool.ruff] line-length = 88 -target-version = "py39" +target-version = "py313" [tool.ruff.lint] extend-select = [] diff --git a/tests/test_json_assets.py b/tests/test_json_assets.py new file mode 100644 index 00000000..e8e5c75a --- /dev/null +++ b/tests/test_json_assets.py @@ -0,0 +1,36 @@ +"""Assert every tracked JSON asset parses, and the release manifest is well-formed.""" + +import json +import re +import subprocess +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parents[1] +# package-lock.json is generated + huge; `npm ci` already validates it. +EXCLUDED = {"frontend/package-lock.json"} + + +def _tracked_json_files(): + """Return repo-relative paths of every git-tracked *.json outside EXCLUDED.""" + out = subprocess.check_output( + ["git", "ls-files", "*.json"], cwd=REPO_ROOT, text=True + ) + return [p for p in out.splitlines() if p and p not in EXCLUDED] + + +def test_all_tracked_json_assets_parse(): + """Every tracked JSON asset must be valid JSON.""" + failures = [] + for rel in _tracked_json_files(): + try: + json.loads((REPO_ROOT / rel).read_text()) + except (json.JSONDecodeError, OSError) as exc: + failures.append(f"{rel}: {exc}") + assert not failures, "Invalid JSON asset(s):\n" + "\n".join(failures) + + +def test_release_manifest_version_is_semver(): + """.release-please-manifest.json '.' is read at runtime by backend/util/version.py.""" + manifest = json.loads((REPO_ROOT / ".release-please-manifest.json").read_text()) + assert "." in manifest, "manifest missing '.' key" + assert re.fullmatch(r"\d+\.\d+\.\d+", manifest["."].strip()), manifest["."] From 314e799af9655f20b3a9aab524deeee055a42b7c Mon Sep 17 00:00:00 2001 From: chodeus Date: Tue, 11 Aug 2026 14:55:41 +0800 Subject: [PATCH 2/8] =?UTF-8?q?Revert=20"ci:=20harden=20pipeline=20?= =?UTF-8?q?=E2=80=94=20guard=20rewrite,=20develop-invariant=20job,=20least?= =?UTF-8?q?-privilege=20perms=20(#501)"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 2ac68c43481063916ab5e430488b357c5ef9c12b. --- .github/workflows/codeql-lint.yml | 83 ++++---------------------- .github/workflows/dep-audit.yml | 3 +- .github/workflows/on-branch-create.yml | 6 -- .github/workflows/on-branch-delete.yml | 24 ++++---- .github/workflows/sync-develop.yml | 10 +--- README.md | 2 +- pyproject.toml | 7 +-- tests/test_json_assets.py | 36 ----------- 8 files changed, 30 insertions(+), 141 deletions(-) delete mode 100644 tests/test_json_assets.py diff --git a/.github/workflows/codeql-lint.yml b/.github/workflows/codeql-lint.yml index d6319fd5..5773a4de 100644 --- a/.github/workflows/codeql-lint.yml +++ b/.github/workflows/codeql-lint.yml @@ -4,7 +4,7 @@ on: workflow_dispatch: push: branches: - - '**' + - '*' tags: - 'v*' paths: @@ -21,14 +21,13 @@ on: - 'deploy/docker/compose.yaml' - 'frontend/package-lock.json' - 'deploy/docker/Dockerfile' - - 'deploy/docker/**' - '.dockerignore' - 'scripts/start.sh' - '.release-please-manifest.json' - '.github/workflows/codeql-lint.yml' pull_request: branches: - - '**' + - '*' paths: - '**.py' - '**.js' @@ -43,7 +42,6 @@ on: - 'deploy/docker/compose.yaml' - 'frontend/package-lock.json' - 'deploy/docker/Dockerfile' - - 'deploy/docker/**' - '.dockerignore' - 'scripts/start.sh' - '.release-please-manifest.json' @@ -52,7 +50,9 @@ on: - cron: '25 4 * * 1' # Weekly on Monday at 04:25 UTC permissions: + security-events: write contents: read + packages: write concurrency: group: codeql-lint-${{ github.ref }} @@ -63,9 +63,6 @@ jobs: codeql-python: name: CodeQL - Python runs-on: ubuntu-latest - permissions: - security-events: write - contents: read steps: - name: Checkout code @@ -85,9 +82,6 @@ jobs: codeql-javascript: name: CodeQL - JavaScript runs-on: ubuntu-latest - permissions: - security-events: write - contents: read steps: - name: Checkout code @@ -112,13 +106,11 @@ jobs: steps: - name: Checkout code uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 - with: - persist-credentials: false - name: Setup Python uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7 with: - python-version: '3.13' + python-version: '3.11' cache: 'pip' - name: Install linting tools @@ -135,13 +127,11 @@ jobs: steps: - name: Checkout code uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 - with: - persist-credentials: false - name: Setup Python uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7 with: - python-version: '3.13' + python-version: '3.11' cache: 'pip' - name: Install dependencies @@ -232,14 +222,9 @@ jobs: echo "Not a main-bound ref — branch-isolation check not applicable." exit 0 fi - # :(icase) — git globs are case-sensitive (else Cl2kMakerPage.jsx slips - # through); cover both extensions dirs, poster_self_heal, and the fonts. - leaks=$(git ls-files -- \ - ':(icase)*cl2k*' \ - ':(icase)*poster_self_heal*' ':(icase)*posterselfheal*' ':(icase)*posterheal*' \ + leaks=$(git ls-files -- '*cl2k*' \ 'backend/extensions/*' ':!backend/extensions/__init__.py' \ - 'frontend/src/extensions/*' ':!frontend/src/extensions/index.js' \ - 'deploy/docker/fonts/*') + 'frontend/src/extensions/*' ':!frontend/src/extensions/index.js') if [ -n "$leaks" ]; then echo "::error::Develop-only extension files found on a main-bound ref:" echo "$leaks" @@ -247,49 +232,6 @@ jobs: fi echo "OK: no develop-only extension code present." - # ---- Develop invariant guard ---- - # Mirror of branch-isolation-guard for the develop half: develop may differ from - # main ONLY by added files plus an append-only deploy/docker/Dockerfile. - develop-invariant-guard: - name: Develop Invariant Guard - # Always runs (no job-level if) so it stays a satisfiable needs on every ref; - # the step no-ops on non-develop-bound refs. - runs-on: ubuntu-latest - permissions: - contents: read - - steps: - - name: Checkout code - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 - with: - fetch-depth: 0 - - - name: Assert develop only adds files (shared files byte-identical to main) - run: | - set -eu - # Only develop-bound refs carry the extension delta; pass on any other ref. - if [ "${GITHUB_REF}" != "refs/heads/develop" ] && [ "${GITHUB_BASE_REF:-}" != "develop" ]; then - echo "Not a develop-bound ref — develop-invariant check not applicable." - exit 0 - fi - git fetch --quiet origin main - # Merge-base diff (origin/main...HEAD): main being ahead of an unsynced - # develop must not false-positive; only develop's own delta is inspected. - bad=$(git diff origin/main...HEAD --name-status \ - | grep -Ev '^A[[:space:]]' \ - | grep -Ev '^M[[:space:]]+deploy/docker/Dockerfile$' || true) - if [ -n "$bad" ]; then - echo "::error::develop diverges from main beyond added files + an append-only Dockerfile:" - echo "$bad" - exit 1 - fi - # The one allowed modification must be append-only: no main lines removed. - if git diff origin/main...HEAD -- deploy/docker/Dockerfile | grep -q '^-[^-]'; then - echo "::error::deploy/docker/Dockerfile removes lines present on main; develop must only append CL2K layers." - exit 1 - fi - echo "OK: develop differs from main only by added files + append-only Dockerfile." - # ---- Docker Build (gated by all quality checks) ---- docker-validate: name: Docker Validate (PR) @@ -298,7 +240,7 @@ jobs: # needs-failure SKIPS this job — which GitHub reports as Success for a # required check, turning a hard gate into a free pass. Promote # "Frontend Tests" in branch protection to make it block instead. - needs: [codeql-python, codeql-javascript, backend-lint, backend-smoke, frontend-lint, branch-isolation-guard, develop-invariant-guard] + needs: [codeql-python, codeql-javascript, backend-lint, backend-smoke, frontend-lint, branch-isolation-guard] runs-on: ubuntu-latest steps: @@ -335,12 +277,9 @@ jobs: docker-push: name: Docker Build & Push if: github.event_name == 'push' || github.event_name == 'workflow_dispatch' - needs: [codeql-python, codeql-javascript, backend-lint, backend-smoke, frontend-lint, frontend-tests, branch-isolation-guard, develop-invariant-guard] + needs: [codeql-python, codeql-javascript, backend-lint, backend-smoke, frontend-lint, frontend-tests, branch-isolation-guard] runs-on: ubuntu-latest timeout-minutes: 45 # see release-please.yml docker-version - permissions: - contents: read - packages: write # Serialize with the release-please docker-version job so concurrent pushes # to the same :latest tag don't race on GHCR auth. cancel-in-progress is # false because both jobs publish artifacts we want to keep — they just @@ -402,7 +341,7 @@ jobs: labels: ${{ steps.meta.outputs.labels }} notify-failure: - needs: [codeql-python, codeql-javascript, backend-lint, backend-smoke, frontend-lint, frontend-tests, branch-isolation-guard, develop-invariant-guard, docker-push] + needs: [codeql-python, codeql-javascript, backend-lint, backend-smoke, frontend-lint, frontend-tests, branch-isolation-guard, docker-push] if: failure() && (github.event_name == 'push' || github.event_name == 'workflow_dispatch') uses: chodeus/chodeus-ops/.github/workflows/notify-discord.yml@aae195c19ec7069d2e91aca88107220ce807ef7e # main with: diff --git a/.github/workflows/dep-audit.yml b/.github/workflows/dep-audit.yml index 26a6fa11..43ef8b6c 100644 --- a/.github/workflows/dep-audit.yml +++ b/.github/workflows/dep-audit.yml @@ -10,11 +10,10 @@ on: schedule: - cron: "43 2 * * 1" pull_request: - # No Dockerfile here: this job scans lockfiles (below), not container base - # images, so triggering on it would imply base-image coverage it lacks. paths: - "requirements.txt" - "frontend/package-lock.json" + - "deploy/docker/Dockerfile" - ".github/workflows/dep-audit.yml" permissions: {} diff --git a/.github/workflows/on-branch-create.yml b/.github/workflows/on-branch-create.yml index 6513dc94..7a4aa16d 100755 --- a/.github/workflows/on-branch-create.yml +++ b/.github/workflows/on-branch-create.yml @@ -11,12 +11,6 @@ permissions: jobs: docker-tag: runs-on: ubuntu-latest - timeout-minutes: 45 - # Shares codeql-lint/release-please's ghcr-push group so same-ref builds take - # turns; caps a wedged arm64 build at 45m instead of the 6h default. - concurrency: - group: ghcr-push-${{ github.ref }} - cancel-in-progress: false # Run on `create` events for branches, and on manual dispatch (so a # CI fix can be re-tested against an existing branch without having # to delete + recreate it). diff --git a/.github/workflows/on-branch-delete.yml b/.github/workflows/on-branch-delete.yml index 36ed4c8b..551c5998 100755 --- a/.github/workflows/on-branch-delete.yml +++ b/.github/workflows/on-branch-delete.yml @@ -10,33 +10,33 @@ jobs: ghcr-delete-tag: runs-on: ubuntu-latest if: github.event.ref_type == 'branch' - # REF via env (not ${{ }} inlined into run:) so a crafted branch name can't inject shell. - env: - REF: ${{ github.event.ref }} steps: + - name: Get the deleted branch name + id: get_branch + run: echo "BRANCH_NAME=${{ github.event.ref }}" >> $GITHUB_OUTPUT + - name: Skip protected branches - id: guard run: | - if [[ "$REF" == "main" || "$REF" == "master" || "$REF" == "experimental" || "$REF" == "develop" ]]; then - echo "Skipping deletion for protected branch: $REF" - echo "protected=true" >> "$GITHUB_OUTPUT" + BRANCH="${{ steps.get_branch.outputs.BRANCH_NAME }}" + if [[ "$BRANCH" == "main" || "$BRANCH" == "master" || "$BRANCH" == "experimental" ]]; then + echo "Skipping deletion for protected branch: $BRANCH" + exit 0 fi - name: Delete tag from GHCR - if: steps.guard.outputs.protected != 'true' env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + TAG_NAME: ${{ steps.get_branch.outputs.BRANCH_NAME }} run: | REPO="${{ github.repository_owner }}/chub" DIGEST=$(curl -s -H "Authorization: Bearer $GH_TOKEN" \ - "https://ghcr.io/v2/${REPO}/manifests/${REF}" \ + "https://ghcr.io/v2/${REPO}/manifests/${TAG_NAME}" \ -I | grep -i 'docker-content-digest:' | awk '{print $2}' | tr -d '\r') if [[ -z "$DIGEST" ]]; then echo "Tag not found on GHCR" exit 0 fi - # -f so an HTTP error is a real failure, and no `|| echo` mask so it fails the step. - curl -sf -X DELETE -H "Authorization: Bearer $GH_TOKEN" \ + curl -s -X DELETE -H "Authorization: Bearer $GH_TOKEN" \ "https://ghcr.io/v2/${REPO}/manifests/${DIGEST}" \ - && echo "Deleted GHCR tag: $REF" + && echo "Deleted GHCR tag: ${TAG_NAME}" || echo "Failed to delete GHCR tag: ${TAG_NAME}" diff --git a/.github/workflows/sync-develop.yml b/.github/workflows/sync-develop.yml index 9661422c..68cbcfb4 100644 --- a/.github/workflows/sync-develop.yml +++ b/.github/workflows/sync-develop.yml @@ -3,10 +3,8 @@ name: Sync develop from main # Open a back-merge PR from main into develop on every push to main, so develop # (the public app + develop-only extensions) keeps receiving everything that lands # on main — releases, fixes, dependency bumps. The branch model's sync direction -# is main -> develop and shared files are byte-identical on both EXCEPT -# deploy/docker/Dockerfile, which develop extends with CL2K layers — so that file -# is the one EXPECTED merge conflict; resolve it by taking main's FROM/base lines -# and keeping develop's appended CL2K layers. An already-open base:develop / +# is main -> develop and shared files are byte-identical on both, so the merge +# should never conflict while the invariant holds. An already-open base:develop / # head:main PR auto-tracks main's tip, so we only create one when none is open. # # The PR is a NOTIFICATION that develop has drifted, not a mergeable PR: develop @@ -57,9 +55,7 @@ jobs: --base develop \ --head main \ --title "chore: sync develop with main" \ - --body "@coderabbitai ignore - - \`develop\` has drifted behind \`main\` (releases, fixes, dependency bumps). **Do not merge this PR** — it reports the drift, it does not fix it. + --body "\`develop\` has drifted behind \`main\` (releases, fixes, dependency bumps). **Do not merge this PR** — it reports the drift, it does not fix it. \`develop\` requires branches be up to date, and \`head:main\` can never satisfy that without pulling develop's extension files into main, which the branch invariant forbids. Squash or rebase would also leave \`main\` unreachable from \`develop\`, so this workflow would just open another PR next push. diff --git a/README.md b/README.md index 230f29c8..9837bb1e 100755 --- a/README.md +++ b/README.md @@ -9,7 +9,7 @@ A self-hosted, all-in-one media asset manager for your Plex/ARR stack. [![MIT License](https://img.shields.io/badge/license-MIT-463fbc?style=flat-square)](https://opensource.org/licenses/MIT) -[![Python](https://img.shields.io/badge/python-3.13%2B-1992f3?style=flat-square)](https://www.python.org/) +[![Python](https://img.shields.io/badge/python-3.8%2B-1992f3?style=flat-square)](https://www.python.org/) [![Docker Image](https://img.shields.io/badge/ghcr.io-chodeus%2Fchub-463fbc?style=flat-square&logo=docker&logoColor=white)](https://github.com/chodeus/chub/pkgs/container/chub) [![GitHub Issues](https://img.shields.io/github/issues/chodeus/chub?color=463fbc&style=flat-square)](https://github.com/chodeus/chub/issues) [![GitHub Stars](https://img.shields.io/github/stars/chodeus/chub?color=53e8f0&style=flat-square)](https://github.com/chodeus/chub/stargazers) diff --git a/pyproject.toml b/pyproject.toml index c647037b..c8c7bb4d 100755 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,3 @@ -[project] -requires-python = ">=3.13" - [tool.isort] profile = "black" multi_line_output = 3 @@ -12,13 +9,13 @@ line_length = 88 # For Black, set target-version to the lowest Python version you support, e.g., "py39", "py310", "py311" [tool.black] line-length = 88 -target-version = ["py313"] +target-version = ["py39"] skip-string-normalization = false skip-magic-trailing-comma = false [tool.ruff] line-length = 88 -target-version = "py313" +target-version = "py39" [tool.ruff.lint] extend-select = [] diff --git a/tests/test_json_assets.py b/tests/test_json_assets.py deleted file mode 100644 index e8e5c75a..00000000 --- a/tests/test_json_assets.py +++ /dev/null @@ -1,36 +0,0 @@ -"""Assert every tracked JSON asset parses, and the release manifest is well-formed.""" - -import json -import re -import subprocess -from pathlib import Path - -REPO_ROOT = Path(__file__).resolve().parents[1] -# package-lock.json is generated + huge; `npm ci` already validates it. -EXCLUDED = {"frontend/package-lock.json"} - - -def _tracked_json_files(): - """Return repo-relative paths of every git-tracked *.json outside EXCLUDED.""" - out = subprocess.check_output( - ["git", "ls-files", "*.json"], cwd=REPO_ROOT, text=True - ) - return [p for p in out.splitlines() if p and p not in EXCLUDED] - - -def test_all_tracked_json_assets_parse(): - """Every tracked JSON asset must be valid JSON.""" - failures = [] - for rel in _tracked_json_files(): - try: - json.loads((REPO_ROOT / rel).read_text()) - except (json.JSONDecodeError, OSError) as exc: - failures.append(f"{rel}: {exc}") - assert not failures, "Invalid JSON asset(s):\n" + "\n".join(failures) - - -def test_release_manifest_version_is_semver(): - """.release-please-manifest.json '.' is read at runtime by backend/util/version.py.""" - manifest = json.loads((REPO_ROOT / ".release-please-manifest.json").read_text()) - assert "." in manifest, "manifest missing '.' key" - assert re.fullmatch(r"\d+\.\d+\.\d+", manifest["."].strip()), manifest["."] From 4009c1ca3c6533f70ff06ee108bc0e2b039cec87 Mon Sep 17 00:00:00 2001 From: chodeus Date: Tue, 11 Aug 2026 18:25:57 +0800 Subject: [PATCH 3/8] =?UTF-8?q?ci:=20harden=20pipeline=20=E2=80=94=20guard?= =?UTF-8?q?=20rewrite,=20develop-invariant=20job,=20least-privilege=20perm?= =?UTF-8?q?s=20(#503)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Re-land of #501 through a full CodeRabbit review. Branch filters '*'->'**', branch-isolation-guard rewrite (icase + poster_self_heal + fonts), develop-invariant-guard with a byte-prefix Dockerfile check, least-privilege permissions, Python 3.13, on-branch-delete injection/latent-skip/status-handling fixes with bounded curls, on-branch-create timeout + event.ref concurrency, dep-audit Dockerfile-trigger drop, sync-develop @coderabbitai ignore, JSON-asset parse test. --- .github/workflows/codeql-lint.yml | 88 ++++++++++++++++++++++---- .github/workflows/dep-audit.yml | 3 +- .github/workflows/on-branch-create.yml | 6 ++ .github/workflows/on-branch-delete.yml | 61 +++++++++++++----- .github/workflows/sync-develop.yml | 10 ++- README.md | 2 +- pyproject.toml | 4 +- tests/test_json_assets.py | 38 +++++++++++ 8 files changed, 177 insertions(+), 35 deletions(-) create mode 100644 tests/test_json_assets.py diff --git a/.github/workflows/codeql-lint.yml b/.github/workflows/codeql-lint.yml index 5773a4de..805e6fc6 100644 --- a/.github/workflows/codeql-lint.yml +++ b/.github/workflows/codeql-lint.yml @@ -4,7 +4,7 @@ on: workflow_dispatch: push: branches: - - '*' + - '**' tags: - 'v*' paths: @@ -21,13 +21,14 @@ on: - 'deploy/docker/compose.yaml' - 'frontend/package-lock.json' - 'deploy/docker/Dockerfile' + - 'deploy/docker/**' - '.dockerignore' - 'scripts/start.sh' - '.release-please-manifest.json' - '.github/workflows/codeql-lint.yml' pull_request: branches: - - '*' + - '**' paths: - '**.py' - '**.js' @@ -42,6 +43,7 @@ on: - 'deploy/docker/compose.yaml' - 'frontend/package-lock.json' - 'deploy/docker/Dockerfile' + - 'deploy/docker/**' - '.dockerignore' - 'scripts/start.sh' - '.release-please-manifest.json' @@ -50,9 +52,7 @@ on: - cron: '25 4 * * 1' # Weekly on Monday at 04:25 UTC permissions: - security-events: write contents: read - packages: write concurrency: group: codeql-lint-${{ github.ref }} @@ -63,6 +63,9 @@ jobs: codeql-python: name: CodeQL - Python runs-on: ubuntu-latest + permissions: + security-events: write + contents: read steps: - name: Checkout code @@ -82,6 +85,9 @@ jobs: codeql-javascript: name: CodeQL - JavaScript runs-on: ubuntu-latest + permissions: + security-events: write + contents: read steps: - name: Checkout code @@ -106,11 +112,13 @@ jobs: steps: - name: Checkout code uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 + with: + persist-credentials: false - name: Setup Python uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7 with: - python-version: '3.11' + python-version: '3.13' cache: 'pip' - name: Install linting tools @@ -127,11 +135,13 @@ jobs: steps: - name: Checkout code uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 + with: + persist-credentials: false - name: Setup Python uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7 with: - python-version: '3.11' + python-version: '3.13' cache: 'pip' - name: Install dependencies @@ -222,9 +232,14 @@ jobs: echo "Not a main-bound ref — branch-isolation check not applicable." exit 0 fi - leaks=$(git ls-files -- '*cl2k*' \ + # :(icase) — git globs are case-sensitive (else Cl2kMakerPage.jsx slips + # through); cover both extensions dirs, poster_self_heal, and the fonts. + leaks=$(git ls-files -- \ + ':(icase)*cl2k*' \ + ':(icase)*poster_self_heal*' ':(icase)*posterselfheal*' ':(icase)*posterheal*' \ 'backend/extensions/*' ':!backend/extensions/__init__.py' \ - 'frontend/src/extensions/*' ':!frontend/src/extensions/index.js') + 'frontend/src/extensions/*' ':!frontend/src/extensions/index.js' \ + 'deploy/docker/fonts/*') if [ -n "$leaks" ]; then echo "::error::Develop-only extension files found on a main-bound ref:" echo "$leaks" @@ -232,6 +247,54 @@ jobs: fi echo "OK: no develop-only extension code present." + # ---- Develop invariant guard ---- + # Mirror of branch-isolation-guard for the develop half: develop may differ from + # main ONLY by added files plus an append-only deploy/docker/Dockerfile. + develop-invariant-guard: + name: Develop Invariant Guard + # Always runs (no job-level if) so it stays a satisfiable needs on every ref; + # the step no-ops on non-develop-bound refs. + runs-on: ubuntu-latest + permissions: + contents: read + + steps: + - name: Checkout code + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 + with: + fetch-depth: 0 + + - name: Assert develop only adds files (shared files byte-identical to main) + run: | + set -eu + # Only develop-bound refs carry the extension delta; pass on any other ref. + if [ "${GITHUB_REF}" != "refs/heads/develop" ] && [ "${GITHUB_BASE_REF:-}" != "develop" ]; then + echo "Not a develop-bound ref — develop-invariant check not applicable." + exit 0 + fi + git fetch --quiet origin main + # Merge-base diff (origin/main...HEAD): main being ahead of an unsynced + # develop must not false-positive; only develop's own delta is inspected. + bad=$(git diff origin/main...HEAD --name-status \ + | grep -Ev '^A[[:space:]]' \ + | grep -Ev '^M[[:space:]]+deploy/docker/Dockerfile$' || true) + if [ -n "$bad" ]; then + echo "::error::develop diverges from main beyond added files + an append-only Dockerfile:" + echo "$bad" + exit 1 + fi + # Append-only = main's Dockerfile is an exact byte-PREFIX of develop's; a + # mid-file insertion adds no `-` line, so a diff `^-` check would miss it. + base="$(git merge-base origin/main HEAD)" + base_df="$(mktemp)" + git show "$base:deploy/docker/Dockerfile" > "$base_df" + n=$(wc -c < "$base_df") + if ! head -c "$n" deploy/docker/Dockerfile | cmp -s "$base_df" -; then + echo "::error::deploy/docker/Dockerfile is not append-only vs main (mid-file edit or removal); develop must only append CL2K layers." + exit 1 + fi + echo "OK: develop differs from main only by added files + append-only Dockerfile." + # ---- Docker Build (gated by all quality checks) ---- docker-validate: name: Docker Validate (PR) @@ -240,7 +303,7 @@ jobs: # needs-failure SKIPS this job — which GitHub reports as Success for a # required check, turning a hard gate into a free pass. Promote # "Frontend Tests" in branch protection to make it block instead. - needs: [codeql-python, codeql-javascript, backend-lint, backend-smoke, frontend-lint, branch-isolation-guard] + needs: [codeql-python, codeql-javascript, backend-lint, backend-smoke, frontend-lint, branch-isolation-guard, develop-invariant-guard] runs-on: ubuntu-latest steps: @@ -277,9 +340,12 @@ jobs: docker-push: name: Docker Build & Push if: github.event_name == 'push' || github.event_name == 'workflow_dispatch' - needs: [codeql-python, codeql-javascript, backend-lint, backend-smoke, frontend-lint, frontend-tests, branch-isolation-guard] + needs: [codeql-python, codeql-javascript, backend-lint, backend-smoke, frontend-lint, frontend-tests, branch-isolation-guard, develop-invariant-guard] runs-on: ubuntu-latest timeout-minutes: 45 # see release-please.yml docker-version + permissions: + contents: read + packages: write # Serialize with the release-please docker-version job so concurrent pushes # to the same :latest tag don't race on GHCR auth. cancel-in-progress is # false because both jobs publish artifacts we want to keep — they just @@ -341,7 +407,7 @@ jobs: labels: ${{ steps.meta.outputs.labels }} notify-failure: - needs: [codeql-python, codeql-javascript, backend-lint, backend-smoke, frontend-lint, frontend-tests, branch-isolation-guard, docker-push] + needs: [codeql-python, codeql-javascript, backend-lint, backend-smoke, frontend-lint, frontend-tests, branch-isolation-guard, develop-invariant-guard, docker-push] if: failure() && (github.event_name == 'push' || github.event_name == 'workflow_dispatch') uses: chodeus/chodeus-ops/.github/workflows/notify-discord.yml@aae195c19ec7069d2e91aca88107220ce807ef7e # main with: diff --git a/.github/workflows/dep-audit.yml b/.github/workflows/dep-audit.yml index 43ef8b6c..26a6fa11 100644 --- a/.github/workflows/dep-audit.yml +++ b/.github/workflows/dep-audit.yml @@ -10,10 +10,11 @@ on: schedule: - cron: "43 2 * * 1" pull_request: + # No Dockerfile here: this job scans lockfiles (below), not container base + # images, so triggering on it would imply base-image coverage it lacks. paths: - "requirements.txt" - "frontend/package-lock.json" - - "deploy/docker/Dockerfile" - ".github/workflows/dep-audit.yml" permissions: {} diff --git a/.github/workflows/on-branch-create.yml b/.github/workflows/on-branch-create.yml index 7a4aa16d..2694f649 100755 --- a/.github/workflows/on-branch-create.yml +++ b/.github/workflows/on-branch-create.yml @@ -11,6 +11,12 @@ permissions: jobs: docker-tag: runs-on: ubuntu-latest + timeout-minutes: 45 # caps a wedged arm64 build at 45m instead of the 6h default + # `create` sets github.ref to the DEFAULT branch, so key the shared ghcr-push + # group off the created branch (event.ref); ref_name is the dispatch fallback. + concurrency: + group: ghcr-push-refs/heads/${{ github.event_name == 'create' && github.event.ref || github.ref_name }} + cancel-in-progress: false # Run on `create` events for branches, and on manual dispatch (so a # CI fix can be re-tested against an existing branch without having # to delete + recreate it). diff --git a/.github/workflows/on-branch-delete.yml b/.github/workflows/on-branch-delete.yml index 551c5998..c9393670 100755 --- a/.github/workflows/on-branch-delete.yml +++ b/.github/workflows/on-branch-delete.yml @@ -9,34 +9,61 @@ permissions: jobs: ghcr-delete-tag: runs-on: ubuntu-latest + timeout-minutes: 5 # a stalled GHCR request can't keep cleanup running if: github.event.ref_type == 'branch' + # Serialize with the create/push publisher for this branch (raw ref matches + # their ghcr-push group) so a delete never races a mid-publish. + concurrency: + group: ghcr-push-refs/heads/${{ github.event.ref }} + cancel-in-progress: false + # REF via env (not ${{ }} inlined into run:) so a crafted branch name can't inject shell. + env: + REF: ${{ github.event.ref }} steps: - - name: Get the deleted branch name - id: get_branch - run: echo "BRANCH_NAME=${{ github.event.ref }}" >> $GITHUB_OUTPUT - - name: Skip protected branches + id: guard run: | - BRANCH="${{ steps.get_branch.outputs.BRANCH_NAME }}" - if [[ "$BRANCH" == "main" || "$BRANCH" == "master" || "$BRANCH" == "experimental" ]]; then - echo "Skipping deletion for protected branch: $BRANCH" - exit 0 + if [[ "$REF" == "main" || "$REF" == "master" || "$REF" == "experimental" || "$REF" == "develop" ]]; then + echo "Skipping deletion for protected branch: $REF" + echo "protected=true" >> "$GITHUB_OUTPUT" fi - name: Delete tag from GHCR + if: steps.guard.outputs.protected != 'true' env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - TAG_NAME: ${{ steps.get_branch.outputs.BRANCH_NAME }} run: | REPO="${{ github.repository_owner }}/chub" - DIGEST=$(curl -s -H "Authorization: Bearer $GH_TOKEN" \ - "https://ghcr.io/v2/${REPO}/manifests/${TAG_NAME}" \ - -I | grep -i 'docker-content-digest:' | awk '{print $2}' | tr -d '\r') - if [[ -z "$DIGEST" ]]; then - echo "Tag not found on GHCR" + # Mirror on-branch-create's `/`->`-` tag sanitization so we target the tag + # that was actually published, not the raw branch name. + TAG="${REF//\//-}" + # Branch on the HTTP status, not an empty digest: 404 = nothing to delete; + # 401/429/5xx must fail loudly instead of being masked as "not found". + headers="$(mktemp)" + status=$(curl -s --connect-timeout 10 --max-time 30 -o /dev/null -D "$headers" -w '%{http_code}' \ + -H "Authorization: Bearer $GH_TOKEN" \ + -I "https://ghcr.io/v2/${REPO}/manifests/${TAG}") + if [[ "$status" == "404" ]]; then + echo "Tag not found on GHCR: $TAG" exit 0 fi - curl -s -X DELETE -H "Authorization: Bearer $GH_TOKEN" \ - "https://ghcr.io/v2/${REPO}/manifests/${DIGEST}" \ - && echo "Deleted GHCR tag: ${TAG_NAME}" || echo "Failed to delete GHCR tag: ${TAG_NAME}" + if [[ "$status" != 2* ]]; then + echo "::error::GHCR HEAD for $TAG failed with HTTP $status" + exit 1 + fi + DIGEST=$(grep -i '^docker-content-digest:' "$headers" | awk '{print $2}' | tr -d '\r' || true) + if [[ -z "$DIGEST" ]]; then + echo "::error::GHCR returned $status for $TAG but no docker-content-digest header" + exit 1 + fi + del_status=$(curl -s --connect-timeout 10 --max-time 30 -o /dev/null -w '%{http_code}' -X DELETE \ + -H "Authorization: Bearer $GH_TOKEN" \ + "https://ghcr.io/v2/${REPO}/manifests/${DIGEST}") + # A concurrent delete may have already removed it, so treat 404 as success too. + if [[ "$del_status" == 2* || "$del_status" == "404" ]]; then + echo "Deleted GHCR tag $TAG ($del_status)" + else + echo "::error::GHCR DELETE for $TAG ($DIGEST) failed with HTTP $del_status" + exit 1 + fi diff --git a/.github/workflows/sync-develop.yml b/.github/workflows/sync-develop.yml index 68cbcfb4..9661422c 100644 --- a/.github/workflows/sync-develop.yml +++ b/.github/workflows/sync-develop.yml @@ -3,8 +3,10 @@ name: Sync develop from main # Open a back-merge PR from main into develop on every push to main, so develop # (the public app + develop-only extensions) keeps receiving everything that lands # on main — releases, fixes, dependency bumps. The branch model's sync direction -# is main -> develop and shared files are byte-identical on both, so the merge -# should never conflict while the invariant holds. An already-open base:develop / +# is main -> develop and shared files are byte-identical on both EXCEPT +# deploy/docker/Dockerfile, which develop extends with CL2K layers — so that file +# is the one EXPECTED merge conflict; resolve it by taking main's FROM/base lines +# and keeping develop's appended CL2K layers. An already-open base:develop / # head:main PR auto-tracks main's tip, so we only create one when none is open. # # The PR is a NOTIFICATION that develop has drifted, not a mergeable PR: develop @@ -55,7 +57,9 @@ jobs: --base develop \ --head main \ --title "chore: sync develop with main" \ - --body "\`develop\` has drifted behind \`main\` (releases, fixes, dependency bumps). **Do not merge this PR** — it reports the drift, it does not fix it. + --body "@coderabbitai ignore + + \`develop\` has drifted behind \`main\` (releases, fixes, dependency bumps). **Do not merge this PR** — it reports the drift, it does not fix it. \`develop\` requires branches be up to date, and \`head:main\` can never satisfy that without pulling develop's extension files into main, which the branch invariant forbids. Squash or rebase would also leave \`main\` unreachable from \`develop\`, so this workflow would just open another PR next push. diff --git a/README.md b/README.md index 9837bb1e..230f29c8 100755 --- a/README.md +++ b/README.md @@ -9,7 +9,7 @@ A self-hosted, all-in-one media asset manager for your Plex/ARR stack. [![MIT License](https://img.shields.io/badge/license-MIT-463fbc?style=flat-square)](https://opensource.org/licenses/MIT) -[![Python](https://img.shields.io/badge/python-3.8%2B-1992f3?style=flat-square)](https://www.python.org/) +[![Python](https://img.shields.io/badge/python-3.13%2B-1992f3?style=flat-square)](https://www.python.org/) [![Docker Image](https://img.shields.io/badge/ghcr.io-chodeus%2Fchub-463fbc?style=flat-square&logo=docker&logoColor=white)](https://github.com/chodeus/chub/pkgs/container/chub) [![GitHub Issues](https://img.shields.io/github/issues/chodeus/chub?color=463fbc&style=flat-square)](https://github.com/chodeus/chub/issues) [![GitHub Stars](https://img.shields.io/github/stars/chodeus/chub?color=53e8f0&style=flat-square)](https://github.com/chodeus/chub/stargazers) diff --git a/pyproject.toml b/pyproject.toml index c8c7bb4d..35022d40 100755 --- a/pyproject.toml +++ b/pyproject.toml @@ -9,13 +9,13 @@ line_length = 88 # For Black, set target-version to the lowest Python version you support, e.g., "py39", "py310", "py311" [tool.black] line-length = 88 -target-version = ["py39"] +target-version = ["py313"] skip-string-normalization = false skip-magic-trailing-comma = false [tool.ruff] line-length = 88 -target-version = "py39" +target-version = "py313" [tool.ruff.lint] extend-select = [] diff --git a/tests/test_json_assets.py b/tests/test_json_assets.py new file mode 100644 index 00000000..05ed3593 --- /dev/null +++ b/tests/test_json_assets.py @@ -0,0 +1,38 @@ +"""Assert every tracked JSON asset parses, and the release manifest is well-formed.""" + +import json +import re +import subprocess +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parents[1] +# package-lock.json is generated + huge; `npm ci` already validates it. +EXCLUDED = {"frontend/package-lock.json"} + + +def _tracked_json_files(): + """Return repo-relative paths of every git-tracked *.json outside EXCLUDED.""" + out = subprocess.check_output( + ["git", "ls-files", "*.json"], cwd=REPO_ROOT, text=True + ) + return [p for p in out.splitlines() if p and p not in EXCLUDED] + + +def test_all_tracked_json_assets_parse(): + """Every tracked JSON asset must be valid JSON.""" + failures = [] + for rel in _tracked_json_files(): + try: + json.loads((REPO_ROOT / rel).read_text()) + except (json.JSONDecodeError, OSError) as exc: + failures.append(f"{rel}: {exc}") + assert not failures, "Invalid JSON asset(s):\n" + "\n".join(failures) + + +def test_release_manifest_is_stable_base_semver(): + """Manifest '.' is the X.Y.Z base backend/util/version.py appends branch/build to.""" + manifest = json.loads((REPO_ROOT / ".release-please-manifest.json").read_text()) + assert "." in manifest, "manifest missing '.' key" + # ASCII, no leading zeros; version.py splits this as the 3-component base. + semver = r"(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)" + assert re.fullmatch(semver, manifest["."].strip()), manifest["."] From b3a2b5908627c826d92b74b44e4088b9a5c91523 Mon Sep 17 00:00:00 2001 From: chodeus Date: Tue, 11 Aug 2026 18:56:39 +0800 Subject: [PATCH 4/8] =?UTF-8?q?fix(ci):=20develop-invariant=20guard=20?= =?UTF-8?q?=E2=80=94=20pure-insertion=20Dockerfile=20check,=20not=20byte-p?= =?UTF-8?q?refix=20(#505)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Restores the round-1 pure-insertion-hunks check; the byte-prefix variant false-failed legitimate develop (CL2K blocks are mid-file insertions by design), breaking sync PR #502's guard and develop image publishing. --- .github/workflows/codeql-lint.yml | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/.github/workflows/codeql-lint.yml b/.github/workflows/codeql-lint.yml index 805e6fc6..32b369b2 100644 --- a/.github/workflows/codeql-lint.yml +++ b/.github/workflows/codeql-lint.yml @@ -279,21 +279,17 @@ jobs: | grep -Ev '^A[[:space:]]' \ | grep -Ev '^M[[:space:]]+deploy/docker/Dockerfile$' || true) if [ -n "$bad" ]; then - echo "::error::develop diverges from main beyond added files + an append-only Dockerfile:" + echo "::error::develop diverges from main beyond added files + an insertion-only Dockerfile:" echo "$bad" exit 1 fi - # Append-only = main's Dockerfile is an exact byte-PREFIX of develop's; a - # mid-file insertion adds no `-` line, so a diff `^-` check would miss it. - base="$(git merge-base origin/main HEAD)" - base_df="$(mktemp)" - git show "$base:deploy/docker/Dockerfile" > "$base_df" - n=$(wc -c < "$base_df") - if ! head -c "$n" deploy/docker/Dockerfile | cmp -s "$base_df" -; then - echo "::error::deploy/docker/Dockerfile is not append-only vs main (mid-file edit or removal); develop must only append CL2K layers." + # Pure-insertion hunks: no main line removed or edited. CL2K blocks are + # inserted MID-FILE (per build stage), so a byte-prefix check would false-fail. + if git diff origin/main...HEAD -- deploy/docker/Dockerfile | grep -q '^-[^-]'; then + echo "::error::deploy/docker/Dockerfile removes or edits lines present on main; develop may only insert CL2K blocks." exit 1 fi - echo "OK: develop differs from main only by added files + append-only Dockerfile." + echo "OK: develop differs from main only by added files + an insertion-only Dockerfile." # ---- Docker Build (gated by all quality checks) ---- docker-validate: From 4155bc52b2759b75ebdaf84f85c12d16263d0c6f Mon Sep 17 00:00:00 2001 From: chodeus Date: Tue, 11 Aug 2026 20:40:19 +0800 Subject: [PATCH 5/8] fix(ci): correct branch-image workflow across the board (#504) Per-branch GHCR image feature fixed holistically: builds the CREATED branch on create events (github.ref resolves to the default branch there); collision-safe branch->tag encoding (clean names <=128 pass through, else truncate+digest) applied byte-identically in on-branch-create, on-branch-delete, and codeql-lint docker-push; deletion via the GitHub Packages API only when the target tag is the sole tag on its version, with explicit status handling and bounded curls. --- .github/workflows/codeql-lint.yml | 20 ++++- .github/workflows/on-branch-create.yml | 28 +++++-- .github/workflows/on-branch-delete.yml | 105 ++++++++++++++++++------- 3 files changed, 115 insertions(+), 38 deletions(-) diff --git a/.github/workflows/codeql-lint.yml b/.github/workflows/codeql-lint.yml index 32b369b2..a17c87f9 100644 --- a/.github/workflows/codeql-lint.yml +++ b/.github/workflows/codeql-lint.yml @@ -364,7 +364,23 @@ jobs: - name: Get the current branch name id: get_branch - run: echo "BRANCH_NAME=${GITHUB_REF#refs/heads/}" >> $GITHUB_OUTPUT + run: | + name="${GITHUB_REF#refs/heads/}" + # Emit a branch tag only on branch refs; encode it collision-safe to + # match on-branch-create/delete (clean names pass through, else digest). + if [[ "$GITHUB_REF" == refs/heads/* ]]; then IS_BRANCH=true; else IS_BRANCH=false; fi + sanitized="${name//[^A-Za-z0-9_.-]/-}" + sanitized="$(printf '%s' "$sanitized" | sed -E 's/^[.-]+//')" + if [ "$sanitized" = "$name" ] && [ -n "$sanitized" ] && [ "${#sanitized}" -le 128 ]; then + BRANCH_TAG="$sanitized" + else + hash="$(printf '%s' "$name" | sha256sum | cut -c1-12)" + base="${sanitized:0:100}"; [ -n "$base" ] || base="branch" + BRANCH_TAG="${base}-${hash}" + fi + echo "BRANCH_NAME=${name}" >> "$GITHUB_OUTPUT" + echo "BRANCH_TAG=${BRANCH_TAG}" >> "$GITHUB_OUTPUT" + echo "IS_BRANCH=${IS_BRANCH}" >> "$GITHUB_OUTPUT" - name: Set build number run: echo "BUILD_NUMBER=$(git rev-list --count HEAD)" >> $GITHUB_ENV @@ -382,7 +398,7 @@ jobs: with: images: ghcr.io/${{ github.repository_owner }}/chub tags: | - type=ref,event=branch + type=raw,value=${{ steps.get_branch.outputs.BRANCH_TAG }},enable=${{ steps.get_branch.outputs.IS_BRANCH }} type=semver,pattern={{version}} type=semver,pattern={{major}}.{{minor}} type=semver,pattern={{major}} diff --git a/.github/workflows/on-branch-create.yml b/.github/workflows/on-branch-create.yml index 2694f649..364516cc 100755 --- a/.github/workflows/on-branch-create.yml +++ b/.github/workflows/on-branch-create.yml @@ -21,11 +21,17 @@ jobs: # CI fix can be re-tested against an existing branch without having # to delete + recreate it). if: github.event_name == 'workflow_dispatch' || github.event.ref_type == 'branch' + # `create` sets GITHUB_REF to the default branch; REF is the real target + # (event.ref on create, ref_name on dispatch). Kept out of run: shells. + env: + REF: ${{ github.event_name == 'create' && github.event.ref || github.ref_name }} steps: - name: Checkout code uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 with: + # Build the CREATED branch, not the default one github.ref points at. + ref: ${{ github.event_name == 'create' && github.event.ref || github.ref_name }} fetch-depth: 0 - name: Set up QEMU @@ -37,13 +43,21 @@ jobs: - name: Get the new branch name id: get_branch run: | - BRANCH_NAME="${GITHUB_REF#refs/heads/}" - # Docker tags can't contain `/` — convention-style names like - # feat/foo or fix/bar would otherwise fail with "invalid reference - # format". Keep BRANCH_NAME as the human-readable build-arg, but - # expose a sanitized BRANCH_TAG for the registry tag. - BRANCH_TAG="${BRANCH_NAME//\//-}" - echo "BRANCH_NAME=${BRANCH_NAME}" >> "$GITHUB_OUTPUT" + # REF is the real branch (event.ref on create/delete, ref_name on + # dispatch); GITHUB_REF would be the default branch on create/delete. + name="${REF#refs/heads/}" + # Collision-safe tag (shared with codeql-lint docker-push): clean names + # <=128 chars pass through, else truncate+digest so feat/foo != feat-foo. + sanitized="${name//[^A-Za-z0-9_.-]/-}" + sanitized="$(printf '%s' "$sanitized" | sed -E 's/^[.-]+//')" + if [ "$sanitized" = "$name" ] && [ -n "$sanitized" ] && [ "${#sanitized}" -le 128 ]; then + BRANCH_TAG="$sanitized" + else + hash="$(printf '%s' "$name" | sha256sum | cut -c1-12)" + base="${sanitized:0:100}"; [ -n "$base" ] || base="branch" + BRANCH_TAG="${base}-${hash}" + fi + echo "BRANCH_NAME=${name}" >> "$GITHUB_OUTPUT" echo "BRANCH_TAG=${BRANCH_TAG}" >> "$GITHUB_OUTPUT" - name: Set build number diff --git a/.github/workflows/on-branch-delete.yml b/.github/workflows/on-branch-delete.yml index c9393670..c3c6bd23 100755 --- a/.github/workflows/on-branch-delete.yml +++ b/.github/workflows/on-branch-delete.yml @@ -29,41 +29,88 @@ jobs: echo "protected=true" >> "$GITHUB_OUTPUT" fi - - name: Delete tag from GHCR + - name: Resolve GHCR tag for the deleted branch + id: get_branch + if: steps.guard.outputs.protected != 'true' + run: | + # REF is the real branch (event.ref on create/delete, ref_name on + # dispatch); GITHUB_REF would be the default branch on create/delete. + name="${REF#refs/heads/}" + # Collision-safe tag (shared with codeql-lint docker-push): clean names + # <=128 chars pass through, else truncate+digest so feat/foo != feat-foo. + sanitized="${name//[^A-Za-z0-9_.-]/-}" + sanitized="$(printf '%s' "$sanitized" | sed -E 's/^[.-]+//')" + if [ "$sanitized" = "$name" ] && [ -n "$sanitized" ] && [ "${#sanitized}" -le 128 ]; then + BRANCH_TAG="$sanitized" + else + hash="$(printf '%s' "$name" | sha256sum | cut -c1-12)" + base="${sanitized:0:100}"; [ -n "$base" ] || base="branch" + BRANCH_TAG="${base}-${hash}" + fi + echo "BRANCH_NAME=${name}" >> "$GITHUB_OUTPUT" + echo "BRANCH_TAG=${BRANCH_TAG}" >> "$GITHUB_OUTPUT" + + - name: Delete tag version from GHCR (sole-reference safe) if: steps.guard.outputs.protected != 'true' env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + OWNER: ${{ github.repository_owner }} + TAG: ${{ steps.get_branch.outputs.BRANCH_TAG }} + PACKAGE: chub run: | - REPO="${{ github.repository_owner }}/chub" - # Mirror on-branch-create's `/`->`-` tag sanitization so we target the tag - # that was actually published, not the raw branch name. - TAG="${REF//\//-}" - # Branch on the HTTP status, not an empty digest: 404 = nothing to delete; - # 401/429/5xx must fail loudly instead of being masked as "not found". - headers="$(mktemp)" - status=$(curl -s --connect-timeout 10 --max-time 30 -o /dev/null -D "$headers" -w '%{http_code}' \ - -H "Authorization: Bearer $GH_TOKEN" \ - -I "https://ghcr.io/v2/${REPO}/manifests/${TAG}") - if [[ "$status" == "404" ]]; then - echo "Tag not found on GHCR: $TAG" - exit 0 + api="https://api.github.com" + hdr=(-H "Accept: application/vnd.github+json" + -H "Authorization: Bearer $GH_TOKEN" + -H "X-GitHub-Api-Version: 2022-11-28") + # GHCR packages hang off a user OR an org path; probe user first, then org. + scope="users/$OWNER" + probe=$(curl -s --connect-timeout 10 --max-time 30 -o /dev/null -w '%{http_code}' \ + "${hdr[@]}" "$api/$scope/packages/container/$PACKAGE/versions?per_page=1") + if [[ "$probe" == "404" ]]; then + scope="orgs/$OWNER" + probe=$(curl -s --connect-timeout 10 --max-time 30 -o /dev/null -w '%{http_code}' \ + "${hdr[@]}" "$api/$scope/packages/container/$PACKAGE/versions?per_page=1") + fi + if [[ "$probe" == "404" ]]; then + echo "Package $PACKAGE not found for $OWNER; nothing to delete."; exit 0 fi - if [[ "$status" != 2* ]]; then - echo "::error::GHCR HEAD for $TAG failed with HTTP $status" - exit 1 + if [[ "$probe" != 2* ]]; then + echo "::error::Listing $PACKAGE versions failed with HTTP $probe"; exit 1 fi - DIGEST=$(grep -i '^docker-content-digest:' "$headers" | awk '{print $2}' | tr -d '\r' || true) - if [[ -z "$DIGEST" ]]; then - echo "::error::GHCR returned $status for $TAG but no docker-content-digest header" - exit 1 + # A tag maps to exactly one version; page until we find the version whose + # tags include $TAG, capturing its id and full tag list. + vid=""; tags_json=""; page=1 + while [[ "$page" -le 100 ]]; do + body="$(mktemp)" + status=$(curl -s --connect-timeout 10 --max-time 30 -o "$body" -w '%{http_code}' \ + "${hdr[@]}" "$api/$scope/packages/container/$PACKAGE/versions?per_page=100&page=$page") + if [[ "$status" != 2* ]]; then + echo "::error::Listing $PACKAGE versions (page $page) failed with HTTP $status"; exit 1 + fi + match="$(jq -c --arg t "$TAG" '[.[] | select((.metadata.container.tags // []) | index($t))][0] // empty' "$body")" + if [[ -n "$match" ]]; then + vid="$(printf '%s' "$match" | jq -r '.id')" + tags_json="$(printf '%s' "$match" | jq -c '.metadata.container.tags // []')" + break + fi + if [[ "$(jq 'length' "$body")" -lt 100 ]]; then break; fi + page=$((page + 1)) + done + if [[ -z "$vid" ]]; then + echo "Tag not found on GHCR: $TAG; nothing to delete."; exit 0 + fi + # Deleting a version drops its manifest — shared-digest tags would go too, + # so only delete when $TAG is the SOLE tag on that version. + if [[ "$(printf '%s' "$tags_json" | jq 'length')" != "1" ]]; then + others="$(printf '%s' "$tags_json" | jq -r 'join(", ")')" + echo "Version $vid for $TAG also holds other tags: $others. Skipping to avoid removing them." + exit 0 fi - del_status=$(curl -s --connect-timeout 10 --max-time 30 -o /dev/null -w '%{http_code}' -X DELETE \ - -H "Authorization: Bearer $GH_TOKEN" \ - "https://ghcr.io/v2/${REPO}/manifests/${DIGEST}") - # A concurrent delete may have already removed it, so treat 404 as success too. - if [[ "$del_status" == 2* || "$del_status" == "404" ]]; then - echo "Deleted GHCR tag $TAG ($del_status)" + del=$(curl -s --connect-timeout 10 --max-time 30 -o /dev/null -w '%{http_code}' -X DELETE \ + "${hdr[@]}" "$api/$scope/packages/container/$PACKAGE/versions/$vid") + # A concurrent delete may have already removed it, so treat 404 as success. + if [[ "$del" == 2* || "$del" == "404" ]]; then + echo "Deleted GHCR version $vid ($TAG) [$del]" else - echo "::error::GHCR DELETE for $TAG ($DIGEST) failed with HTTP $del_status" - exit 1 + echo "::error::Deleting version $vid ($TAG) failed with HTTP $del"; exit 1 fi From 7881bffc1ca7c160524addb62771ae6220578e1e Mon Sep 17 00:00:00 2001 From: chodeus Date: Wed, 12 Aug 2026 01:03:44 +0800 Subject: [PATCH 6/8] fix(backend): correctness + perf batch (STATIC_DIR, LIKE escaping, fail-open guards, hot-path caching) (#506) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(backend): correctness + perf batch - /api/posters/list: resolve posters via the shared STATIC_DIR (get_static_dir in util/helper), not a hardcoded templates/ path that is empty in Docker - LIKE escaping: one escape_like() helper in db_base; fix the 3 unescaped sites (poster prefix guard, poster search, media tags) + ESCAPE clause, and collapse the copy-pasted escape chains into it - /api/media/duplicates: narrow the config fail-open to ConfigError + a filter_applied flag, so a config error no longer silently reports quality pairs - module_is_disabled: fail CLOSED on a config read error (skip the run) - schema sync: gate init_schema per db file (process-level), and pass the SSE poll's open db through get_module_status — ~43 ChubDB opens/tick -> 1 - load_config: cache the validated config keyed on mtime/size/inode (returns a deep copy; callers mutate before saving), cleared on save + watcher reload - posters.py: make the 3 blocking PIL/db routes plain def so Starlette offloads them, and drop the "without blocking the event loop" claim that wasn't true - move backup logic to util/backup.py so util/maintenance no longer imports up into the api layer * test: single import style for backend.util.config (CodeQL py/import-and-import-from) * fix(backend): address review findings - /api/media/duplicates: deny on ConfigError (shared CONFIG_INVALID handler) instead of returning unfiltered results; drop the filter_applied field; validate exclusion-group members before building sets - get_backup_dir: propagate ConfigError instead of silently falling back; backup routes re-raise so the shared handler answers - save_backup: exclusive creation with a collision suffix, and resolve the destination before paying for the dump - schema sync: hold the (now reentrant) registry lock across the DDL, and skip only non-empty known files so a recreated db re-syncs despite inode reuse - prune_old_backups: re-confine the resolved root against the allowed roots and lstat through dir_fd before unlinking - 1-line docstrings on every touched function still missing one * test: single import style per module (CodeQL py/import-and-import-from) * fix(backend): round-2 review findings on maintenance - prune_old_backups: config is now required — the re-confinement check can no longer be skipped by omission - maintenance loop: skip the pass when config won't load instead of running destructive pruning against the startup snapshot; drop the now-unused start_maintenance config param - trim the prune-loop safety comment to two lines * fix(backend): bind prune authorization to the opened descriptor Authorize the resolved root before opening it, verify fstat(dir_fd) matches stat(root), and enumerate through the descriptor with one nofollow stat per entry (CWE-367). --- backend/api/main.py | 9 +- backend/api/media_api.py | 65 +++++---- backend/api/modules.py | 8 +- backend/api/posters.py | 37 +++-- backend/api/system.py | 113 ++------------- backend/util/backup.py | 128 +++++++++++++++++ backend/util/config.py | 72 +++++++++- backend/util/database/__init__.py | 3 +- backend/util/database/db_base.py | 50 ++++++- backend/util/database/media_cache.py | 23 +-- backend/util/database/poster_cache.py | 31 +++-- backend/util/helper.py | 8 ++ backend/util/maintenance.py | 99 ++++++++----- backend/util/module_orchestrator.py | 16 ++- main.py | 4 + tests/test_backup_dir.py | 72 ++++++++-- tests/test_config.py | 39 ++++++ tests/test_database.py | 53 +++++++ tests/test_maintenance.py | 21 ++- tests/test_module_disable.py | 11 ++ tests/test_prune_backups_safety.py | 193 ++++++++++++++++++++++++-- tests/test_regression_review_2026.py | 138 ++++++++++++++++++ 22 files changed, 920 insertions(+), 273 deletions(-) create mode 100644 backend/util/backup.py diff --git a/backend/api/main.py b/backend/api/main.py index 5d6e5ed4..064cfc99 100755 --- a/backend/api/main.py +++ b/backend/api/main.py @@ -1,8 +1,6 @@ -import os import threading import time from contextlib import asynccontextmanager -from pathlib import Path from typing import Any, AsyncGenerator from fastapi import APIRouter, FastAPI, HTTPException, Request @@ -45,6 +43,7 @@ load_config, ) from backend.util.database import ChubDB +from backend.util.helper import get_static_dir from backend.util.job_processor import process_job from backend.util.notification import install_error_notify_handler @@ -302,7 +301,7 @@ def shared_db_process_job(job, logger): if "startup_config" in locals() and startup_config is not None: from backend.util.maintenance import start_maintenance - app.state.maintenance_thread = start_maintenance(startup_config, logger) + app.state.maintenance_thread = start_maintenance(logger) except Exception as e: if log: log.error(f"Failed to start maintenance thread: {e}") @@ -415,9 +414,7 @@ def stop_worker_with_timeout( # Frontend static directory — configurable via STATIC_DIR env var. # Defaults to templates/ (local dev); Docker sets STATIC_DIR=/app/public. -STATIC_DIR = Path( - os.environ.get("STATIC_DIR", str(Path(__file__).parents[2] / "templates")) -) +STATIC_DIR = get_static_dir() app.mount( "/assets", diff --git a/backend/api/media_api.py b/backend/api/media_api.py index 5a90ed96..2cb3c246 100644 --- a/backend/api/media_api.py +++ b/backend/api/media_api.py @@ -18,8 +18,8 @@ from backend.api.utils import error, get_database, get_logger, ok from backend.util.arr import create_arr_client -from backend.util.config import load_config -from backend.util.database import ChubDB +from backend.util.config import ConfigError, load_config +from backend.util.database import ChubDB, escape_like from backend.util.ssrf_guard import is_safe_url, safe_external_get router = APIRouter( @@ -464,35 +464,29 @@ async def get_duplicates( # quality group (configured in general.duplicate_exclude_groups). # e.g. [["radarr", "radarr4k"]] means radarr+radarr4k pairs # are intentional quality copies, not real duplicates. - try: - from backend.util.config import load_config - - raw_groups = load_config().general.duplicate_exclude_groups - if raw_groups: - # Normalise: accept both [["a","b"]] and [{"instances":["a","b"]}] - exclude_sets = [] - for g in raw_groups: - if isinstance(g, dict): - exclude_sets.append(set(g.get("instances", []))) - else: - exclude_sets.append(set(g)) - exclude_sets = [s for s in exclude_sets if len(s) >= 2] - - if exclude_sets: - - def _not_excluded(dup): - raw = dup.get("instances", "") - instances = {s.strip() for s in raw.split(",") if s.strip()} - return not any( - instances.issubset(group) for group in exclude_sets - ) - - duplicates = [d for d in duplicates if _not_excluded(d)] - folder_collisions = [ - d for d in folder_collisions if _not_excluded(d) - ] - except Exception: - pass # Config not loaded — skip filtering + raw_groups = load_config().general.duplicate_exclude_groups + if raw_groups: + # Normalise: accept both [["a","b"]] and [{"instances":["a","b"]}]. + # The field is List[Any] — skip malformed entries, don't raise. + exclude_sets = [] + for g in raw_groups: + members = g.get("instances") if isinstance(g, dict) else g + if isinstance(members, (list, tuple, set)) and all( + isinstance(m, str) and m.strip() for m in members + ): + exclude_sets.append({m.strip() for m in members}) + exclude_sets = [s for s in exclude_sets if len(s) >= 2] + + if exclude_sets: + + def _not_excluded(dup): + """True when this group isn't wholly inside one exclude set.""" + raw = dup.get("instances", "") + instances = {s.strip() for s in raw.split(",") if s.strip()} + return not any(instances.issubset(group) for group in exclude_sets) + + duplicates = [d for d in duplicates if _not_excluded(d)] + folder_collisions = [d for d in folder_collisions if _not_excluded(d)] return ok( f"Found {len(duplicates)} duplicate groups, " @@ -505,6 +499,10 @@ def _not_excluded(dup): }, ) + except ConfigError: + # Deny: unfiltered results report intentional quality pairs as + # duplicates next to a bulk-delete button. main.py's handler answers. + raise except Exception as e: logger.error(f"Error finding duplicates: {e}") return error( @@ -2037,6 +2035,7 @@ async def generate_collection_from_tag( logger: Any = Depends(get_logger), db: ChubDB = Depends(get_database), ) -> JSONResponse: + """Build a poster_collection from every media row carrying `tag`.""" try: payload = await request.json() tag = (payload.get("tag") or "").strip() @@ -2050,8 +2049,8 @@ async def generate_collection_from_tag( media_rows = ( db.worker.execute_query( "SELECT id, tmdb_id, tvdb_id, imdb_id, season_number, title, year " - "FROM media_cache WHERE tags LIKE ?", - (f"%{tag}%",), + "FROM media_cache WHERE tags LIKE ? ESCAPE '\\'", + (f"%{escape_like(tag)}%",), fetch_all=True, ) or [] diff --git a/backend/api/modules.py b/backend/api/modules.py index ce860f66..168d3525 100755 --- a/backend/api/modules.py +++ b/backend/api/modules.py @@ -383,7 +383,7 @@ async def get_all_run_states( from backend.modules import MODULES for mod_name in MODULES: - job_status = orchestrator.get_module_status(mod_name) + job_status = orchestrator.get_module_status(mod_name, db=db) if job_status.get("running"): if mod_name in states_by_name: states_by_name[mod_name]["status"] = "running" @@ -428,6 +428,7 @@ async def module_events(request: Request): from backend.modules import MODULES async def event_generator(): + """Yield an SSE frame whenever any module's run state changes.""" previous_states = {} try: while True: @@ -439,6 +440,7 @@ async def event_generator(): # the event loop — otherwise each tick blocks the loop (and all # other requests/streams) for the duration of the queries. def _poll_states(): + """Read every module's current run state from the db.""" states_by_name = {} with ChubDB(request.app.state.logger, quiet=True) as db: run_states = db.run_state.get_all() @@ -454,7 +456,9 @@ def _poll_states(): orchestrator = request.app.state.module_orchestrator if orchestrator: for mod_name in MODULES: - job_status = orchestrator.get_module_status(mod_name) + job_status = orchestrator.get_module_status( + mod_name, db=db + ) if job_status.get("running"): job_id = job_status.get("job_id") job = ( diff --git a/backend/api/posters.py b/backend/api/posters.py index 01b184d7..f87f8462 100644 --- a/backend/api/posters.py +++ b/backend/api/posters.py @@ -18,7 +18,8 @@ from backend.api.utils import error, get_database, get_logger, get_module_logger, ok from backend.modules.sync_gdrive import SyncGDrive from backend.modules.unmatched_assets import UnmatchedAssets -from backend.util.database import ChubDB +from backend.util.database import ChubDB, escape_like +from backend.util.helper import get_static_dir router = APIRouter( prefix="/api/posters", @@ -1175,7 +1176,7 @@ def _optimize_posters_sync( @router.get( "/list", summary="List available poster files", - description="List available poster files from the templates/posters directory.", + description="List available poster files from the static posters directory.", responses={ 200: { "description": "Poster files listed successfully", @@ -1193,7 +1194,7 @@ def _optimize_posters_sync( ) async def list_poster_files(logger: Any = Depends(get_logger)) -> JSONResponse: """ - List available poster files from templates/posters directory. + List available poster files from the static posters directory. Returns just the filenames for dynamic discovery by the frontend. Used for default poster selection and asset management. @@ -1204,7 +1205,9 @@ async def list_poster_files(logger: Any = Depends(get_logger)) -> JSONResponse: try: logger.debug("Serving GET /api/posters/list") - posters_dir = Path(__file__).parents[2] / "templates" / "posters" + # Same directory main.py mounts at /posters — resolved via STATIC_DIR, + # which Docker points at /app/public (templates/ isn't in the image). + posters_dir = get_static_dir() / "posters" allowed_extensions = {".jpg", ".jpeg", ".png", ".webp"} if not posters_dir.exists(): @@ -2810,13 +2813,16 @@ def upload_collection_posters( summary="Backfill poster width/height", description="Walk poster_cache rows missing width/height and populate " "them by opening the file with PIL. Processes up to `limit` rows per call " - "so it can be run incrementally without blocking the event loop.", + "so it can be run incrementally.", ) -async def backfill_poster_dimensions( +def backfill_poster_dimensions( limit: int = 200, logger: Any = Depends(get_logger), db: ChubDB = Depends(get_database), ) -> JSONResponse: + """Populate missing poster width/height for up to `limit` rows.""" + # Sync on purpose: the PIL/db loop blocks, so Starlette runs the whole + # endpoint in a threadpool instead of stalling the event loop. try: limit = max(1, min(limit, 2000)) rows = ( @@ -3624,7 +3630,7 @@ async def get_poster( 404: {"description": "Poster or file not found"}, }, ) -async def get_poster_thumbnail( +def get_poster_thumbnail( poster_id: int, width: int = Query(200, ge=50, le=500, description="Thumbnail width in pixels"), logger: Any = Depends(get_logger), @@ -3636,6 +3642,9 @@ async def get_poster_thumbnail( Generates a downsized JPEG thumbnail on first request and caches it in a .thumbnails subdirectory. Subsequent requests serve from cache. + Sync on purpose: the LANCZOS resize + JPEG encode are blocking, so + Starlette runs this in a threadpool instead of stalling the event loop. + Args: poster_id: The unique identifier of the poster width: Target thumbnail width (height scales proportionally) @@ -3710,7 +3719,7 @@ async def get_poster_thumbnail( 404: {"description": "Poster or file not found"}, }, ) -async def download_poster( +def download_poster( poster_id: int, size: Optional[int] = Query( None, ge=100, le=4000, description="Max dimension in pixels" @@ -3729,6 +3738,9 @@ async def download_poster( When size, format, or quality are specified, processes the image before serving. + Sync on purpose: the optional resize/encode is blocking, so Starlette + runs this in a threadpool instead of stalling the event loop. + Args: poster_id: The unique identifier of the poster to download size: Optional max dimension for resize @@ -3889,13 +3901,8 @@ async def delete_poster( # Find media items that were matched to this poster by original_file if full_path: # Escape LIKE metacharacters so a basename with %/_ can't - # unmatch the wrong media rows (mirror poster_cache). - basename_like = ( - os.path.basename(full_path) - .replace("\\", "\\\\") - .replace("%", "\\%") - .replace("_", "\\_") - ) + # unmatch the wrong media rows. + basename_like = escape_like(os.path.basename(full_path)) media_items = ( db.media.execute_query( "SELECT id, title, instance_name, asset_type, year, season_number FROM media_cache WHERE original_file LIKE ? ESCAPE '\\'", diff --git a/backend/api/system.py b/backend/api/system.py index 2c49a84e..885c082c 100755 --- a/backend/api/system.py +++ b/backend/api/system.py @@ -22,6 +22,7 @@ from starlette.concurrency import run_in_threadpool from backend.api.utils import error, get_database, get_logger, ok +from backend.util.backup import get_backup_dir, save_backup from backend.util.config import ( ConfigError, ChubConfig, @@ -552,107 +553,6 @@ async def test( # ==== Backup / Restore ==== -def _get_db_path() -> str: - """Get the SQLite database path.""" - config_dir = os.environ.get("CONFIG_DIR") or str( - Path(__file__).parents[2] / "config" - ) - return os.path.join(config_dir, "chub.db") - - -def _default_backup_dir() -> Path: - config_dir = os.environ.get("CONFIG_DIR") or str( - Path(__file__).parents[2] / "config" - ) - return Path(config_dir) / "backups" - - -def _get_backup_dir(logger: Any = None) -> Path: - """Resolve general.backup_dir, falling back to CONFIG_DIR/backups.""" - default = _default_backup_dir() - - configured = "" - config = None - try: - config = load_config() - configured = (getattr(config.general, "backup_dir", "") or "").strip() - except Exception as exc: - if logger: - logger.error(f"Could not read backup_dir from config: {exc}") - - if configured and config is not None: - if not is_path_allowed(configured, config): - if logger: - logger.error( - f"backup_dir '{configured}' is outside the allowed roots; " - f"backing up to {default} instead" - ) - else: - try: - target = Path(configured).expanduser().resolve() - target.mkdir(parents=True, exist_ok=True) - # Re-confine the RESOLVED target: is_path_allowed() authorised a - # path that a swapped symlink could since have re-pointed. - if is_path_allowed(str(target), config): - return target - if logger: - logger.error( - f"backup_dir '{configured}' resolved outside the allowed " - f"roots ({target}); backing up to {default} instead" - ) - except OSError as exc: - if logger: - logger.error( - f"backup_dir '{configured}' is not usable ({exc}); " - f"backing up to {default} instead" - ) - - default.mkdir(parents=True, exist_ok=True) - return default - - -def build_backup_bytes() -> bytes: - """Build a backup zip (config.yml + chub.db.sql dump) and return its bytes. - - Uses SQLite's backup API to safely snapshot the database while it may be in - use. Shared by the download endpoint and the auto-backup maintenance thread. - """ - config_path = get_config_path() - db_path = _get_db_path() - - buf = io.BytesIO() - with zipfile.ZipFile(buf, "w", zipfile.ZIP_DEFLATED) as zf: - if os.path.exists(config_path): - zf.write(config_path, "config.yml") - - if os.path.exists(db_path): - db_buf = io.BytesIO() - src = sqlite3.connect(db_path) - try: - mem = sqlite3.connect(":memory:") - src.backup(mem) - for line in mem.iterdump(): - db_buf.write(f"{line}\n".encode("utf-8")) - mem.close() - finally: - src.close() - db_buf.seek(0) - zf.writestr("chub.db.sql", db_buf.read()) - - return buf.getvalue() - - -def save_backup(logger: Any = None) -> Path: - """Write a timestamped backup into the backups directory; return its path.""" - data = build_backup_bytes() - timestamp = datetime.now().strftime("%Y%m%d-%H%M%S") - backup_path = _get_backup_dir(logger) / f"chub-backup-{timestamp}.zip" - backup_path.write_bytes(data) - if logger: - logger.info(f"Backup created: {backup_path.name}") - return backup_path - - @router.post( "/backup", summary="Create backup", @@ -680,6 +580,8 @@ def create_backup( headers={"Content-Disposition": f"attachment; filename={backup_path.name}"}, ) + except ConfigError: + raise # deny — main.py's handler returns CONFIG_INVALID except Exception as e: logger.error(f"Backup creation failed: {e}") return error( @@ -697,7 +599,7 @@ def create_backup( async def list_backups(logger: Any = Depends(get_logger)) -> JSONResponse: """List backup files in the backups directory.""" try: - backup_dir = _get_backup_dir() + backup_dir = get_backup_dir() backups = [] for f in sorted(backup_dir.glob("chub-backup-*.zip"), reverse=True): stat = f.stat() @@ -709,6 +611,8 @@ async def list_backups(logger: Any = Depends(get_logger)) -> JSONResponse: } ) return ok(f"Found {len(backups)} backups", {"backups": backups}) + except ConfigError: + raise # deny — main.py's handler returns CONFIG_INVALID except Exception as e: logger.error(f"Error listing backups: {e}") return error("Error listing backups", code="BACKUP_LIST_ERROR", status_code=500) @@ -757,6 +661,7 @@ async def restore_backup( # The zip parse + yaml validation + file writes are blocking; run them # off the event loop so a large upload doesn't stall the whole server. def _do_restore() -> JSONResponse: + """Validate the zip and write config.yml back; blocking.""" buf = io.BytesIO(content) if not zipfile.is_zipfile(buf): @@ -822,7 +727,7 @@ def _do_restore() -> JSONResponse: # If DB dump is included, save it for reference if "chub.db.sql" in names: - backup_dir = _get_backup_dir() + backup_dir = get_backup_dir() sql_path = backup_dir / "restored-db.sql" sql_path.write_bytes(zf.read("chub.db.sql")) restored_items.append( @@ -837,6 +742,8 @@ def _do_restore() -> JSONResponse: return await run_in_threadpool(_do_restore) + except ConfigError: + raise # deny — main.py's handler returns CONFIG_INVALID except Exception as e: logger.error(f"Restore failed: {e}") return error("Restore failed", code="RESTORE_ERROR", status_code=500) diff --git a/backend/util/backup.py b/backend/util/backup.py new file mode 100644 index 00000000..a0a33dfd --- /dev/null +++ b/backend/util/backup.py @@ -0,0 +1,128 @@ +"""Config + database backup archives. + +Owns where backups go and how they're built, so both the /api/backup routes and +the maintenance thread can use them without either importing the other. +""" + +import io +import os +import sqlite3 +import zipfile +from datetime import datetime +from pathlib import Path +from typing import Any + +from backend.util.config import get_config_path, load_config +from backend.util.path_safety import is_path_allowed + +# Suffixes tried when two backups land in the same second (-1 .. -N). +_SAVE_COLLISION_RETRIES = 100 + + +def _get_db_path() -> str: + """Get the SQLite database path.""" + config_dir = os.environ.get("CONFIG_DIR") or str( + Path(__file__).parents[2] / "config" + ) + return os.path.join(config_dir, "chub.db") + + +def _default_backup_dir() -> Path: + """Fallback backup location: CONFIG_DIR/backups.""" + config_dir = os.environ.get("CONFIG_DIR") or str( + Path(__file__).parents[2] / "config" + ) + return Path(config_dir) / "backups" + + +def get_backup_dir(logger: Any = None) -> Path: + """Resolve general.backup_dir (default CONFIG_DIR/backups); propagates + ConfigError — callers must deny rather than guess a location.""" + default = _default_backup_dir() + + config = load_config() + configured = (getattr(config.general, "backup_dir", "") or "").strip() + + if configured: + if not is_path_allowed(configured, config): + if logger: + logger.error( + f"backup_dir '{configured}' is outside the allowed roots; " + f"backing up to {default} instead" + ) + else: + try: + target = Path(configured).expanduser().resolve() + target.mkdir(parents=True, exist_ok=True) + # Re-confine the RESOLVED target: is_path_allowed() authorised a + # path that a swapped symlink could since have re-pointed. + if is_path_allowed(str(target), config): + return target + if logger: + logger.error( + f"backup_dir '{configured}' resolved outside the allowed " + f"roots ({target}); backing up to {default} instead" + ) + except OSError as exc: + if logger: + logger.error( + f"backup_dir '{configured}' is not usable ({exc}); " + f"backing up to {default} instead" + ) + + default.mkdir(parents=True, exist_ok=True) + return default + + +def build_backup_bytes() -> bytes: + """Backup zip (config.yml + chub.db.sql dump) bytes; SQLite backup API snapshots + the db safely while in use. Shared by the download endpoint + maintenance thread.""" + config_path = get_config_path() + db_path = _get_db_path() + + buf = io.BytesIO() + with zipfile.ZipFile(buf, "w", zipfile.ZIP_DEFLATED) as zf: + if os.path.exists(config_path): + zf.write(config_path, "config.yml") + + if os.path.exists(db_path): + db_buf = io.BytesIO() + src = sqlite3.connect(db_path) + try: + mem = sqlite3.connect(":memory:") + src.backup(mem) + for line in mem.iterdump(): + db_buf.write(f"{line}\n".encode("utf-8")) + mem.close() + finally: + src.close() + db_buf.seek(0) + zf.writestr("chub.db.sql", db_buf.read()) + + return buf.getvalue() + + +def save_backup(logger: Any = None) -> Path: + """Write a timestamped backup into the backups directory; return its path.""" + # Resolve the destination first — a config error must not cost a full dump. + directory = get_backup_dir(logger) + data = build_backup_bytes() + stem = f"chub-backup-{datetime.now().strftime('%Y%m%d-%H%M%S')}" + + for attempt in range(_SAVE_COLLISION_RETRIES): + suffix = "" if attempt == 0 else f"-{attempt}" + backup_path = directory / f"{stem}{suffix}.zip" + try: + # "xb", never write_bytes: the name is second-precision, so a + # concurrent API + maintenance backup would truncate each other. + with open(backup_path, "xb") as fh: + fh.write(data) + except FileExistsError: + continue + break + else: + raise OSError(f"No free backup filename for {stem} in {directory}") + + if logger: + logger.info(f"Backup created: {backup_path.name}") + return backup_path diff --git a/backend/util/config.py b/backend/util/config.py index d681bc3d..c0730eef 100755 --- a/backend/util/config.py +++ b/backend/util/config.py @@ -3,6 +3,7 @@ import pathlib import sys import tempfile +import threading from typing import Any, Dict, List, Literal, Optional, Union import yaml @@ -1112,6 +1113,49 @@ def get_config_path() -> str: return config_file_path +# Parsed configs keyed by path -> (file version, config). AuthMiddleware calls +# load_config() per request; re-reading + revalidating 35 models costs ~3ms. +_config_cache: Dict[str, tuple] = {} +_config_cache_lock = threading.Lock() +_CONFIG_CACHE_MAX = 8 + + +def _config_file_version(path: str) -> Optional[tuple]: + """Stat signature identifying a config file's contents; None if unreadable.""" + try: + st = os.stat(path) + except OSError: + return None + return (st.st_mtime_ns, st.st_size, st.st_ino) + + +def _cached_config(path: str, version: tuple) -> Optional[ChubConfig]: + """Return a private copy of the cached config for this file version, if any.""" + with _config_cache_lock: + entry = _config_cache.get(path) + if entry is None or entry[0] != version: + return None + # A copy, not the cached object: callers (auth/setup/notifications) mutate + # what load_config returns before saving, and must not poison the cache. + return entry[1].model_copy(deep=True) + + +def _cache_config(path: str, version: tuple, config: ChubConfig) -> None: + """Cache `config` under the file version it was parsed from.""" + if _config_file_version(path) != version: + return # rewritten while we read it (e.g. legacy migration) — don't cache + with _config_cache_lock: + if len(_config_cache) >= _CONFIG_CACHE_MAX: + _config_cache.clear() + _config_cache[path] = (version, config.model_copy(deep=True)) + + +def clear_config_cache() -> None: + """Drop every cached config (call after config.yml changes out of band).""" + with _config_cache_lock: + _config_cache.clear() + + def format_validation_errors(validation_error: ValidationError) -> List[str]: """Return one humanized "loc: msg" line per Pydantic field error. @@ -1194,14 +1238,22 @@ def load_config(path: Optional[str] = None) -> ChubConfig: Raises ConfigError subclasses on failure so callers can handle errors appropriately (API returns HTTP errors, CLI prints and exits). + + Repeat loads are served from a cache keyed on the file's mtime/size, so an + edit (or save_config) is picked up but an unchanged file isn't re-parsed. """ from backend.util.config_migrator import is_legacy_config config_path = path or get_config_path() - if not os.path.exists(config_path): + version = _config_file_version(config_path) + if version is None: return ChubConfig() + cached = _cached_config(config_path, version) + if cached is not None: + return cached + try: with open(config_path, "r") as f: raw = yaml.safe_load(f) @@ -1219,7 +1271,7 @@ def load_config(path: Optional[str] = None) -> ChubConfig: _backfill_setup_completed(raw) try: - return ChubConfig.model_validate(raw) + config = ChubConfig.model_validate(raw) except ValidationError as e: raise ConfigValidationError( f"Configuration validation failed in {config_path}", @@ -1228,6 +1280,9 @@ def load_config(path: Optional[str] = None) -> ChubConfig: except Exception as e: raise ConfigError(f"Unexpected configuration error: {e}") from e + _cache_config(config_path, version, config) + return config + def _auto_migrate_and_persist(raw: dict, config_path: str) -> dict: """Back up the original, migrate, write the migrated YAML back, log notes. @@ -1338,8 +1393,14 @@ def module_is_disabled(module_name: str, config: Optional[ChubConfig] = None) -> getattr(getattr(cfg, "general", None), "disabled_modules", None) or [] ) return module_name in disabled - except Exception: - return False + except Exception as exc: + # Fail CLOSED: these modules mutate the filesystem, so a config we + # can't read must skip the run, not green-light it. + _config_log.warning( + f"Could not determine disabled state for '{module_name}' " + f"({exc}); treating it as disabled" + ) + return True def save_config(config: ChubConfig, path: Optional[str] = None) -> None: @@ -1353,6 +1414,9 @@ def save_config(config: ChubConfig, path: Optional[str] = None) -> None: with os.fdopen(fd, "w") as f: yaml.safe_dump(config.model_dump(mode="python"), f, sort_keys=False) os.replace(tmp_path, config_path) + # The stat key catches this too; clearing makes it immediate even on + # a filesystem with coarse mtime granularity. + clear_config_cache() except BaseException: # Clean up temp file on any failure try: diff --git a/backend/util/database/__init__.py b/backend/util/database/__init__.py index da2d3c99..4e5c82b1 100644 --- a/backend/util/database/__init__.py +++ b/backend/util/database/__init__.py @@ -9,7 +9,7 @@ from .border_state import BorderState from .collection_cache import CollectionCache -from .db_base import DatabaseBase +from .db_base import DatabaseBase, escape_like from .holiday import HolidayStatus from .media_asset_matches import MediaAssetMatches from .media_cache import MediaCache @@ -414,5 +414,6 @@ def my_db_operation(db): "MediaCache", "MediaAssetMatches", "WebhookCache", + "escape_like", "with_database", ] diff --git a/backend/util/database/db_base.py b/backend/util/database/db_base.py index 58270793..3bf3062a 100755 --- a/backend/util/database/db_base.py +++ b/backend/util/database/db_base.py @@ -9,6 +9,27 @@ from backend.util.logger import Logger +# Files already schema-synced (by _schema_file_state) — every construction calls +# init_schema, else 24-table re-syncs N×. RLock: SchemaManager imports under it. +_SCHEMA_SYNCED: set = set() +_SCHEMA_SYNCED_LOCK = threading.RLock() + + +def _schema_file_state(db_path: str) -> Tuple[Optional[Tuple], int]: + """(identity, size) of the db FILE; (None, 0) when it doesn't exist yet.""" + try: + st = os.stat(db_path) + except OSError: + return None, 0 + return (os.path.abspath(db_path), st.st_dev, st.st_ino), st.st_size + + +def escape_like(value: str) -> str: + """Escape LIKE metacharacters so `value` matches literally.""" + # The SQL MUST carry ESCAPE '\\' too, or this silently does nothing. + return value.replace("\\", "\\\\").replace("%", "\\%").replace("_", "\\_") + + class DatabaseBase: """Base class for all database operations with proper resource management.""" @@ -163,12 +184,27 @@ def execute_transaction( conn.commit() @staticmethod - def init_schema(db_path: str) -> None: - """Initialize database schema - called by ChubDB.""" + def init_schema(db_path: str, force: bool = False) -> None: + """Initialize database schema — skips files already synced this process.""" from .schema import SchemaManager - conn = sqlite3.connect(db_path) - try: - SchemaManager.init_database(conn) - finally: - conn.close() + # Held across the DDL, not just the lookup: two threads that both miss + # the set would otherwise run SchemaManager concurrently and collide. + with _SCHEMA_SYNCED_LOCK: + # size: a deleted+recreated db can REUSE the inode, so identity + # alone would skip a 0-byte file that has no schema yet. + identity, size = _schema_file_state(db_path) + if not force and size > 0 and identity in _SCHEMA_SYNCED: + return + + conn = sqlite3.connect(db_path, timeout=30) + try: + conn.execute("PRAGMA busy_timeout=30000") + SchemaManager.init_database(conn) + finally: + conn.close() + + # Re-stat: the file may not have existed before connect() created it. + identity, _ = _schema_file_state(db_path) + if identity is not None: + _SCHEMA_SYNCED.add(identity) diff --git a/backend/util/database/media_cache.py b/backend/util/database/media_cache.py index fc16ba88..f94b0656 100755 --- a/backend/util/database/media_cache.py +++ b/backend/util/database/media_cache.py @@ -4,7 +4,7 @@ from backend.util.helper import parse_search_id from backend.util.normalization import normalize_titles -from .db_base import DatabaseBase +from .db_base import DatabaseBase, escape_like # Library-health SQL fragments, shared across the stats queries so by_type, # totals and by_instance can't drift. Everything is counted in CONTENT UNITS — @@ -877,17 +877,9 @@ def search( # whose normalized form has the punctuation stripped. Raw # title LIKE is the case-insensitive fallback for exact # stored substrings. Same fix as poster_cache.search. - # Escape LIKE metacharacters so a query with %/_ matches literally - # (mirrors poster_cache.delete_by_path_prefix). - norm_esc = ( - normalize_titles(query) - .replace("\\", "\\\\") - .replace("%", "\\%") - .replace("_", "\\_") - ) - raw_esc = ( - query.replace("\\", "\\\\").replace("%", "\\%").replace("_", "\\_") - ) + # Escape LIKE metacharacters so a query with %/_ matches literally. + norm_esc = escape_like(normalize_titles(query)) + raw_esc = escape_like(query) sub = [ "normalized_title LIKE ? ESCAPE '\\'", "title LIKE ? ESCAPE '\\'", @@ -915,12 +907,7 @@ def search( if genres: genre_conditions = ["genre LIKE ? ESCAPE '\\'" for _ in genres] conditions.append(f"({' OR '.join(genre_conditions)})") - params.extend( - "%" - + g.replace("\\", "\\\\").replace("%", "\\%").replace("_", "\\_") - + "%" - for g in genres - ) + params.extend(f"%{escape_like(g)}%" for g in genres) if year_min is not None: conditions.append("CAST(year AS INTEGER) >= ?") diff --git a/backend/util/database/poster_cache.py b/backend/util/database/poster_cache.py index 6300db68..529222cd 100755 --- a/backend/util/database/poster_cache.py +++ b/backend/util/database/poster_cache.py @@ -4,7 +4,7 @@ from backend.util.helper import parse_search_id from backend.util.normalization import normalize_titles -from .db_base import DatabaseBase +from .db_base import DatabaseBase, escape_like # Additional-artwork image_type values (everything that isn't a poster and @@ -79,13 +79,10 @@ def _title_search_clause(query: str) -> tuple: poster_cache.search / media_cache.search). """ - def esc(s: str) -> str: - return s.replace("\\", "\\\\").replace("%", "\\%").replace("_", "\\_") - if "*" in query: parts = query.split("*") - norm_pat = "%".join(esc(normalize_titles(p)) for p in parts) - raw_pat = "%".join(esc(p) for p in parts) + norm_pat = "%".join(escape_like(normalize_titles(p)) for p in parts) + raw_pat = "%".join(escape_like(p) for p in parts) sub = [ "normalized_title LIKE ? ESCAPE '\\'", "title LIKE ? ESCAPE '\\'", @@ -97,8 +94,8 @@ def esc(s: str) -> str: "title LIKE ? ESCAPE '\\'", ] sub_params = [ - f"%{esc(normalize_titles(query))}%", - f"%{esc(query)}%", + f"%{escape_like(normalize_titles(query))}%", + f"%{escape_like(query)}%", ] tmdb, tvdb, imdb = parse_search_id(query) @@ -261,8 +258,8 @@ def has_rows_under_prefix(self, path_prefix: str) -> bool: """ prefix = path_prefix.rstrip("/") + "/" row = self.execute_query( - "SELECT 1 FROM poster_cache WHERE file LIKE ? LIMIT 1", - (prefix + "%",), + "SELECT 1 FROM poster_cache WHERE file LIKE ? ESCAPE '\\' LIMIT 1", + (escape_like(prefix) + "%",), fetch_one=True, ) return row is not None @@ -414,7 +411,7 @@ def delete_by_path_prefix(self, path_prefix: str) -> int: prefix = path_prefix.rstrip("/") + "/" # Escape LIKE metacharacters so a folder named e.g. `My_Movies` can't # match siblings via the `_`/`%` wildcards. - like = prefix.replace("\\", "\\\\").replace("%", "\\%").replace("_", "\\_") + like = escape_like(prefix) return self.execute_query( "DELETE FROM poster_cache WHERE file LIKE ? ESCAPE '\\'", (like + "%",) ) @@ -427,7 +424,7 @@ def delete_asset_rows_by_path_prefix(self, path_prefix: str) -> int: shared poster_cache. """ prefix = path_prefix.rstrip("/") + "/" - like = prefix.replace("\\", "\\\\").replace("%", "\\%").replace("_", "\\_") + like = escape_like(prefix) return self.execute_query( "DELETE FROM poster_cache WHERE file LIKE ? ESCAPE '\\' " "AND image_type != 'poster'", @@ -467,8 +464,14 @@ def search( # column so hyphenated / special-char searches ("x-men") still # find rows where the stored value collapsed to "xmen". Fall back # to a raw `title LIKE` so exact stored substrings still hit too. - sub = ["normalized_title LIKE ?", "title LIKE ?"] - sub_params: list = [f"%{normalize_titles(query)}%", f"%{query}%"] + sub = [ + "normalized_title LIKE ? ESCAPE '\\'", + "title LIKE ? ESCAPE '\\'", + ] + sub_params: list = [ + f"%{escape_like(normalize_titles(query))}%", + f"%{escape_like(query)}%", + ] # Also match an id pasted from a filename tag ({tmdb-…}/{tvdb-…}/ # {imdb-tt…}) or a bare IMDb id, so users can search by id. tmdb, tvdb, imdb = parse_search_id(query) diff --git a/backend/util/helper.py b/backend/util/helper.py index 02fe75e0..ae13b3b3 100755 --- a/backend/util/helper.py +++ b/backend/util/helper.py @@ -345,6 +345,14 @@ def get_config_dir() -> str: return str(config_dir) +def get_static_dir() -> Path: + """Get the frontend static directory (STATIC_DIR env, else templates/).""" + static_dir = os.environ.get("STATIC_DIR") + if static_dir: + return Path(static_dir) + return Path(__file__).resolve().parents[2] / "templates" + + def extract_year(text: str) -> Optional[int]: """Extract first 4-digit year from text, returns None if not found.""" try: diff --git a/backend/util/maintenance.py b/backend/util/maintenance.py index d1a50f79..ee68cb7a 100644 --- a/backend/util/maintenance.py +++ b/backend/util/maintenance.py @@ -9,12 +9,17 @@ running. All work is wrapped so a transient failure never kills the thread. """ +import fnmatch import os import stat import threading import time from pathlib import Path +from backend.util.backup import get_backup_dir, save_backup +from backend.util.config import load_config +from backend.util.path_safety import is_path_allowed + def _log_base() -> Path: """Resolve the logs base directory the same way the logger does.""" @@ -52,42 +57,69 @@ def prune_old_logs(retention_days: int, logger=None) -> int: return removed -def prune_old_backups(backup_dir: Path, keep: int, logger=None) -> int: - """Keep only the newest `keep` chub-backup archives. Returns the count removed.""" +def prune_old_backups(backup_dir: Path, keep: int, config, logger=None) -> int: + """Keep only the newest `keep` chub-backup archives; returns the count removed. + `config` is REQUIRED — it re-confines the RESOLVED root right before deleting.""" if keep <= 0: return 0 + try: - backups = sorted( - backup_dir.glob("chub-backup-*.zip"), - key=lambda f: f.stat().st_mtime, - reverse=True, - ) + root = backup_dir.resolve(strict=True) except OSError: return 0 + # Authorise BEFORE opening anything: a component of backup_dir can be + # re-pointed after get_backup_dir() cleared it, so resolve() may land out. + if not is_path_allowed(str(root), config): + if logger: + logger.error(f"Refusing to prune '{root}': outside the allowed roots") + return 0 + try: - root = backup_dir.resolve(strict=True) dir_fd = os.open(str(root), os.O_RDONLY | getattr(os, "O_DIRECTORY", 0)) except OSError: return 0 removed = 0 try: - for old in backups[keep:]: + # The path can be swapped between the check and the open, so pin the + # descriptor to the directory that was actually authorised. + opened, authorised = os.fstat(dir_fd), os.stat(root) + if (opened.st_dev, opened.st_ino) != (authorised.st_dev, authorised.st_ino): + if logger: + logger.error(f"Refusing to prune '{root}': it changed under us") + return 0 + + entries = [] + for name in os.listdir(dir_fd): + if not fnmatch.fnmatch(name, "chub-backup-*.zip"): + continue try: - # NEVER resolve-then-unlink: that deletes a symlink's TARGET. - # lstat + S_ISREG rejects links and dirs; unlinking by name - # against dir_fd stops a swapped parent redirecting the delete. - st = os.lstat(os.path.join(str(root), old.name)) - if not stat.S_ISREG(st.st_mode): - if logger: - logger.warning(f"Skipping non-regular backup entry: {old}") - continue - os.unlink(old.name, dir_fd=dir_fd) + # ONE nofollow stat through dir_fd: following a link to read + # its mtime would stat a target the link owner chose. + st = os.lstat(name, dir_fd=dir_fd) + except OSError as e: + if logger: + logger.debug(f"Could not read backup {root / name}: {e}") + continue + entries.append((st.st_mtime, name, stat.S_ISREG(st.st_mode))) + entries.sort(key=lambda entry: entry[0], reverse=True) + + for _mtime, name, is_regular in entries[keep:]: + # NEVER resolve-then-unlink: that deletes a symlink's TARGET. + # S_ISREG rejects links/dirs; dir_fd anchors the unlink. + if not is_regular: + if logger: + logger.warning(f"Skipping non-regular backup entry: {root / name}") + continue + try: + os.unlink(name, dir_fd=dir_fd) removed += 1 except OSError as e: if logger: - logger.debug(f"Could not prune backup {old}: {e}") + logger.debug(f"Could not prune backup {root / name}: {e}") + except OSError: + return removed finally: os.close(dir_fd) if removed and logger: @@ -97,12 +129,9 @@ def prune_old_backups(backup_dir: Path, keep: int, logger=None) -> int: def run_auto_backup(keep: int, logger=None) -> None: """Write one backup and trim the directory to `keep` archives.""" - # Imported lazily — backend.api.system pulls in FastAPI/router state that we - # don't want to import at module load (this util is imported early). - from backend.api.system import _get_backup_dir, save_backup - save_backup(logger) - prune_old_backups(_get_backup_dir(logger), keep, logger) + root = get_backup_dir(logger) + prune_old_backups(root, keep, load_config(), logger) def _run_once(config, logger) -> None: @@ -123,24 +152,22 @@ def _run_once(config, logger) -> None: logger.error(f"Log pruning failed: {e}") -def start_maintenance(config, logger, interval: int = 86400) -> threading.Thread: - """Start the daily maintenance daemon thread and return it. - - Config is loaded fresh each pass (the reference on config-reload is replaced, - not mutated) so settings changes take effect without a restart. The first - pass runs after `interval` so a just-restarted container doesn't back up on - every boot. - """ +def start_maintenance(logger, interval: int = 86400) -> threading.Thread: + """Start the daily maintenance daemon thread and return it. Config is loaded + fresh each pass; the first pass waits `interval` (no backup on every boot).""" def loop(): + """Wake once per `interval` and run a pass against freshly loaded config.""" while True: time.sleep(interval) try: - from backend.util.config import load_config - current = load_config() - except Exception: - current = config + except Exception as e: + # Never fall back to the startup snapshot: these passes delete + # files, and stale retention/paths would delete the wrong ones. + if logger: + logger.warning(f"Maintenance pass skipped, config unreadable: {e}") + continue _run_once(current, logger) thread = threading.Thread(target=loop, name="maintenance", daemon=True) diff --git a/backend/util/module_orchestrator.py b/backend/util/module_orchestrator.py index 631d63f5..52c7fb85 100755 --- a/backend/util/module_orchestrator.py +++ b/backend/util/module_orchestrator.py @@ -1,6 +1,6 @@ # util/module_orchestrator.py import time -from typing import Any, Dict +from typing import Any, Dict, Optional from backend.modules import MODULES from backend.util.database import ChubDB @@ -226,14 +226,22 @@ def run_module_cli(self, module_names: list) -> None: self._log("error", f"Error in run_modules_cli: {e}", "cli", exc_info=True) raise - def get_module_status(self, module_name: str) -> Dict[str, Any]: + def get_module_status( + self, module_name: str, db: Optional[ChubDB] = None + ) -> Dict[str, Any]: """ Get current status of a module by checking active jobs. + + Pass `db` to reuse an open context — callers that poll every module on a + tick would otherwise open one ChubDB per module per tick. """ try: - # Always use a new context to avoid race conditions with FastAPI lifespan - with ChubDB(self.logger, quiet=True) as db: + if db is not None: running_job = db.worker.get_running_module_job(module_name) + else: + # Otherwise a new context, to avoid racing the FastAPI lifespan. + with ChubDB(self.logger, quiet=True) as own_db: + running_job = own_db.worker.get_running_module_job(module_name) if running_job: return { diff --git a/main.py b/main.py index 3ac04a17..19d204d0 100755 --- a/main.py +++ b/main.py @@ -17,6 +17,7 @@ ChubConfig, ConfigError, ConfigValidationError, + clear_config_cache, format_validation_errors, get_config_path, load_config, @@ -370,6 +371,9 @@ def _on_config_changed(self) -> None: """ log = self.logger.get_adapter("MAIN") if self.logger else None try: + # The watcher fires on external edits, which the cache's stat key + # may not distinguish on a coarse-mtime filesystem. + clear_config_cache() new_config = load_config() self.config = new_config if self.scheduler: diff --git a/tests/test_backup_dir.py b/tests/test_backup_dir.py index 48b409c5..ca19cf73 100644 --- a/tests/test_backup_dir.py +++ b/tests/test_backup_dir.py @@ -1,35 +1,56 @@ -"""Tests for backend.api.system._get_backup_dir — the configurable backup -location. A backup landing in the wrong place beats one silently never written, -so every failure path must fall back to CONFIG_DIR/backups, loudly.""" +"""Tests for backend.util.backup.get_backup_dir — the configurable backup +location. A rejected or unusable backup_dir falls back to CONFIG_DIR/backups, +loudly; a config that won't LOAD denies instead — nothing validated the path.""" -import backend.api.system as system_mod +from datetime import datetime -_default_backup_dir = system_mod._default_backup_dir -_get_backup_dir = system_mod._get_backup_dir +import pytest + +import backend.util.backup as backup_mod +from backend.util.config import ConfigParseError + +_default_backup_dir = backup_mod._default_backup_dir +_get_backup_dir = backup_mod.get_backup_dir class _Log: + """Collects error lines so a fallback can be asserted as loud, not silent.""" + def __init__(self): self.errors = [] def info(self, *a, **k): - pass + """No-op.""" def error(self, msg, *a, **k): + """Record an error line.""" self.errors.append(str(msg)) class _Cfg: + """Minimal stand-in exposing only general.backup_dir.""" + def __init__(self, backup_dir=""): self.general = type("G", (), {"backup_dir": backup_dir})() +class _FrozenClock: + """datetime stand-in pinning now() so two saves share one timestamp.""" + + @staticmethod + def now(): + """Fixed instant.""" + return datetime(2026, 8, 11, 12, 0, 0) + + def _stub_config(monkeypatch, backup_dir="", allowed=True): - monkeypatch.setattr(system_mod, "load_config", lambda: _Cfg(backup_dir)) - monkeypatch.setattr(system_mod, "is_path_allowed", lambda p, c: allowed) + """Point backup_mod at a fake config with a fixed is_path_allowed verdict.""" + monkeypatch.setattr(backup_mod, "load_config", lambda: _Cfg(backup_dir)) + monkeypatch.setattr(backup_mod, "is_path_allowed", lambda p, c: allowed) def test_defaults_to_config_dir_when_unset(monkeypatch, tmp_path): + """No backup_dir configured: land in CONFIG_DIR/backups and create it.""" monkeypatch.setenv("CONFIG_DIR", str(tmp_path)) _stub_config(monkeypatch, backup_dir="") @@ -38,6 +59,7 @@ def test_defaults_to_config_dir_when_unset(monkeypatch, tmp_path): def test_uses_configured_dir_when_allowed(monkeypatch, tmp_path): + """An allowed backup_dir is used verbatim and created.""" monkeypatch.setenv("CONFIG_DIR", str(tmp_path / "cfg")) target = tmp_path / "elsewhere" / "chub" _stub_config(monkeypatch, backup_dir=str(target), allowed=True) @@ -47,6 +69,7 @@ def test_uses_configured_dir_when_allowed(monkeypatch, tmp_path): def test_rejected_path_falls_back_and_logs(monkeypatch, tmp_path): + """A backup_dir outside the allowed roots falls back and never gets made.""" monkeypatch.setenv("CONFIG_DIR", str(tmp_path)) outside = tmp_path / "not-allowed" _stub_config(monkeypatch, backup_dir=str(outside), allowed=False) @@ -58,6 +81,7 @@ def test_rejected_path_falls_back_and_logs(monkeypatch, tmp_path): def test_unusable_path_falls_back_and_logs(monkeypatch, tmp_path): + """An allowed-but-unmakeable backup_dir falls back loudly.""" monkeypatch.setenv("CONFIG_DIR", str(tmp_path)) # A file where the directory should be — mkdir raises OSError. blocker = tmp_path / "blocker" @@ -69,14 +93,32 @@ def test_unusable_path_falls_back_and_logs(monkeypatch, tmp_path): assert any("not usable" in e for e in logger.errors) -def test_unreadable_config_falls_back_and_logs(monkeypatch, tmp_path): +def test_unreadable_config_denies(monkeypatch, tmp_path): + """Fail closed: an unloadable config validated no path, so write nowhere.""" monkeypatch.setenv("CONFIG_DIR", str(tmp_path)) def boom(): - raise RuntimeError("config.yml is unparseable") + """Stand-in load_config that fails.""" + raise ConfigParseError("config.yml is unparseable") - monkeypatch.setattr(system_mod, "load_config", boom) - logger = _Log() + monkeypatch.setattr(backup_mod, "load_config", boom) - assert _get_backup_dir(logger) == _default_backup_dir() - assert any("Could not read backup_dir" in e for e in logger.errors) + with pytest.raises(ConfigParseError): + _get_backup_dir(_Log()) + assert not (tmp_path / "backups").exists() + + +def test_save_backup_never_overwrites_a_same_second_archive(monkeypatch, tmp_path): + """Two backups in the same second must both survive, not truncate.""" + monkeypatch.setenv("CONFIG_DIR", str(tmp_path)) + _stub_config(monkeypatch, backup_dir="") + monkeypatch.setattr(backup_mod, "build_backup_bytes", lambda: b"payload") + # Frozen clock: the collision is the point, not a race the test might lose. + monkeypatch.setattr(backup_mod, "datetime", _FrozenClock) + + first = backup_mod.save_backup() + second = backup_mod.save_backup() + + assert first != second + assert first.read_bytes() == second.read_bytes() == b"payload" + assert len(list(_default_backup_dir().glob("chub-backup-*.zip"))) == 2 diff --git a/tests/test_config.py b/tests/test_config.py index 02edf6b3..12e7a5cc 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -165,6 +165,45 @@ def test_save_config_creates_config_directory(tmp_path): assert config_path.exists() +def test_load_config_caches_until_the_file_changes(tmp_path, monkeypatch): + """Repeat loads come from cache; a rewrite is picked up on the next load.""" + config_path = str(tmp_path / "config.yml") + config = ChubConfig() + config.auth.username = "first" + save_config(config, config_path) + + parses = [] + # Same module object backend.util.config calls yaml.safe_load on. + real_safe_load = yaml.safe_load + monkeypatch.setattr( + yaml, + "safe_load", + lambda stream: (parses.append(1), real_safe_load(stream))[1], + ) + + assert load_config(config_path).auth.username == "first" + assert load_config(config_path).auth.username == "first" + assert len(parses) == 1 + + config.auth.username = "second" + save_config(config, config_path) + assert load_config(config_path).auth.username == "second" + + +def test_load_config_does_not_share_mutable_state(tmp_path): + """Callers mutate what load_config returns before saving — never the cache.""" + config_path = str(tmp_path / "config.yml") + save_config(ChubConfig(), config_path) + + first = load_config(config_path) + first.auth.username = "never-saved" + first.general.disabled_modules.append("nohl") + + second = load_config(config_path) + assert second.auth.username == "" + assert second.general.disabled_modules == [] + + def test_legacy_notifications_auto_heal_on_load(tmp_path): """An existing per-module notifications config must auto-migrate to the per-destination shape on load AND be rewritten to disk, with the original diff --git a/tests/test_database.py b/tests/test_database.py index 37366d6f..fd54cd01 100644 --- a/tests/test_database.py +++ b/tests/test_database.py @@ -104,6 +104,59 @@ def test_wal_mode_enabled(self): os.unlink(db_path) +def _stub_sync(syncs): + """init_database stand-in that records the call and leaves the file non-empty.""" + + def _init(cls, conn, **_kw): + """Record one sync and write a marker table.""" + syncs.append(1) + conn.execute("CREATE TABLE IF NOT EXISTS marker (x INTEGER)") + conn.commit() + + return classmethod(_init) + + +def test_init_schema_syncs_each_db_file_once(monkeypatch, tmp_path): + """Every interface construction calls init_schema; only the first may sync.""" + from backend.util.database import db_base + + syncs = [] + monkeypatch.setattr(SchemaManager, "init_database", _stub_sync(syncs)) + + db_path = str(tmp_path / "chub.db") + db_base.DatabaseBase.init_schema(db_path) + db_base.DatabaseBase.init_schema(db_path) + assert len(syncs) == 1 + + # A different file still syncs, and force re-runs an already-synced one. + db_base.DatabaseBase.init_schema(str(tmp_path / "other.db")) + db_base.DatabaseBase.init_schema(db_path, force=True) + assert len(syncs) == 3 + + +def test_init_schema_resyncs_a_recreated_empty_db(monkeypatch, tmp_path): + """Inode reuse can hand a brand-new empty db an already-synced identity.""" + from backend.util.database import db_base + + syncs = [] + monkeypatch.setattr(SchemaManager, "init_database", _stub_sync(syncs)) + + db_path = str(tmp_path / "chub.db") + db_base.DatabaseBase.init_schema(db_path) + assert len(syncs) == 1 + + # Stand in for delete+recreate landing on the SAME inode: a 0-byte file + # whose identity the registry already holds. + os.unlink(db_path) + open(db_path, "wb").close() + identity, size = db_base._schema_file_state(db_path) + db_base._SCHEMA_SYNCED.add(identity) + assert size == 0 + + db_base.DatabaseBase.init_schema(db_path) + assert len(syncs) == 2, "an empty db at a reused identity must re-sync" + + def test_collection_update_persists_file_hash_and_mtime(): """collection_cache.update must accept file_hash/file_mtime (previously it didn't, so collection poster hashes were never stored → re-upload every run).""" diff --git a/tests/test_maintenance.py b/tests/test_maintenance.py index eb850462..fce09e8e 100644 --- a/tests/test_maintenance.py +++ b/tests/test_maintenance.py @@ -3,9 +3,17 @@ import time from unittest.mock import MagicMock +from backend.util.config import ChubConfig from backend.util.maintenance import prune_old_backups, prune_old_logs +def _allowing(root): + """A config whose allowed roots cover `root`, so prune may delete under it.""" + config = ChubConfig() + config.poster_renamerr.source_dirs = [str(root)] + return config + + def _touch(path, age_days=0): path.write_text("x") if age_days: @@ -40,11 +48,14 @@ def test_prune_old_logs_removes_only_stale(tmp_path, monkeypatch): def test_prune_old_backups_keeps_newest(tmp_path): + """Only the newest `keep` archives survive a prune.""" for i in range(5): f = tmp_path / f"chub-backup-2026010{i}-000000.zip" _touch(f, age_days=5 - i) # i=0 oldest, i=4 newest - removed = prune_old_backups(tmp_path, keep=2, logger=MagicMock()) + removed = prune_old_backups( + tmp_path, keep=2, config=_allowing(tmp_path), logger=MagicMock() + ) assert removed == 3 remaining = sorted(p.name for p in tmp_path.glob("chub-backup-*.zip")) # Newest two (i=3, i=4) survive @@ -55,6 +66,12 @@ def test_prune_old_backups_keeps_newest(tmp_path): def test_prune_old_backups_keep_zero_is_noop(tmp_path): + """keep=0 disables pruning entirely.""" _touch(tmp_path / "chub-backup-20260101-000000.zip") - assert prune_old_backups(tmp_path, keep=0, logger=MagicMock()) == 0 + assert ( + prune_old_backups( + tmp_path, keep=0, config=_allowing(tmp_path), logger=MagicMock() + ) + == 0 + ) assert list(tmp_path.glob("chub-backup-*.zip")) diff --git a/tests/test_module_disable.py b/tests/test_module_disable.py index 23197c64..1f2083aa 100644 --- a/tests/test_module_disable.py +++ b/tests/test_module_disable.py @@ -41,6 +41,17 @@ def test_module_is_disabled_empty_default(): assert module_is_disabled("poster_renamerr", cfg) is False +def test_module_is_disabled_fails_closed_on_config_error(monkeypatch): + """An unreadable config must skip the run — these modules mutate the disk.""" + + def boom(*_a, **_kw): + """Stand-in load_config that fails.""" + raise RuntimeError("config.yml is unparseable") + + monkeypatch.setattr("backend.util.config.load_config", boom) + assert module_is_disabled("poster_renamerr") is True + + def test_orchestrator_async_skips_disabled(): orch = ModuleOrchestrator(logger=StubLogger()) with patch("backend.util.config.module_is_disabled", return_value=True): diff --git a/tests/test_prune_backups_safety.py b/tests/test_prune_backups_safety.py index f0a886c0..b645dc4c 100644 --- a/tests/test_prune_backups_safety.py +++ b/tests/test_prune_backups_safety.py @@ -1,29 +1,51 @@ """Deletion safety for prune_old_backups. backup_dir is user-configurable, so the prune loop deletes from a path the user -supplied. It must remove the directory entry itself and never follow a link.""" +supplied. It must remove the directory entry itself, never follow a link, and +re-confine the RESOLVED root against the config before touching anything — and +the maintenance thread must never drive it from a stale config.""" import os +import threading +from types import SimpleNamespace +from unittest.mock import patch -from backend.util.maintenance import prune_old_backups +import backend.util.maintenance as maintenance +from backend.util.config import ChubConfig, ConfigParseError # Fixed mtimes so "oldest" is explicit rather than an artefact of creation order. _BASE_MTIME = 1_700_000_000 class _Log: + """Captures warning/error lines so a skip can be asserted as loud.""" + def __init__(self): + """Start with empty capture lists.""" self.warnings = [] + self.errors = [] def info(self, *a, **k): - pass + """No-op.""" def debug(self, *a, **k): - pass + """No-op.""" def warning(self, msg, *a, **k): + """Record a warning line.""" self.warnings.append(str(msg)) + def error(self, msg, *a, **k): + """Record an error line.""" + self.errors.append(str(msg)) + + +def _allowing(root): + """A config whose allowed roots cover `root`, so prune may delete under it.""" + config = ChubConfig() + config.poster_renamerr.source_dirs = [str(root)] + return config + def _archive(directory, name, age=0, body="zip"): """Write an archive whose mtime is `age` days before the base timestamp.""" @@ -34,15 +56,19 @@ def _archive(directory, name, age=0, body="zip"): def _age(path, days): + """Set `path`'s mtime to `days` before the base timestamp.""" when = _BASE_MTIME - days * 86400 os.utime(path, (when, when), follow_symlinks=False) def test_prunes_oldest_and_keeps_newest(tmp_path): + """Only the newest `keep` archives survive.""" for i in range(5): _archive(tmp_path, f"chub-backup-2026080{i}-000000.zip", age=i) - removed = prune_old_backups(tmp_path, keep=2) + removed = maintenance.prune_old_backups( + tmp_path, keep=2, config=_allowing(tmp_path) + ) assert removed == 3 survivors = {p.name for p in tmp_path.glob("chub-backup-*.zip")} @@ -58,17 +84,19 @@ def test_a_symlinked_backup_never_unlinks_its_target(tmp_path): backups.mkdir() precious = tmp_path / "precious.zip" precious.write_text("must survive") - # The sort key is f.stat().st_mtime, which FOLLOWS the link — so ageing the - # target is what puts the link last and into the delete window. - _age(precious, 2) _archive(backups, "chub-backup-20260803-000000.zip", age=0) _archive(backups, "chub-backup-20260802-000000.zip", age=1) link = backups / "chub-backup-20260801-000000.zip" link.symlink_to(precious) + # The sort key is the LINK's OWN mtime — prune never follows it — so age the + # link, not the target, to put it last and into the delete window. + _age(link, 2) logger = _Log() - prune_old_backups(backups, keep=2, logger=logger) + maintenance.prune_old_backups( + backups, keep=2, config=_allowing(tmp_path), logger=logger + ) assert precious.exists(), "prune followed the symlink and deleted its target" assert link.is_symlink(), "the link itself should be left alone, not resolved" @@ -76,6 +104,7 @@ def test_a_symlinked_backup_never_unlinks_its_target(tmp_path): def test_a_directory_named_like_a_backup_is_skipped(tmp_path): + """A directory matching the archive glob is skipped, not rmtree'd.""" _archive(tmp_path, "chub-backup-20260803-000000.zip", age=0) _archive(tmp_path, "chub-backup-20260802-000000.zip", age=1) stray = tmp_path / "chub-backup-20260801-000000.zip" @@ -83,19 +112,157 @@ def test_a_directory_named_like_a_backup_is_skipped(tmp_path): _age(stray, 2) logger = _Log() - removed = prune_old_backups(tmp_path, keep=2, logger=logger) + removed = maintenance.prune_old_backups( + tmp_path, keep=2, config=_allowing(tmp_path), logger=logger + ) assert removed == 0 assert (tmp_path / "chub-backup-20260801-000000.zip").is_dir() def test_keep_zero_or_negative_is_a_noop(tmp_path): + """keep <= 0 disables pruning entirely.""" _archive(tmp_path, "chub-backup-20260803-000000.zip") - assert prune_old_backups(tmp_path, keep=0) == 0 - assert prune_old_backups(tmp_path, keep=-1) == 0 + config = _allowing(tmp_path) + assert maintenance.prune_old_backups(tmp_path, keep=0, config=config) == 0 + assert maintenance.prune_old_backups(tmp_path, keep=-1, config=config) == 0 assert len(list(tmp_path.glob("chub-backup-*.zip"))) == 1 def test_missing_directory_returns_zero(tmp_path): - assert prune_old_backups(tmp_path / "nope", keep=1) == 0 + """A backup_dir that doesn't exist prunes nothing rather than raising.""" + config = _allowing(tmp_path) + assert maintenance.prune_old_backups(tmp_path / "nope", keep=1, config=config) == 0 + + +def test_unconfined_root_deletes_nothing(tmp_path, monkeypatch): + """A root outside the allowed roots is refused before anything is opened.""" + for i in range(4): + _archive(tmp_path, f"chub-backup-2026080{i}-000000.zip", age=i) + logger = _Log() + + opened = [] + real_open = os.open + monkeypatch.setattr( + maintenance.os, + "open", + lambda path, *a, **k: (opened.append(path), real_open(path, *a, **k))[1], + ) + + # A default ChubConfig's allowed roots are CONFIG_DIR + mounts, never tmp_path. + removed = maintenance.prune_old_backups( + tmp_path, keep=1, config=ChubConfig(), logger=logger + ) + + assert removed == 0 + assert opened == [], "opened a descriptor on a root that was never authorised" + assert len(list(tmp_path.glob("chub-backup-*.zip"))) == 4 + assert any("outside the allowed roots" in e for e in logger.errors) + + +def test_root_swapped_after_the_check_deletes_nothing(tmp_path, monkeypatch): + """The opened descriptor must BE the directory is_path_allowed cleared.""" + for i in range(4): + _archive(tmp_path, f"chub-backup-2026080{i}-000000.zip", age=i) + logger = _Log() + + real_stat = os.stat + root = str(tmp_path.resolve()) + + def _swapped_stat(path, *a, **kw): + """Report a different inode for the root, as a mid-flight swap would.""" + st = real_stat(path, *a, **kw) + if str(path) != root: + return st + fields = list(st) + fields[1] += 1 # st_ino + return os.stat_result(fields) + + monkeypatch.setattr(maintenance.os, "stat", _swapped_stat) + removed = maintenance.prune_old_backups( + tmp_path, keep=1, config=_allowing(tmp_path), logger=logger + ) + + assert removed == 0 + assert len(list(tmp_path.glob("chub-backup-*.zip"))) == 4 + assert any("changed under us" in e for e in logger.errors) + + +def test_confined_root_still_prunes(tmp_path): + """The re-confinement must not break the normal configured-root case.""" + backups = tmp_path / "backups" + backups.mkdir() + for i in range(4): + _archive(backups, f"chub-backup-2026080{i}-000000.zip", age=i) + + assert ( + maintenance.prune_old_backups(backups, keep=1, config=_allowing(tmp_path)) == 3 + ) + assert len(list(backups.glob("chub-backup-*.zip"))) == 1 + + +def test_auto_backup_hands_prune_a_config(monkeypatch, tmp_path): + """The caller must hand prune the config that authorises the delete.""" + captured = {} + monkeypatch.setattr(maintenance, "save_backup", lambda logger=None: None) + monkeypatch.setattr(maintenance, "get_backup_dir", lambda logger=None: tmp_path) + monkeypatch.setattr(maintenance, "load_config", ChubConfig) + monkeypatch.setattr( + maintenance, + "prune_old_backups", + lambda d, k, config, lg=None: captured.update(config=config), + ) + + maintenance.run_auto_backup(3) + assert isinstance(captured.get("config"), ChubConfig) + + +def test_auto_backup_failure_skips_the_cycle_without_killing_the_thread(): + """A config the backup path can't read logs and skips, never propagates.""" + logger = _Log() + general = SimpleNamespace( + auto_backup=True, auto_backup_keep=2, log_retention_days=0 + ) + + with patch.object(maintenance, "save_backup", side_effect=ConfigParseError("bad")): + maintenance._run_once(SimpleNamespace(general=general), logger) + + assert any("Auto-backup failed" in e for e in logger.errors) + + +class _OneShotClock: + """time stand-in that lets exactly one loop iteration through.""" + + def __init__(self): + """Start with no sleeps recorded.""" + self.sleeps = 0 + self.done = threading.Event() + + def sleep(self, _seconds): + """Park the daemon thread on the second wake rather than raising out of it.""" + self.sleeps += 1 + if self.sleeps > 1: + self.done.set() + threading.Event().wait() + + +def test_pass_is_skipped_when_config_will_not_load(monkeypatch): + """A pass must never run against the startup snapshot — retention goes stale.""" + ran = [] + logger = _Log() + clock = _OneShotClock() + + def boom(): + """Stand-in load_config that fails.""" + raise ConfigParseError("config.yml is unparseable") + + monkeypatch.setattr(maintenance, "time", clock) + monkeypatch.setattr(maintenance, "load_config", boom) + monkeypatch.setattr(maintenance, "_run_once", lambda cfg, lg: ran.append(cfg)) + + maintenance.start_maintenance(logger, interval=0) + assert clock.done.wait(timeout=5), "the maintenance loop never completed a pass" + + assert ran == [], "ran the destructive pass against the stale startup config" + assert any("config unreadable" in w for w in logger.warnings) diff --git a/tests/test_regression_review_2026.py b/tests/test_regression_review_2026.py index 2b838abd..4ba7c28e 100644 --- a/tests/test_regression_review_2026.py +++ b/tests/test_regression_review_2026.py @@ -10,6 +10,7 @@ from fastapi import FastAPI from fastapi.testclient import TestClient +from backend.util.config import ChubConfig, ConfigError, ConfigParseError from backend.util.database import ChubDB from backend.util.normalization import normalize_titles @@ -457,3 +458,140 @@ def test_redact_oauth_tokens_in_yaml_form(): assert "1//0gLongRefreshTokenValue1234567890abcd" not in out assert "access_token: [redacted]" in out assert "refresh_token: [redacted]" in out + + +# --- Round 3: backend correctness + perf audit --- + + +class _StubLog: + """Swallows every log call; get_adapter returns itself.""" + + def __getattr__(self, _): + """Any log method is a no-op.""" + return lambda *a, **k: None + + def get_adapter(self, *_a, **_kw): + """Adapters are the same sink.""" + return self + + +def _app(router, db=None): + """Mount `router` on a bare app carrying main.py's real ConfigError handler.""" + import backend.api.main as apimain + + app = FastAPI() + app.state.logger = _StubLog() + app.state.db = db + app.add_exception_handler(ConfigError, apimain.handle_config_error) + app.include_router(router) + # raise_server_exceptions=False so the handler answers instead of re-raising. + return TestClient(app, raise_server_exceptions=False) + + +# 18. /api/posters/list must resolve through STATIC_DIR, not a hardcoded +# templates/ path that doesn't exist in the Docker image (empty list). +def test_poster_list_uses_static_dir(monkeypatch, tmp_path): + """The list endpoint reads STATIC_DIR/posters, not templates/posters.""" + import backend.api.posters as posters + + posters_dir = tmp_path / "posters" + posters_dir.mkdir() + (posters_dir / "default-movie.jpg").write_bytes(b"x") + (posters_dir / "notes.txt").write_text("ignored") + monkeypatch.setenv("STATIC_DIR", str(tmp_path)) + + resp = _app(posters.router).get("/api/posters/list") + assert resp.status_code == 200 + assert resp.json()["data"]["files"] == ["default-movie.jpg"] + + +# 19a. has_rows_under_prefix must escape LIKE metacharacters, or a sibling +# folder makes the skip-on-zero-change guard wrongly believe rows exist. +def test_has_rows_under_prefix_escapes_like_wildcards(db): + """`_` in a folder name must not wildcard-match a sibling folder.""" + db.poster.upsert(_poster("/data/MyXMovies/p.jpg", "/data/MyXMovies")) + + assert db.poster.has_rows_under_prefix("/data/MyXMovies") is True + assert db.poster.has_rows_under_prefix("/data/My_Movies") is False + + +# 19b. poster_cache.search must escape too — the raw `title LIKE` branch keeps +# the metacharacters that normalize_titles strips. +def test_poster_search_escapes_like_wildcards(db): + """The raw `title LIKE` branch keeps metacharacters — escape them too.""" + for title, tmdb_id in (("My_Movies", 1), ("MyXMovies", 2)): + item = _poster(f"/data/{title}/p.jpg", f"/data/{title}") + item.update( + title=title, normalized_title=normalize_titles(title), tmdb_id=tmdb_id + ) + db.poster.upsert(item) + + titles = {row["title"] for row in db.poster.search("My_Movies")["items"]} + assert titles == {"My_Movies"} + + +# 19c. The tag→collection query must escape as well, or one tag silently +# sweeps in every similarly-named tag's media. +def test_collection_from_tag_escapes_like_wildcards(db): + """One tag must not sweep in every similarly-named tag's media.""" + import backend.api.media_api as media_api + + for title, tag in (("Dune", "4K_UHD"), ("Sicario", "4KxUHD")): + db.media.upsert( + { + "title": title, + "normalized_title": normalize_titles(title), + "year": 2021, + "tags": [tag], + }, + "movie", + "radarr", + "radarr", + ) + + resp = _app(media_api.router, db).post( + "/api/media/collections/from-tag", json={"tag": "4K_UHD"} + ) + assert resp.status_code == 200 + assert resp.json()["data"]["matched_media"] == 1 + + +# 20. The duplicate-exclusion filter must fail CLOSED: a config it can't read +# has to deny, since a bulk-delete button sits next to these results. +def test_duplicates_denies_when_config_unreadable(db, monkeypatch): + """An unreadable config yields CONFIG_INVALID, never unfiltered duplicates.""" + import backend.api.media_api as media_api + + client = _app(media_api.router, db) + + cfg = ChubConfig() + cfg.general.duplicate_exclude_groups = [["radarr", "radarr4k"]] + monkeypatch.setattr(media_api, "load_config", lambda: cfg) + assert client.get("/api/media/duplicates").status_code == 200 + + def boom(): + """Stand-in load_config that fails.""" + raise ConfigParseError("corrupt config") + + monkeypatch.setattr(media_api, "load_config", boom) + body = client.get("/api/media/duplicates") + assert body.status_code == 500 + assert body.json()["error_code"] == "CONFIG_INVALID" + + +# 20b. duplicate_exclude_groups is List[Any] — a non-string member must skip +# that group, not blow up set() and surface as DUPLICATES_ERROR. +def test_duplicates_skips_malformed_exclude_group_members(db, monkeypatch): + """A group holding an unhashable member is skipped, not a 500.""" + import backend.api.media_api as media_api + + cfg = ChubConfig() + cfg.general.duplicate_exclude_groups = [ + [{"instances": "nope"}, "radarr4k"], + ["radarr", "radarr4k"], + ] + monkeypatch.setattr(media_api, "load_config", lambda: cfg) + + resp = _app(media_api.router, db).get("/api/media/duplicates") + assert resp.status_code == 200 + assert resp.json()["success"] is True From 0a150200776051fe581d896b5ae647fc3bcd8028 Mon Sep 17 00:00:00 2001 From: chodeus Date: Wed, 12 Aug 2026 09:43:26 +0800 Subject: [PATCH 7/8] fix(frontend): dead Tailwind classes + used-vs-emitted CI guard, dev-route gating, dead code sweep (#507) * fix(frontend): dead Tailwind classes, dev-route gating, dead code sweep - Define or replace every class used in src/ that the Tailwind build didn't emit (bg-canvas, max-h-modal-body, rounded-t-xl, focus:ring-error, dropdown sizes; Separator -> bg-border-light, ButtonBase -> text-on-color/ bg-surface-inset, dead h-header/bg-*-hover dropped) - Add scripts/check-tailwind-classes.mjs + a CI step: fail when a class used in src/ is missing from the built CSS, so this class of bug can't return - Gate the 15 dev pages behind import.meta.env.DEV (routes AND chunks are dropped from the prod bundle) and remove the duplicate dev/toolbar path - Delete dead code: interactions.css, legacy navigation.css block, posterPreview.js (shadowed the real API), RenderField, forms/index, NotificationCard, useArrayField - vite.config: chunk react-router (the actual dependency), not react-router-dom * fix(frontend): check clsx object keys in className expressions A quoted key in a className region is a class (clsx conditionals), so the variant-map key exclusion now applies only to *Classes assignments. Regression tests run the checker against fixture trees. * fix(frontend): exclude .test. files from the class checker walk Fixture markup in test files (this checker's own regression tests included) is not UI and was flagged as used-but-unemitted. --- .github/workflows/codeql-lint.yml | 10 + frontend/package.json | 1 + frontend/scripts/check-tailwind-classes.mjs | 156 +++++++++ frontend/src/App.jsx | 305 +++++++----------- frontend/src/components/ToolBar/Separator.jsx | 2 +- frontend/src/components/ToolBar/ToolBar.jsx | 2 +- .../src/components/fields/RenderField.jsx | 54 ---- frontend/src/components/forms/index.js | 20 -- .../notifications/NotificationCard.jsx | 130 -------- .../ui/button/primitives/ButtonBase.jsx | 6 +- frontend/src/css/components/interactions.css | 240 -------------- frontend/src/css/components/navigation.css | 104 ------ frontend/src/css/index.css | 1 - frontend/src/css/tailwind.css | 25 ++ frontend/src/hooks/useArrayField.js | 114 ------- frontend/src/pages/dev/AccordionTestPage.jsx | 4 +- frontend/src/pages/dev/ApiTestPage.jsx | 2 +- frontend/src/pages/dev/FieldTestPage.jsx | 2 +- frontend/src/pages/dev/SettingsMockPage.jsx | 6 +- frontend/src/pages/dev/SpinnerTestPage.jsx | 8 +- .../src/pages/dev/ToolbarCompoundTest.jsx | 16 +- .../src/utils/checkTailwindClasses.test.js | 62 ++++ frontend/src/utils/posterPreview.js | 43 --- frontend/vite.config.js | 2 +- 24 files changed, 400 insertions(+), 915 deletions(-) create mode 100644 frontend/scripts/check-tailwind-classes.mjs delete mode 100755 frontend/src/components/fields/RenderField.jsx delete mode 100755 frontend/src/components/forms/index.js delete mode 100755 frontend/src/components/notifications/NotificationCard.jsx delete mode 100755 frontend/src/css/components/interactions.css delete mode 100755 frontend/src/hooks/useArrayField.js create mode 100644 frontend/src/utils/checkTailwindClasses.test.js delete mode 100755 frontend/src/utils/posterPreview.js diff --git a/.github/workflows/codeql-lint.yml b/.github/workflows/codeql-lint.yml index a17c87f9..e742593f 100644 --- a/.github/workflows/codeql-lint.yml +++ b/.github/workflows/codeql-lint.yml @@ -209,6 +209,16 @@ jobs: working-directory: frontend run: npm run test:run + # Tailwind silently drops classes it cannot resolve, so the built CSS is + # the only place a dead utility shows up — check:classes reads dist/. + - name: Build frontend + working-directory: frontend + run: npm run build + + - name: Check for used-but-unemitted Tailwind classes + working-directory: frontend + run: npm run check:classes + # ---- Branch isolation guard ---- # Develop-only extensions (e.g. the CL2K poster maker) must never reach main. # On main (and PRs targeting main) fail if any extension code is present: diff --git a/frontend/package.json b/frontend/package.json index 7f313a26..788ef9e6 100755 --- a/frontend/package.json +++ b/frontend/package.json @@ -11,6 +11,7 @@ "test:run": "vitest run", "lint": "eslint src --ext .js,.jsx && node scripts/check-field-types.js", "check:schema": "node scripts/check-field-types.js", + "check:classes": "node scripts/check-tailwind-classes.mjs", "format": "prettier --write \"src/**/*.{js,jsx,css,json,md}\"", "stylelint": "stylelint 'src/**/*.css' --fix" }, diff --git a/frontend/scripts/check-tailwind-classes.mjs b/frontend/scripts/check-tailwind-classes.mjs new file mode 100644 index 00000000..a3508b1a --- /dev/null +++ b/frontend/scripts/check-tailwind-classes.mjs @@ -0,0 +1,156 @@ +#!/usr/bin/env node +// Fails when a class used in src/ is missing from the built CSS — Tailwind +// silently drops unresolvable candidates. Run after `npm run build` (reads dist/). +import fs from 'node:fs'; +import path from 'node:path'; + +const SRC = 'src'; +const DIST = 'dist/assets'; + +/** Classes deliberately absent from the Tailwind build (each needs a reason). */ +const ALLOWLIST = new Set([ + // Google's Material Symbols webfont ships this one; base.css only sizes it. + 'material-symbols-outlined', +]); + +/** Every .js/.jsx file Tailwind scans (mirrors @source in src/css/tailwind.css). */ +function sourceFiles(dir) { + const out = []; + for (const e of fs.readdirSync(dir, { withFileTypes: true })) { + const p = path.join(dir, e.name); + if (e.isDirectory()) out.push(...sourceFiles(p)); + // .test. files hold fixture markup (this checker's own included) — not UI. + else if (/\.(js|jsx)$/.test(p) && !/\.test\./.test(p)) out.push(p); + } + return out; +} + +/** Class selectors from the built CSS, with CSS escapes (\: \/ \[) removed. */ +function emittedClasses() { + const files = fs.existsSync(DIST) ? fs.readdirSync(DIST).filter(f => f.endsWith('.css')) : []; + if (files.length === 0) { + console.error(`check:classes — no CSS in ${DIST}. Run \`npm run build\` first.`); + process.exit(2); + } + const set = new Set(); + for (const f of files) { + const css = fs.readFileSync(path.join(DIST, f), 'utf8'); + for (const m of css.matchAll(/\.((?:\\[^\s]|[a-zA-Z0-9_-])+)/g)) { + set.add(m[1].replace(/\\(.)/g, '$1')); + } + } + return set; +} + +/** Slice the expression after a `className=` / `…Classes =` match, so only + * strings that really hold classes are scanned (not class-shaped prop enums). */ +function sliceRegion(text, start) { + const open = { '{': '}', '[': ']' }; + const c = text[start]; + if (c === "'" || c === '"' || c === '`') { + for (let i = start + 1; i < text.length; i++) { + if (text[i] === '\\') i++; + else if (text[i] === c) return text.slice(start, i + 1); + } + return text.slice(start); + } + if (!open[c]) return ''; + const stack = [c]; + let i = start + 1; + while (i < text.length && stack.length) { + const ch = text[i]; + const top = stack[stack.length - 1]; + if (top === "'" || top === '"') { + if (ch === '\\') i++; + else if (ch === top) stack.pop(); + } else if (top === '`') { + if (ch === '\\') i++; + else if (ch === '`') stack.pop(); + else if (ch === '$' && text[i + 1] === '{') { + stack.push('{'); + i++; + } + } else if (ch === '/' && text[i + 1] === '/') { + i = text.indexOf('\n', i); + if (i < 0) break; + } else if (ch === '/' && text[i + 1] === '*') { + i = text.indexOf('*/', i) + 1; + if (i < 1) break; + } else if (ch === "'" || ch === '"' || ch === '`' || open[ch]) { + stack.push(ch); + } else if (ch === '}' || ch === ']') { + stack.pop(); + } + i++; + } + return text.slice(start, i); +} + +const REGION_START = /(?:className|\b[A-Za-z_$][\w$]*(?:Class|Classes|ClassName))\s*[=:]\s*/g; +const TOKEN = /^[a-zA-Z][a-zA-Z0-9_:/.%[\]-]*$/; + +/** Class tokens in one file; template-literal fragments touching a `${}` are + * dropped (`log-block--${lvl}` yields `log-block--`, not a class). */ +function usedClasses(text) { + const out = new Set(); + const strings = + /'([^'\\\n]*(?:\\.[^'\\\n]*)*)'|"([^"\\\n]*(?:\\.[^"\\\n]*)*)"|`([^`\\]*(?:\\.[^`\\]*)*)`/gs; + for (const start of text.matchAll(REGION_START)) { + // In a className expression a quoted object key IS a class (clsx + // conditionals); in a *Classes map it's a lookup key — skip only there. + const isAttr = start[0].startsWith('className'); + const region = sliceRegion(text, start.index + start[0].length); + for (const m of region.matchAll(strings)) { + // Key-colon has no space before it (prettier), a ternary's does. + if (!isAttr && region[m.index + m[0].length] === ':') continue; + const chunks = (m[1] ?? m[2] ?? m[3]).split(/\$\{[^}]*\}/s); + chunks.forEach((chunk, i) => { + const parts = chunk.split(/\s+/).filter(Boolean); + // A token glued to an interpolation is a fragment, not a class: + // `log-block--${lvl}` yields `log-block--`. + if (i > 0 && !/^\s/.test(chunk)) parts.shift(); + if (i < chunks.length - 1 && !/\s$/.test(chunk)) parts.pop(); + for (const p of parts) if (TOKEN.test(p)) out.add(p); + }); + } + } + return out; +} + +/** True when the class shares a `-` prefix with an emitted one — a live-family + * utility (`bg-canvas`), not a bespoke component class (`item-counter`). */ +function inLiveFamily(cls, prefixes) { + const bits = cls.split(':').pop().split('-'); + for (let i = bits.length - 1; i > 0; i--) { + if (prefixes.has(bits.slice(0, i).join('-') + '-')) return true; + } + return false; +} + +const emitted = emittedClasses(); +const prefixes = new Set(); +for (const cls of emitted) { + const bits = cls.split(':').pop().split('-'); + for (let i = 1; i < bits.length; i++) prefixes.add(bits.slice(0, i).join('-') + '-'); +} + +const missing = new Map(); +for (const file of sourceFiles(SRC)) { + for (const token of usedClasses(fs.readFileSync(file, 'utf8'))) { + if (emitted.has(token) || ALLOWLIST.has(token)) continue; + if (!token.includes('-') || !inLiveFamily(token, prefixes)) continue; + if (!missing.has(token)) missing.set(token, new Set()); + missing.get(token).add(file); + } +} + +if (missing.size === 0) { + console.log(`check:classes — OK (${emitted.size} classes emitted).`); + process.exit(0); +} +console.error(`check:classes — ${missing.size} class(es) used in ${SRC}/ but not emitted:\n`); +for (const [cls, files] of [...missing].sort()) { + console.error(` ${cls}\n ${[...files].join('\n ')}`); +} +console.error('\nDefine the token/utility in src/css/tailwind.css, or use the intended class.'); +process.exit(1); diff --git a/frontend/src/App.jsx b/frontend/src/App.jsx index 2512e336..ff0d5856 100755 --- a/frontend/src/App.jsx +++ b/frontend/src/App.jsx @@ -62,24 +62,103 @@ const BorderPreviewPage = React.lazy(() => import('./pages/poster/BorderPreviewP const UnmatchedAssetsPage = React.lazy(() => import('./pages/poster/UnmatchedAssetsPage.jsx')); const PosterStatsPage = React.lazy(() => import('./pages/poster/PosterStatsPage.jsx')); -// Lazy-loaded dev pages -const ErrorTestPage = React.lazy(() => import('./pages/dev/ErrorTestPage.jsx')); -const FieldTestPage = React.lazy(() => import('./pages/dev/FieldTestPage.jsx')); -const ApiTestPage = React.lazy(() => import('./pages/dev/ApiTestPage.jsx')); -const ToolbarTestPage = React.lazy(() => import('./pages/dev/ToolbarTestPage.jsx')); -const ToolbarCompoundTest = React.lazy(() => import('./pages/dev/ToolbarCompoundTest.jsx')); -const SpinnerTestPage = React.lazy(() => import('./pages/dev/SpinnerTestPage.jsx')); -const SettingsMockPage = React.lazy(() => import('./pages/dev/SettingsMockPage.jsx')); -const ArrayObjectFieldPage = React.lazy(() => import('./pages/dev/ArrayObjectFieldPage.jsx')); -const AccordionTestPage = React.lazy(() => import('./pages/dev/AccordionTestPage.jsx')); -const StatsPrimitivesTestPage = React.lazy(() => import('./pages/dev/StatsPrimitivesTestPage.jsx')); -const ButtonPrimitivesTestPage = React.lazy( - () => import('./pages/dev/ButtonPrimitivesTestPage.jsx') -); -const CardPrimitivesTestPage = React.lazy(() => import('./pages/dev/CardPrimitivesTestPage.jsx')); -const FormCompoundsTest = React.lazy(() => import('./pages/dev/FormCompoundsTest.jsx')); -const ModalsTestPage = React.lazy(() => import('./pages/dev/ModalsTestPage.jsx')); -const LogPerformance = React.lazy(() => import('./pages/dev/LogPerformance.jsx')); +// Dev-only pages. The React.lazy() calls MUST stay inside this conditional — +// DEV is statically false in prod, so vite drops the routes AND their chunks. +const devRoutes = import.meta.env.DEV + ? [ + { + path: 'dev/error', + pageName: 'Error Test', + pageDescription: 'Error handling demonstration page', + Component: React.lazy(() => import('./pages/dev/ErrorTestPage.jsx')), + }, + { + path: 'dev/fields', + pageName: 'Field Test', + pageDescription: 'Field system development testing interface', + Component: React.lazy(() => import('./pages/dev/FieldTestPage.jsx')), + }, + { + path: 'dev/api', + pageName: 'API Test', + pageDescription: 'API Testing', + Component: React.lazy(() => import('./pages/dev/ApiTestPage.jsx')), + }, + { + path: 'dev/toolbar', + pageName: 'Toolbar Test', + pageDescription: 'Toolbar overflow testing', + Component: React.lazy(() => import('./pages/dev/ToolbarTestPage.jsx')), + }, + { + path: 'dev/toolbar-compound', + pageName: 'Toolbar Compound Pattern Test', + pageDescription: 'Toolbar compound component pattern testing', + Component: React.lazy(() => import('./pages/dev/ToolbarCompoundTest.jsx')), + }, + { + path: 'dev/spinner', + pageName: 'Spinner Test', + pageDescription: 'Spinner component testing and development', + Component: React.lazy(() => import('./pages/dev/SpinnerTestPage.jsx')), + }, + { + path: 'dev/settings', + pageName: 'Settings Mock', + pageDescription: 'Settings accordion interface mockup and design exploration', + Component: React.lazy(() => import('./pages/dev/SettingsMockPage.jsx')), + }, + { + path: 'dev/array-object-field', + pageName: 'Array Object Field', + pageDescription: 'Unified ArrayObjectField component demonstration', + Component: React.lazy(() => import('./pages/dev/ArrayObjectFieldPage.jsx')), + }, + { + path: 'dev/accordion', + pageName: 'Accordion Test', + pageDescription: 'AccordionItem compound component validation and testing', + Component: React.lazy(() => import('./pages/dev/AccordionTestPage.jsx')), + }, + { + path: 'dev/stats', + pageName: 'Statistics Primitives Test', + pageDescription: 'Statistics System primitive composition and layout testing', + Component: React.lazy(() => import('./pages/dev/StatsPrimitivesTestPage.jsx')), + }, + { + path: 'dev/buttons', + pageName: 'Button Primitives Test', + pageDescription: 'Button System primitive composition and component testing', + Component: React.lazy(() => import('./pages/dev/ButtonPrimitivesTestPage.jsx')), + }, + { + path: 'dev/card', + pageName: 'Card Primitives Test', + pageDescription: 'Card System primitive composition and variant testing', + Component: React.lazy(() => import('./pages/dev/CardPrimitivesTestPage.jsx')), + }, + { + path: 'dev/form-compounds', + pageName: 'Form Compounds Test', + pageDescription: + 'Form System compound composition validation (Header, Section, Actions)', + Component: React.lazy(() => import('./pages/dev/FormCompoundsTest.jsx')), + }, + { + path: 'dev/modals', + pageName: 'Modal Test', + pageDescription: 'Modal System comprehensive testing and real-world examples', + Component: React.lazy(() => import('./pages/dev/ModalsTestPage.jsx')), + }, + { + path: 'dev/log-performance', + pageName: 'Log Performance Test', + pageDescription: 'Phase 2 Log Output component performance validation', + Component: React.lazy(() => import('./pages/dev/LogPerformance.jsx')), + }, + ] + : []; const SuspenseFallback = () => ; @@ -499,172 +578,30 @@ const App = () => { } /> - {/* Development Routes */} - - - - } - /> - - - - } - /> - - - - } - /> - - - - } - /> - - - - } - /> - - - - } - /> - - - - } - /> - - - - } - /> - - - - } - /> - - - - } - /> - - - - } - /> - - - - } - /> - - - - } - /> - - - - } - /> - - - - } - /> + {/* Development routes — dev builds only */} + {devRoutes.map( + ({ + path, + pageName, + pageDescription, + Component, + }) => ( + + + + } + /> + ) + )} { return null; } - const baseClasses = 'w-px h-5 bg-text-secondary mx-3 flex-shrink-0 self-center'; + const baseClasses = 'w-px h-5 bg-border-light mx-3 flex-shrink-0 self-center'; return