diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a4c1130..100067d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -22,6 +22,7 @@ jobs: api: ${{ steps.filter.outputs.api }} simulator: ${{ steps.filter.outputs.simulator }} frontend: ${{ steps.filter.outputs.frontend }} + webflask: ${{ steps.filter.outputs.webflask }} cicd: ${{ steps.filter.outputs.cicd }} wiki: ${{ steps.filter.outputs.wiki }} steps: @@ -65,6 +66,17 @@ jobs: - 'tools/emulator_spec/**' frontend: - 'frontend/**' + # The Flask NAS UI. Deliberately NOT including 'pyproject.toml' here: + # the `backend` filter already carries it, so a dependency bump + # (including every Dependabot PR) still gets ruff + mypy + the whole + # tests/unit run, which imports this package. Adding it here would + # only tack a docker build onto that traffic. 'docker-compose.yml' IS + # included because it defines the web-flask service this job builds. + webflask: + - 'openblade/web_flask/**' + - 'tests/unit/test_web_flask_app.py' + - 'Dockerfile.web' + - 'docker-compose.yml' cicd: - '.github/workflows/**' # docs/wiki/reference/{cli,api}.md are GENERATED from the Typer CLI @@ -101,11 +113,14 @@ jobs: needs: changes if: ${{ github.event_name == 'workflow_dispatch' || needs.changes.outputs.backend == 'true' || needs.changes.outputs.api == 'true' || needs.changes.outputs.simulator == 'true' }} runs-on: ubuntu-latest - # Advisory: ~300 pre-existing mypy errors across the god-file modules. Fixing - # them safely is a large, staged effort (some involve API param names that are - # part of the wire contract). This job still runs and reports, but does not - # fail CI. Not a required check. Tracked for incremental cleanup. - continue-on-error: true + # BLOCKING since the toolchain was pinned (mypy==2.3.0 via the dev extra) and + # the baseline cleared to zero errors under `strict = true`. If this job goes + # red, the fix is an annotation or a narrowing at the reported site with the + # PINNED version -- never a version bump in the same change, and never a + # widen-to-Any. `[tool.mypy]` carries no per-module overrides -- the whole + # package is checked at the same strictness -- and each of the four + # `# type: ignore` comments left in openblade/ (down from 18) carries its + # justification at the site. steps: - uses: actions/checkout@v5 - uses: actions/setup-python@v5 @@ -213,6 +228,32 @@ jobs: - run: npm run build - run: npm run test -- --run + web-flask-smoke: + name: web-flask-smoke + needs: changes + if: ${{ github.event_name == 'workflow_dispatch' || needs.changes.outputs.webflask == 'true' }} + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v5 + - uses: actions/setup-python@v5 + with: + python-version: "3.12" + cache: "pip" + - run: pip install -e '.[dev]' + # BLOCKING mypy, scoped to this package alone. `mypy openblade` in + # backend-typecheck has to stay advisory because of ~300 pre-existing errors + # in the god-file modules; openblade/web_flask is clean under strict mode + # today, and this job is what keeps it that way instead of letting it join + # the backlog. + - run: mypy openblade/web_flask + # Redundant with backend-tests (tests/unit covers this file) but kept so the + # job is a complete gate on its own for a web_flask-only change. Seconds. + - run: pytest tests/unit/test_web_flask_app.py -q --tb=short + # The only thing in CI that builds this image. web-flask sits behind a + # compose profile, so name it explicitly — a bare `docker compose build` + # skips profiled services. + - run: docker compose build web-flask + wiki-reference: name: wiki-reference needs: changes @@ -260,7 +301,7 @@ jobs: # branch-protection check (plus operability-gate). It understands a valid # `skipped` result from path-filtered jobs and fails on any real failure or # cancellation, so branch protection no longer has to reason about skips. - # backend-typecheck is advisory (reported, never blocking); backend-lint blocks. + # backend-lint and backend-typecheck both block through this gate now. ci-gate: name: ci-gate needs: @@ -270,6 +311,7 @@ jobs: - api-aml-integration - i3-smoke - frontend-build-test + - web-flask-smoke - cicd-workflow-validate - wiki-reference if: ${{ always() }} @@ -288,12 +330,20 @@ jobs: check api-aml-integration "${{ needs.api-aml-integration.result }}" check i3-smoke "${{ needs.i3-smoke.result }}" check frontend-build-test "${{ needs.frontend-build-test.result }}" + check web-flask-smoke "${{ needs.web-flask-smoke.result }}" check cicd-workflow-validate "${{ needs.cicd-workflow-validate.result }}" # Blocking on purpose: the fix is one command # (`python3 tools/gen_wiki_reference.py`) and a stale generated page # is indistinguishable from a correct one to a reader. check wiki-reference "${{ needs.wiki-reference.result }}" - echo "advisory (non-blocking): backend-lint=${{ needs.backend-lint.result }} backend-typecheck=${{ needs.backend-typecheck.result }}" + # Blocking since the mypy baseline was cleared to zero under + # `strict = true` and the toolchain pinned. A red typecheck means a new + # annotation is missing, which is cheap to fix at the reported site. + check backend-typecheck "${{ needs.backend-typecheck.result }}" + # Blocking: the pinned-toolchain baseline is zero findings, and a + # lint red outside this gate never actually gated when ci-gate is + # the only required check (the earlier flip missed this row). + check backend-lint "${{ needs.backend-lint.result }}" if [ "$fail" -ne 0 ]; then echo "::error::ci-gate: a required job failed or was cancelled" exit 1 diff --git a/.github/workflows/mhvtl-weekly.yml b/.github/workflows/mhvtl-weekly.yml index b6d4e6c..571e088 100644 --- a/.github/workflows/mhvtl-weekly.yml +++ b/.github/workflows/mhvtl-weekly.yml @@ -126,6 +126,18 @@ jobs: echo "--- systemd (assumption 3):" systemctl is-system-running 2>&1 || true ps -p 1 -o comm= || true + # Assumption 4, added after run 35688121678: mhvtl only registers SCSI + # targets; /dev/nstN comes from the kernel's `st` upper-level driver, + # which an Azure kernel keeps in linux-modules-extra rather than the + # base linux-modules package the generic kernel uses. + echo "--- upper-level SCSI drivers st/ch (assumption 4):" + modinfo st 2>&1 | head -3 || echo "st: NOT in this kernel's module set" + modinfo ch 2>&1 | head -3 || echo "ch: NOT in this kernel's module set" + apt-cache policy "linux-modules-extra-$(uname -r)" || true + # Assumption 5: /dev/sgN is 0660 root:tape, and env.sh runs sg_inq as + # the unprivileged runner user. + echo "--- identity of the user that will run env.sh (assumption 5):" + id || true echo "--- disk / cpu budget:" df -h / || true nproc || true @@ -160,9 +172,17 @@ jobs: timeout-minutes: 20 run: | set -euxo pipefail + # libsnmp-dev added after run 35688121678: ltfs configure looks for + # net-snmp >= 5.3 and there is no --disable-snmp, so without it the + # build dies at + # ./configure: line 14786: net-snmp-config: command not found + # configure: error: Package requirements (net-snmp >= 5.3) were not met + # libsnmp-dev is what provides net-snmp-config and the pkg-config file. + # The rehearsal host already had it installed for unrelated reasons, + # which is why docs/runbooks/mhvtl-rehearsal.md §1 never mentions it. sudo apt-get install -y --no-install-recommends \ build-essential autoconf automake libtool pkg-config git \ - libfuse-dev libicu-dev libxml2-dev uuid-dev + libfuse-dev libicu-dev libxml2-dev uuid-dev libsnmp-dev # ./configure probes `icu-config`, removed in ICU 62+. The documented # ICU_MODULE_CFLAGS / ICU_MODULE_LIBS escape hatch does NOT work — @@ -203,9 +223,24 @@ jobs: continue-on-error: true run: | set -euxo pipefail + # No tarball means the build step failed (or was skipped). Say so and + # stop, rather than emitting a confusing `tar: Cannot open` on top of + # the real error further up the log. + if [ ! -f /tmp/ltfs-stage.tar ]; then + echo "No /tmp/ltfs-stage.tar — the LTFS build did not produce one. See the build step above." + exit 1 + fi # The cached tarball only carries the build products; the shared # libraries it links against still have to be present at runtime. - sudo apt-get install -y --no-install-recommends libfuse2 libicu-dev libxml2 uuid-runtime || true + # Installed ONE AT A TIME on purpose: these names are release-specific + # (noble renamed libfuse2 -> libfuse2t64 and bumps libsnmpNN), and a + # single apt-get with one unknown name installs NOTHING while the + # trailing `|| true` hides it. Per-package means a wrong guess costs + # only that guess. + for pkg in libfuse2t64 libfuse2 libicu-dev libxml2 uuid-runtime \ + libsnmp40 libsnmp-dev; do + sudo apt-get install -y --no-install-recommends "$pkg" || true + done sudo tar -C / -xf /tmp/ltfs-stage.tar sudo ldconfig command -v mkltfs @@ -246,6 +281,31 @@ jobs: sudo dmesg | tail -80 || true cat /etc/mhvtl/device.conf || true + # Resolving the rig is its own step so that "the rig cannot be addressed" + # fails HERE, loudly, instead of leaking into the pytest step. + # + # Run 35688121678 is exactly why. The old code was + # `eval "$(scripts/mhvtl/env.sh)"` inside the pytest step: a failing + # command substitution does NOT fail the enclosing command even under + # `set -e`, so env.sh's refusal became `eval ""`, the step carried on, and + # the first visible symptom was `OPENBLADE_BACKEND: unbound variable` 20 + # lines later — with the real diagnosis scrolled off as mere stderr. + - name: Resolve the rig's device paths + id: rig-env + run: | + set -euo pipefail + # env.sh DISCOVERS the device paths (they move between boots) and + # identifies the changer by unit serial, never by "first mediumx". + if ! block=$(scripts/mhvtl/env.sh); then + echo "::error::scripts/mhvtl/env.sh could not resolve the rig; the hardware suite cannot run. The diagnosis is in this step's stderr above." + exit 1 + fi + printf '%s\n' "$block" + # Only the `export K=V` lines — env.sh also emits comments, which + # $GITHUB_ENV would reject. + printf '%s\n' "$block" \ + | sed -n 's/^export \([A-Z_][A-Z0-9_]*\)=\(.*\)$/\1=\2/p' >> "$GITHUB_ENV" + - name: Run the hardware suite against the rig timeout-minutes: 20 env: @@ -255,9 +315,13 @@ jobs: OPENBLADE_FAULT_TESTS: ${{ inputs.fault_tests == true && 'enabled' || '' }} run: | set -euxo pipefail - # env.sh DISCOVERS the device paths (they move between boots) and - # identifies the changer by unit serial, never by "first mediumx". - eval "$(scripts/mhvtl/env.sh)" + # Belt and braces: the previous step is what guarantees these exist, + # and if it somehow did not, say which one is missing. + : "${OPENBLADE_BACKEND:?not set — the rig-env step did not run}" + : "${OPENBLADE_REAL_HARDWARE_ENABLED:?not set — the rig-env step did not run}" + : "${OPENBLADE_CHANGER_DEVICE:?not set — the rig-env step did not run}" + : "${OPENBLADE_DRIVE_DEVICES:?not set — the rig-env step did not run}" + : "${OPENBLADE_SCRATCH_BARCODES:?not set — the rig-env step did not run}" env | grep '^OPENBLADE_' | sort if [ "${{ steps.ltfs.outputs.available }}" = "true" ]; then diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml new file mode 100644 index 0000000..da266b7 --- /dev/null +++ b/.github/workflows/release.yml @@ -0,0 +1,92 @@ +name: Release + +# Tag-triggered release: builds the controller image, pushes it to GHCR, and +# cuts a GitHub Release from the tag. Runs on `v*` tag pushes only -- it never +# runs on a branch push, so it cannot race the version-bump automation in +# other workflows. +# +# NOTE: this workflow has never been exercised by an actual tag push (no `v*` +# tag exists on this repo yet). The first real run is what proves the GHCR +# login/push and release-notes generation actually work end to end; until +# then, treat it as reviewed-but-unverified. See docs/wiki/guides/releasing.md +# for what to check if that first run fails, and keep changes to this file +# small and easy to bisect for exactly that reason. + +on: + push: + tags: + - "v*" + +concurrency: + group: release-${{ github.ref }} + cancel-in-progress: false + +# Workflow-wide default is read-only, matching every other workflow in this +# repo (ci.yml, operability.yml, emulator-change-gates.yml, ...). The one job +# that needs to push images/create a release grants exactly those extra +# scopes itself, rather than widening this default. +permissions: + contents: read + +jobs: + release: + name: build-and-release + runs-on: ubuntu-latest + permissions: + contents: write # to create the GitHub Release from the tag + packages: write # to push images to ghcr.io + steps: + - uses: actions/checkout@v5 + + - name: Derive version from the tag + id: version + run: | + tag="${GITHUB_REF#refs/tags/}" + version="${tag#v}" + echo "tag=${tag}" >> "$GITHUB_OUTPUT" + echo "version=${version}" >> "$GITHUB_OUTPUT" + + - name: Cross-check tag against pyproject [project] version + run: | + pyproject_version="$(python3 -c 'import tomllib; print(tomllib.load(open("pyproject.toml", "rb"))["project"]["version"])')" + if [ "$pyproject_version" != "${{ steps.version.outputs.version }}" ]; then + echo "::error::tag v${{ steps.version.outputs.version }} does not match pyproject version ${pyproject_version} -- bump pyproject.toml and re-tag" + exit 1 + fi + + - name: Log in to GHCR + uses: docker/login-action@v3 + with: + registry: ghcr.io + username: ${{ github.actor }} + password: ${{ secrets.GITHUB_TOKEN }} + + - name: Set up Docker Buildx + uses: docker/setup-buildx-action@v3 + + # Controller image: the root Dockerfile (FastAPI/Typer app). This is the + # only image this workflow builds -- see the Dockerfile.web note below. + - name: Build and push controller image + uses: docker/build-push-action@v6 + with: + context: . + file: Dockerfile + push: true + tags: | + ghcr.io/amantux/openblade:${{ steps.version.outputs.version }} + ghcr.io/amantux/openblade:latest + + # `Dockerfile.web` is not tracked on master as of this workflow's + # authoring (only `frontend/Dockerfile` exists, consumed by + # docker-compose's `web` service, not published standalone to GHCR). + # If/when a top-level `Dockerfile.web` is added for a standalone web + # image, add a matching build-push step here tagged + # `ghcr.io/amantux/openblade-web:{version,latest}` -- do not invent one + # now. + + - name: Create GitHub Release + uses: softprops/action-gh-release@v2 + with: + tag_name: ${{ steps.version.outputs.tag }} + name: ${{ steps.version.outputs.tag }} + generate_release_notes: true diff --git a/Dockerfile.web b/Dockerfile.web new file mode 100644 index 0000000..eb80963 --- /dev/null +++ b/Dockerfile.web @@ -0,0 +1,34 @@ +FROM python:3.12-slim + +WORKDIR /app + +COPY pyproject.toml README.md ./ +COPY openblade/ ./openblade/ + +RUN pip install --no-cache-dir -e . \ + && pip install --no-cache-dir gunicorn gevent packaging + +# packaging is explicit on purpose: gunicorn.workers.ggevent imports it, but +# neither gunicorn nor gevent declares it, so `-k gevent` died at container start +# with ModuleNotFoundError while `docker compose build` stayed green. Verified by +# running the image, which is the only way this class of bug shows up. + +# Non-root, mirroring Dockerfile. The Flask UI holds no state, so it needs no +# /data volume and therefore no chown entrypoint -- a plain USER switch suffices. +RUN groupadd -r openblade && useradd -r -g openblade -d /app openblade + +# production, NOT development. This image is the deployable artifact, and +# OPENBLADE_ENV gates two hardening checks in openblade.web_flask.app: _secret_key() +# refuses to start without FLASK_SECRET_KEY, and SESSION_COOKIE_SECURE defaults on. +# Baking "development" here silently disabled both for anyone deploying the image +# as-is. docker-compose.yml overrides it back to development for the local dev +# stack, where a per-boot random key and plain-HTTP cookies are what you want. +ENV OPENBLADE_ENV=production +ENV OPENBLADE_WEB_BACKEND_URL=http://api:8000 +ENV PYTHONUNBUFFERED=1 + +USER openblade + +EXPOSE 8080 + +CMD ["gunicorn", "-k", "gevent", "-w", "1", "-b", "0.0.0.0:8080", "openblade.web_flask.app:app"] diff --git a/README.md b/README.md index 1f09304..5755345 100644 --- a/README.md +++ b/README.md @@ -25,7 +25,7 @@ OpenBlade CI/CD is intentionally split by layer so each change runs only relevan | --- | --- | --- | | API + backend domain | `CI`: `backend-lint`, `backend-typecheck`, `backend-tests`, `api-aml-integration` | `openblade/**/*.py`, AML integration tests, backend config | | Simulator/emulator parity | `CI`: `i3-smoke`; `i3-emulator-compliance`; `emulator-change-gates` | simulator, AML routes, emulator contract/tools, i3 tests, compose/runtime wiring | -| Frontend/UI | `CI`: `frontend-build-test` | `frontend/**` | +| Frontend/UI | `CI`: `frontend-build-test` (React SPA), `web-flask-smoke` (Flask NAS UI) | `frontend/**`; `openblade/web_flask/**`, `Dockerfile.web`, `docker-compose.yml` | | CI/CD policy layer | `CI`: `cicd-workflow-validate` | `.github/workflows/**` | This keeps checks up to date and targeted while preserving full coverage on workflow dispatch and on emulator-specific workflows. @@ -39,6 +39,8 @@ openblade inventory uvicorn openblade.api.main:app --reload # Flask-style WSGI deployment option (same API behavior): gunicorn openblade.api.wsgi:application +# Flask NAS operator UI (a separate app that talks to the API over HTTP): +FLASK_SECRET_KEY=dev-only gunicorn -k gevent -w 1 -b 0.0.0.0:5175 openblade.web_flask.app:app ``` ## Multi-Library Setup @@ -60,6 +62,41 @@ gunicorn openblade.api.wsgi:application - Override UI proxy targets with `EMULATOR_UI_TARGET_LIBRARY{1,2,3}_URL` in the standalone env file - Override OpenBlade controller routing with `OPENBLADE_EMULATOR_URLS` when targeting the standalone emulator endpoints +## Flask NAS operator UI +`openblade/web_flask/` is a NAS-first operator console. It is a **separate WSGI +app**, not a second implementation: it owns no domain logic and reaches the +FastAPI control plane over HTTP through `web_flask.client.BackendClient`. It sits +alongside the React SPA in `frontend/` rather than replacing it — both surfaces +are named in `CLAUDE.md`, and each has its own CI job. + +Run it with `docker compose --profile web-flask up` (http://localhost:5175); it is +profile-gated so the default stack is unchanged. + +Surfaces, each backed by existing API endpoints: +- **Storage → Write Path** — replication/sharding/cache-drive/ingest policy + (`/nas/policies`, `/nas/cache-drives`, `/nas/source-stream`), and folder-to-pool + share mappings with per-folder access modes (`/nas/shares`) +- **Storage → Catalog** — dataset table with per-dataset checksum verification + (`/nas/datasets`, `/nas/datasets/{dataset_id}/verify`) +- **Storage → Archive / Restore** — archive and restore submission (`/archive/`, + `/restore/`) +- **Devices** — register/probe libraries and drive inventory, mount/unmount, move, + import/export, and magazine eject/insert against `/api/libraries` and `/aml/*` +- **System** — AML users and role assignments (`/aml/users`) + +### Configuration +| Variable | Purpose | +| --- | --- | +| `FLASK_SECRET_KEY` | Session signing key. **Required** when `OPENBLADE_ENV=production` (the `Dockerfile.web` default) — the app refuses to start without it rather than minting a throwaway key that would invalidate every session on restart. | +| `OPENBLADE_WEB_BACKEND_URL` | Base URL of the FastAPI control plane. | +| `OPENBLADE_API_TOKEN` | Must match the backend's value. The backend gates its native surface (`/api`, `/jobs`, `/nas`, `/catalog`, `/status`, `/system`, `/volume-groups`, `/archive`, `/restore`) on this bearer, while `/aml/*` uses the per-user session this UI obtains at login. Omit it only where the backend also leaves native auth off. | +| `OPENBLADE_SERVICE_TOKEN` | Must match the backend's value; `/aml/mount` and `/aml/unmount` require it. | +| `OPENBLADE_WEB_SECURE_COOKIES` | Forces `Secure` session cookies; defaults on under `OPENBLADE_ENV=production`. | +| `OPENBLADE_WEB_ALLOW_UNSAFE_DEVICE_TARGETS` | Development escape hatch: permits loopback, link-local (including cloud metadata) and multicast device connection URLs. By default these are refused based on the host's **resolved addresses**, so respellings like `127.1` or `2130706433` are refused too. RFC1918 LAN addresses are always allowed — that is where real libraries live. Leave unset in production. | +| `OPENBLADE_WEB_ALLOWED_PATH_ROOTS` | Comma-separated absolute roots that operator-supplied archive/restore/cache paths must sit under. Defaults to `/openblade,/var/lib/openblade,/pools,/data,/share,/shares,/datasets,/mnt,/srv`. This is a confinement boundary, not just a traversal check: the backend's `/archive` and `/restore` routes have no per-user authorization, so without it a logged-in operator could archive `/etc` or `/proc/self/environ` to tape. | +| `OPENBLADE_CONNECT_HOST` | Pins the hostname printed in the copy-paste `net use` / `mount -t cifs` / `sftp` hints. Otherwise the (character-validated) request `Host` header is used — pin this wherever that header is not trustworthy. | +| `OPENBLADE_WEB_TIMEOUT_SECONDS` | Backend HTTP timeout, default 8. | + ## Safety defaults - Mock backend is the default - Real hardware requires `OPENBLADE_BACKEND=real` and `OPENBLADE_REAL_HARDWARE_ENABLED=true` diff --git a/docker-compose.yml b/docker-compose.yml index 00bc9eb..7e97d47 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -7,6 +7,12 @@ services: OPENBLADE_BACKEND: mock OPENBLADE_DB_URL: sqlite:////data/openblade.db OPENBLADE_EMULATOR_URLS: ${OPENBLADE_EMULATOR_URLS:-http://host.docker.internal:8010,http://host.docker.internal:8011,http://host.docker.internal:8012} + # Declared so `api` and `web-flask` cannot drift apart on either credential. + # Empty is the documented "unset" for both (api_auth: empty => native auth + # disabled; service_auth: empty => dev fallback token), so the default stack + # behaves exactly as before. Export either var and BOTH services pick it up. + OPENBLADE_API_TOKEN: ${OPENBLADE_API_TOKEN:-} + OPENBLADE_SERVICE_TOKEN: ${OPENBLADE_SERVICE_TOKEN:-} volumes: - openblade-data:/data ports: @@ -40,5 +46,66 @@ services: api: condition: service_healthy + # Nightly catalog backup sidecar (off by default -- uncomment to enable). + # Mirrors `openblade catalog backup --dest /backups --keep 14` on a loop; + # gzip snapshots land on the `openblade-backups` volume, pruned to the 14 + # most recent. See docs/wiki/guides/the-catalog.md#openblade-catalog-backup--restore-backup. + # backup: + # build: + # context: . + # dockerfile: Dockerfile + # environment: + # OPENBLADE_BACKEND: mock + # OPENBLADE_DB_URL: sqlite:////data/openblade.db + # volumes: + # - openblade-data:/data + # - openblade-backups:/backups + # entrypoint: ["/bin/sh", "-c"] + # command: + # - | + # while true; do + # openblade catalog backup --dest /backups --keep 14 + # sleep 86400 + # done + # depends_on: + # api: + # condition: service_healthy + + # The Flask NAS operator UI (openblade/web_flask). ADDITIVE, not a replacement + # for `web` above: CLAUDE.md describes "React frontend + Flask NAS UI" as two + # surfaces, and frontend-build-test still gates the React one. Off by default + # via a profile, following `emulator`: `docker compose --profile web-flask up` + # runs it, and the default stack is byte-for-byte unchanged. Host port 5175 + # deliberately: 5173 is the React service and 5174 is the standalone emulator UI + # (deploy/emulator/docker-compose.standalone.yml), so both would clash under + # `make fleet-up`. + web-flask: + build: + context: . + dockerfile: Dockerfile.web + profiles: ["web-flask"] + environment: + # Relaxes Dockerfile.web's production default: the local stack is plain + # HTTP, so a per-boot random SECRET_KEY and non-Secure cookies are correct + # here. A real deployment keeps production and supplies FLASK_SECRET_KEY. + OPENBLADE_ENV: development + OPENBLADE_WEB_BACKEND_URL: http://api:8000 + # Both halves of the backend's auth model; both MUST match `api`. + # OPENBLADE_API_TOKEN is the native-surface bearer (empty => api has native + # auth off, so an empty bearer here is equally correct). + OPENBLADE_API_TOKEN: ${OPENBLADE_API_TOKEN:-} + # The controller header, required by /aml/mount and /aml/unmount, which this + # UI calls. Unlike the bearer it has no "off" state: when the var is unset + # api falls back to service_auth._DEFAULT_SERVICE_TOKEN, so the UI has to + # send that exact value or mount/unmount 403s in the dev stack. Naming the + # literal here keeps the two in step; exporting the var overrides both. + OPENBLADE_SERVICE_TOKEN: ${OPENBLADE_SERVICE_TOKEN:-openblade-controller-dev-token-do-not-expose} + ports: + - "5175:8080" + depends_on: + api: + condition: service_healthy + volumes: openblade-data: + # openblade-backups: diff --git a/docs/decisions/2026-09-11-parked-flask-nas-ci-readme.md b/docs/decisions/2026-09-11-parked-flask-nas-ci-readme.md index 0db7393..77cfae8 100644 --- a/docs/decisions/2026-09-11-parked-flask-nas-ci-readme.md +++ b/docs/decisions/2026-09-11-parked-flask-nas-ci-readme.md @@ -1,7 +1,13 @@ # Parked: CI + README tail of the Flask NAS UI and iBlade parity-coverage work **Date:** 2026-09-11 -**Status:** parked in `git stash` (not lost, not landed) +**Status:** ⚠️ SUPERSEDED 2026-09-26 — the parked state has been resolved. See +[`2026-09-26-flask-nas-disposition.md`](2026-09-26-flask-nas-disposition.md) for +the final per-component disposition and the stash cleanup commands. The "How to +recover" section below is **obsolete**: the Flask UI, `Dockerfile.web` and the +`web-flask-smoke` job were landed (with corrections), and the parity generator +plus its artifacts turned out to have already landed on master in `ce89749`. +Read the rest of this file as the historical record of *why* it was parked. **Origin:** abandoned session of 2026-08-22, checkpoint `caca9241` (see `.claude/RESUME.md`) ## Why this note exists diff --git a/docs/decisions/2026-09-26-flask-nas-disposition.md b/docs/decisions/2026-09-26-flask-nas-disposition.md new file mode 100644 index 0000000..28fd043 --- /dev/null +++ b/docs/decisions/2026-09-26-flask-nas-disposition.md @@ -0,0 +1,328 @@ +# Disposition: the parked Flask NAS UI and iBlade parity-coverage work + +**Date:** 2026-09-26 +**Status:** decided — the parked state is ENDED +**Supersedes:** [`2026-09-11-parked-flask-nas-ci-readme.md`](2026-09-11-parked-flask-nas-ci-readme.md) +**Branch:** `feat/flask-nas-disposition` + +The 2026-09-11 note parked four files in `git stash` plus a set of untracked +prerequisites, because they "could be neither safely landed nor honestly +discarded". This note closes that out. Each component was evaluated against +current `master` and decided on its own. + +## Summary + +| Component | Disposition | Why | +| --- | --- | --- | +| `openblade/web_flask/` (+ `tests/unit/test_web_flask_app.py`) | **LANDED** | All 46 endpoints it consumes now exist on master; it needed no functional port | +| `Dockerfile.web` | **LANDED, corrected** | Shipped a dead gevent worker, a root process, and dev-mode hardening | +| `docker-compose.yml` `web` rewire | **RETIRED**, replaced | The parked version retired the React SPA as a side effect | +| `ci.yml` `web-flask-smoke` job | **LANDED, filter corrected** | Its blocker (untracked inputs) is gone; the filter was over-broad | +| `tools/emulator_spec/generate_iblade_parity_coverage.py` | **RETIRED — already on master** | Landed independently in `ce89749`; master's copy is the formatted one | +| `emulator_contract/openblade_iblade_*parity*.{json,md}` | **RETIRED — already on master** | Byte-identical to the tracked files | +| `emulator-change-gates.yml` / `i3-emulator-compliance.yml` parity steps | **RETIRED (deferred)** | Out of scope here; see "Deferred" below | +| README parity/NAS hunks | **PARTIALLY LANDED** | Two hunks were wrong and were dropped | + +## What changed since the work was parked + +The parked note's central blocker was that the Flask UI called NAS/AML endpoints +that existed only in the abandoned session's dirty tree. **That is no longer +true.** Extracting every path from `BackendClient` and diffing it against +`app.openapi()` (862 paths on master) gives **46 present, 0 missing**, with +matching methods. The backend surface landed independently in the intervening +commits, chiefly `bd4f3b1`. + +Consequently the package needed no functional port at all: + +- `pytest tests/unit/test_web_flask_app.py` — **31/31 passed unchanged** +- `mypy openblade/web_flask` (strict) — clean, 4 files +- `ruff check` — clean + +The only mechanical work was `ruff format` drift, because the repo's formatter +baseline moved after the work was parked (`2026-09-11-pin-ruff-toolchain.md`). +The parity generator had drifted the *same* way, which is the tell that master's +copy is the descendant of the parked one and not a divergent reimplementation. + +## Per-component rationale + +### LANDED: `openblade/web_flask/` + +Kept because it is not a duplicate of the React SPA. It is a separate WSGI app +that owns **no** domain logic and reaches the FastAPI control plane over HTTP +through `web_flask.client.BackendClient`, which is exactly the surface +relationship `CLAUDE.md` describes with "React frontend + Flask NAS UI". Its +content is NAS-appliance-shaped (write-path policy, cache-drive staging, +source-stream profiles, folder-to-pool share mappings, dataset checksum +verification) rather than a second rendering of the SPA's pages. + +Two real auth defects were found and fixed, both by probing the running backend +rather than by reading it: + +1. **The native surface was entirely broken under native auth.** + `openblade/api/api_auth.py` gates the OpenBlade-native surface on the static + `OPENBLADE_API_TOKEN` bearer, while `/aml/*` authenticates per-user through + `routes_aml_auth.require_auth`. The client sent its AML *session* token as the + only bearer, so with `OPENBLADE_API_TOKEN` set, **all 21 native endpoints it + uses returned 401** — the whole Storage/Jobs/Reports/System surface. Verified + against a live uvicorn instance: `/jobs/`, `/nas/policies`, `/api/libraries` + and `/volume-groups/` all 401, while `/aml/*` worked. + + Fixed in one place. `require_auth` checks the `sessionID` cookie *before* the + bearer, so the session token now travels as that cookie and `Authorization` + is free for the native token. With no native token configured it falls back to + the previous behaviour, so nothing changes for deployments that leave native + auth off. + +2. **The login lockout was bypassable.** `_client_ip()` honoured the + caller-controlled `X-Forwarded-For`, so rotating the header minted a fresh + rate-limit bucket per attempt and defeated `_is_login_blocked` entirely. Now + peer-address only, matching the rule already documented on + `openblade.api.routes_assist.client_key`. + +Both fixes carry **mutation-checked** regression tests: restoring either +vulnerable line makes the new test fail (verified by doing it). Note that the +pre-existing `test_login_rate_limit_blocks_after_repeated_failures` passed +against the vulnerable code — it was vacuous for the forged-header attack, which +is precisely the failure mode the mutation rule exists to catch. + +`flask` moved into the runtime dependencies: `Dockerfile.web` serves this under +gunicorn, so `pip install -e .` has to be enough to import it. + +### LANDED, corrected: `Dockerfile.web` + +Three defects, all found by *running* the built image. The parked CI job's only +image check was `docker compose build`, which passes on all three: + +- **`-k gevent` was dead.** `gunicorn.workers.ggevent` imports `packaging`, which + neither gunicorn nor gevent declares. The container died at start with + `ModuleNotFoundError: No module named 'packaging'`. +- **`ENV OPENBLADE_ENV=development` was baked into the deployable artifact**, + silently disabling both hardening checks the code offers: `_secret_key()`'s + refusal to start without `FLASK_SECRET_KEY`, and `SESSION_COOKIE_SECURE`. Now + `production`, with compose overriding it back for the local plain-HTTP stack. +- **gunicorn ran as root**, against the repo's own convention in `Dockerfile`. + +Verified on the rebuilt image: it exits with "FLASK_SECRET_KEY is required in +production" when the key is absent; with the key it serves `/login` 200 with +`X-Frame-Options: DENY`, and every process reports `uid=999`. + +### RETIRED and replaced: the `docker-compose.yml` `web` rewire + +The parked hunk pointed the existing `web` service at `Dockerfile.web`, i.e. it +**replaced the React SPA with the Flask UI**. That is a bigger decision than +"land the parked tail", it contradicts `CLAUDE.md`'s two-surface description, and +it would have left `frontend-build-test` gating a service nothing deployed. The +parked note had already flagged the matching README deletion as suspect. + +Replaced with an **additive** `web-flask` service behind a compose profile, +following the `emulator` precedent in the same file, so `docker compose up` is +byte-for-byte unchanged. Host port **5175**: 5173 is the React service and 5174 +is the standalone emulator UI, so the parked choice of 5173 *and* an obvious +alternative of 5174 would both have clashed under `make fleet-up`. + +Both services now take `OPENBLADE_API_TOKEN` and `OPENBLADE_SERVICE_TOKEN` from +one variable each so they cannot drift apart on either credential. Empty is the +documented "unset" for both, so default behaviour is unchanged. The service +token's dev default must be repeated as a literal in compose (it cannot import +Python) because `/aml/mount` and `/aml/unmount` require it — a test asserts the +two copies agree. + +### LANDED, filter corrected: `web-flask-smoke` + +The parked filter included `pyproject.toml`, so it fired on every dependency +bump. The `backend` filter already carries that file, so a pyproject change still +gets ruff + mypy + the whole `tests/unit` run (which imports this package); +adding it here only tacked a docker build onto Dependabot traffic. +`docker-compose.yml` is kept, because it defines the service the job builds. + +Given the existing layering, the job's unique value is narrow but real: +`backend-lint` already runs ruff tree-wide and `backend-tests` already runs +`tests/unit`, but `mypy openblade` in `backend-typecheck` must stay +`continue-on-error` because of ~300 pre-existing errors in the god-file modules. +`openblade/web_flask` is strict-clean today, so this job type-checks it +**blocking** and keeps it out of that backlog. It is also the only thing in CI +that builds `Dockerfile.web`. Registered in `ci-gate`'s `needs` *and* its `check` +list — without the latter a failure would not have blocked anything. + +### RETIRED (zero loss): the parity generator and artifacts + +**These already landed on master** in `ce89749` ("Add iBlade Rev A parity +tracker"). The untracked copies are redundant: + +- `openblade_iblade_rev_a_parity.json`, `openblade_iblade_parity_coverage.json` + and `.md` are **byte-identical** to the tracked files. +- `generate_iblade_parity_coverage.py` differs only in line wrapping; master has + the `ruff format`ed version, i.e. master's copy is strictly the better one. + +Confirmed live: `python3 tools/emulator_spec/generate_iblade_parity_coverage.py +--allow-partial` exits 0 (`implemented=12, partial=4, missing=1`) and regenerates +the tracked artifacts with **no** diff. The parked note's recorded +`FileNotFoundError` is therefore stale, and the T3 "commits new +`emulator_contract/**` artifacts" concern no longer applies — that already +happened through the normal path. + +### Deferred, not landed: the parity steps in the two emulator workflows + +The generator step and artifact uploads for `emulator-change-gates.yml` and +`i3-emulator-compliance.yml` are **not** present on master — only the generator +itself landed. They are deliberately out of scope for this change, which was +scoped to touch `.github/workflows/` only for `web-flask-smoke`. + +Their original blocker is gone (the generator runs green and its inputs are +tracked), so this is now an ordinary small change rather than a parked one: add +`generate_iblade_parity_coverage.py --allow-partial` plus the two artifact paths +to those workflows. Worth doing; it just is not this change. **Nothing needs to +be recovered from the stash to do it** — the hunks are reproduced in the parked +note's own diff table and are four lines of YAML. + +### Partially landed: README + +Taken: the `web-flask-smoke` row in the layered-CI table, and a Flask NAS UI +section — rewritten rather than copied, with a configuration table that documents +the `OPENBLADE_API_TOKEN` lockstep requirement the parked version did not know +about. Every endpoint and Flask route named in it was verified to exist. + +**Not taken**, both deliberately: + +- The deletion of the `cd frontend && npm install && npm run test && npm run + build` quickstart line. The React frontend is not being retired and CI still + gates it. (The parked note flagged this itself.) +- The reword of goal 4 from "fleet and library workflows without out-of-scope + feature leakage" to "NAS-first operations with device-scoped deep controls". + Landing a UI is no reason to restate the project's goals, and the reword drops + an explicit scope-discipline clause. + +## Adversarial review round (T3) + +The reviewer returned **BLOCK** with four MUST-FIX items, two carrying working +repros against live sockets. All are fixed; the suite went 37 → 73 tests. + +| # | Finding | Fix | +| --- | --- | --- | +| 1 | **SSRF.** The device-URL deny-list compared host *spellings*, so `127.1`, `2130706433` and `localhost.` all reached a loopback listener (proven), and any attacker-controlled DNS name could resolve into private space. Gave a logged-in user an internal port scanner and a cloud-metadata reach test. | Resolve the host and judge every returned address | +| 2 | **Redirect bypass + credential replay.** The probe followed redirects, so a host answering `302 Location: http://127.0.0.1/…` turned a blocked target into a reachable one; httpx replays the body on 307/308, so the submitted device password could be harvested. | `follow_redirects=False`; 3xx no longer counts as reachable | +| 3 | **No path confinement.** The guard blocked `..` but not *which* absolute roots were reachable, and the values become real filesystem sinks in a backend whose `/archive` and `/restore` have no per-user authz — so `/proc/self/environ`, which holds `OPENBLADE_API_TOKEN`, was archivable to tape. | `_is_allowed_storage_root` on the four genuine filesystem paths | +| 4 | **Confused deputy.** Because the client presents the instance-wide `OPENBLADE_API_TOKEN` as the native bearer, and the native config surface has no `require_auth`, **any** session could rewrite storage policy regardless of AML role. Fix (2) in the landing commit turned "broken for everyone" into "authorized for everyone", so this was a widening introduced by that change. | Role gate on the six config-write routes, resolved from `/aml/users/me`, failing closed on an unknown role | + +Ten SHOULD-FIX/NIT items were also closed: dot-only identifiers (`_POOL_ID_ALLOWED` +permitted `.`, so `..` matched and collapsed a URL segment), unvalidated +`volume_group`, non-JSON upstream bodies echoed into the browser, 3xx crashing +`.json()`, an unbounded pre-auth login bucket plus a missing IP-only counter, no +logout revocation, backslash accepted in redirect targets, the reflected `Host` in +copy-paste mount commands, and bare `int()` on backend values. + +**One reviewer recommendation was deliberately not taken.** It proposed adding +`is_private` to the blocked-address set. That would block `10.x`, `172.16.x` and +`192.168.x` — which is exactly where a real Scalar i3 lives — and would refuse +every legitimate device. The check blocks loopback, link-local (which covers +169.254.169.254), unspecified, multicast and reserved, and allows RFC1918. This +matches the original intent of `_DANGEROUS_DEVICE_HOSTS`, which never listed +private ranges; the defect was that it compared spellings, not that its *scope* was +wrong. + +**Areas the reviewer attacked and found clean:** session/CSRF/privilege plumbing +(independently re-verified here — all 36 non-exempt endpoints 302 to login when +anonymous, and all 24 mutating routes return 400 without a CSRF token, with no GET +mutating state); template/XSS (no `|safe`, `Markup`, `render_template_string`, +inline ` + + + + + + + diff --git a/openblade/web_flask/templates/devices/index.html b/openblade/web_flask/templates/devices/index.html new file mode 100644 index 0000000..2799537 --- /dev/null +++ b/openblade/web_flask/templates/devices/index.html @@ -0,0 +1,51 @@ +{% extends "base.html" %} +{% block title %}Connected Devices · OpenBlade NAS{% endblock %} +{% block content %} + + +
+ Online: {{ summary.online }} + Degraded: {{ summary.warning }} + Offline: {{ summary.offline }} + Unknown: {{ summary.unknown }} + Total: {{ summary.total }} + Last update: initial +
+ + {% if devices %} +
+ {% for device in devices %} +
+
+ {{ status_text(device.status) }} + +
+

{{ device.name }}

+
+ Model: {{ device.model }} + URL: {{ device.emulator_url }} +
+
+ {% endfor %} +
+ {% else %} +
+

No devices registered yet

+

Register your first device to unlock library management pages.

+ Register your first device +
+ {% endif %} + + +{% endblock %} diff --git a/openblade/web_flask/templates/devices/manage.html b/openblade/web_flask/templates/devices/manage.html new file mode 100644 index 0000000..cd357b2 --- /dev/null +++ b/openblade/web_flask/templates/devices/manage.html @@ -0,0 +1,24 @@ +{% extends "base.html" %} +{% block title %}Manage {{ device.name }} · OpenBlade NAS{% endblock %} +{% block content %} + + +
+ {% for section_key, section_label in sections %} +
+

{{ section_label }}

+

Device-scoped {{ section_label | lower }} controls for {{ device.name }}.

+ Open {{ section_label }} +
+ {% endfor %} +
+{% endblock %} diff --git a/openblade/web_flask/templates/devices/manage_section.html b/openblade/web_flask/templates/devices/manage_section.html new file mode 100644 index 0000000..2d229fc --- /dev/null +++ b/openblade/web_flask/templates/devices/manage_section.html @@ -0,0 +1,723 @@ +{% extends "base.html" %} +{% block title %}{{ section|capitalize }} · {{ device.name }} · OpenBlade NAS{% endblock %} +{% block content %} + + + + + {% if section == "overview" %} +
+
+

Device Health

+

Status: {{ status_text(device.status) }}

+

Connection: {{ device.emulator_url }}

+
+
+

Library Snapshot

+ {% set status = section_data.library_status or {} %} +

Overall: {{ status_text(status.overall or status.get('overall', 'unknown')) }}

+

Drives: {{ section_data.drives|length }}

+

Media: {{ section_data.media_count or 0 }}

+
+
+ {% elif section == "inventory" %} +
+

Slot Inventory

+ {% set media = section_data.media or [] %} + {% if media %} + + + + + + + + + + + + {% for item in media %} + + + + + + + + {% endfor %} + +
BarcodeTypeStatePartitionSlot
{{ item.barcode or "—" }}{{ item.type or "—" }}{{ status_text(item.state or "unknown") }}{{ item.partition or "—" }}{{ item.slot or item.location or "—" }}
+ {% else %} +
+

No inventory records available

+

Run an inventory operation to refresh the media map for this device.

+ Go to Operations +
+ {% endif %} +
+ {% elif section == "drives" %} +
+

Drive Status

+ {% set drives = section_data.drives or [] %} + {% if drives %} + + + + + + + + + + + {% for drive in drives %} + + + + + + + {% endfor %} + +
SerialTypeStateLoaded Media
{{ drive.serialNumber or "—" }}{{ drive.type or "—" }}{{ status_text(drive.state or drive.status or "unknown") }}{{ drive.loadedMedia or "—" }}
+ {% else %} +
+

No drives reported

+

This library has not reported drives yet.

+
+ {% endif %} +
+ {% elif section == "partitions" %} +
+

Partitions

+ {% set partitions = section_data.partitions or [] %} + {% if partitions %} + + + + + + + + + + + {% for partition in partitions %} + + + + + + + {% endfor %} + +
NameTypeStateSlots
{{ partition.name or partition.partitionName or "—" }}{{ partition.type or "—" }}{{ status_text(partition.state or "unknown") }}{{ partition.slotCount or partition.totalSlots or "—" }}
+ {% else %} +
+

No partitions configured

+

This device currently has no partition records.

+
+ {% endif %} +
+ {% elif section == "jobs" %} +
+

Device Jobs

+ {% set jobs = section_data.jobs or [] %} + {% if jobs %} + + + + + + + + + + + + {% for job in jobs %} + + + + + + + + {% endfor %} + +
Job IDTypeStatusUpdatedError
{{ (job.id or "—")[:8] }}{% if job.id %}…{% endif %}{{ job_type_label(job.job_type or "unknown") }}{{ status_text(job.state or "unknown") }}{{ job.updated_at or "—" }}{{ job.error or "" }}
+ {% else %} +
+

No jobs for {{ device.name }}

+

Run an operation to create archive, restore, or inventory jobs for this device.

+ Go to Operations +
+ {% endif %} +
+ {% elif section == "operations" %} + {% set active_ops = section_data.active_ops or [] %} + {% set drives = section_data.drives or [] %} + {% set media = section_data.media or [] %} + {% set partitions = section_data.partitions or [] %} + {% set ie_stations = section_data.ie_stations or [] %} + {% set mounts = section_data.mounts or [] %} + {% set pools = section_data.pools or [] %} + {% set shares = section_data.shares or [] %} + {% set volume_groups = section_data.volume_groups or [] %} + {% set magazines = section_data.magazines or [] %} + {% set catalog_files = section_data.catalog_files or [] %} + {% set drive_media_index = section_data.drive_media_index or [] %} + {% set inventory_status = section_data.inventory_status or {} %} + {% set import_status = section_data.import_status or {} %} + {% set export_status = section_data.export_status or {} %} + {% if active_ops %} + + {% endif %} + + {% for item in media %} + {% if item.barcode %} + + {% endif %} + {% endfor %} + {% for mount in mounts %} + {% if mount.barcode %} + + {% endif %} + {% endfor %} + + + {% for drive in drives %} + + {% endfor %} + + + {% for item in media %} + {% if item.slot %} + + {% endif %} + {% endfor %} + + + {% for partition in partitions %} + {% set partition_name = partition.name or partition.partitionName %} + {% if partition_name %} + + {% endif %} + {% endfor %} + + + {% for station in ie_stations %} + + {% endfor %} + + + {% for pool in pools %} + + {% endfor %} + + + {% for file in catalog_files %} + {% if file.path %} + + {% endif %} + {% endfor %} + +
+

Operations

+
Operations directly control hardware behavior. Confirm each action before execution.
+
+
+

Operation Status

+

Inventory: {{ status_text(inventory_status.state or "idle") }}

+

Import: {{ status_text(import_status.state or "idle") }}

+

Export: {{ status_text(export_status.state or "idle") }}

+
+ + +
+
+ +
+

Move Media

+
+ + + + + +
+
+ +
+

Mount Media

+
+ + + + + +
+
+ +
+

Unmount Media

+
+ + + + +
+
+ +
+

Import from I/E Station

+
+ + + + +
+
+ +
+

Export to I/E Station

+
+ + + + +
+
+ +
+

Pool Controls

+
+ + + + + + + + + + + +
+
+ +
+

Map Share to Pool

+
+ + + + + +
+ {% if volume_groups %} +

Available Volume Groups: {% for group in volume_groups %}{{ group.id }}{% if not loop.last %}, {% endif %}{% endfor %}

+ {% endif %} +
+ +
+

Hydrate File from Tape

+
+ + + + +
+
+
+ +

Pools

+ {% if pools %} + + + + + + + + + + + + + {% for pool in pools %} + + + + + + + + + {% endfor %} + +
IDNameReplicationBackup OrderAccessVolume Groups
{{ pool.id }}{{ pool.name }}{{ pool.replication_factor or 1 }}{{ status_text(pool.backup_order_mode or "sequential") }}{{ status_text(pool.access_mode or "read_only") }}{{ (pool.volume_group_ids or [])|join(", ") or "—" }}
+ {% else %} +
+

No pools configured

+

Create a pool to control tape sequencing and replication behavior.

+
+ {% endif %} + +

Pool Shares

+ {% if shares %} + + + + + + + + + + + {% for share in shares %} + + + + + + + {% endfor %} + +
NamePathTypeWritable
{{ share.name }}{{ share.path }}{{ share_type_label(share.share_type or "pool") }}{{ "Yes" if share.writable else "No" }}
+ {% endif %} + +

Magazine Control

+ {% if magazines %} + + + + + + + + + + + + + + {% for magazine in magazines %} + + + + + + + + + + {% endfor %} + +
IDLocationStatusSlotsMapped SlotsTapesActions
{{ magazine.id }}{{ magazine.location or "—" }}{{ status_text(magazine.status or "unknown") }}{{ magazine.slotCount or 0 }} + {{ magazine.mappedSlotCount or 0 }} + {% if not magazine.slotMapAligned %} + Mismatch + {% endif %} + {{ magazine.occupiedSlots or 0 }} +
+
+ + +
+
+ + +
+
+
+ {% else %} +
+

No magazines reported

+

Magazine inventory will appear after physical sync and inventory scan.

+
+ {% endif %} + +

Tape and Content Search

+ + {% if drive_media_index %} + + + + + + + + + + + + + {% for item in drive_media_index %} + + + + + + + + + {% endfor %} + +
BarcodeDriveSlotPartitionStateCatalog Paths
{{ item.barcode }}{{ item.drive }}{{ item.slot }}{{ item.partition }}{{ status_text(item.state or "unknown") }} + {% if item.sample_paths %} +
    + {% for path in item.sample_paths %} +
  • {{ path }}
  • + {% endfor %} +
+ {% else %} + — + {% endif %} +
+ {% else %} +
+

No tape content indexed

+

Run inventory and archive jobs to populate searchable tape content.

+
+ {% endif %} + + {% if mounts %} +

Current Mounts

+ + + + + + + + + + + {% for mount in mounts %} + + + + + + + {% endfor %} + +
BarcodeDriveStateMounted
{{ mount.barcode or "—" }}{{ mount.drive or "—" }}{{ status_text(mount.state or "unknown") }}{{ mount.mountTime or "—" }}
+ {% endif %} + + {% if active_ops %} +

Active Operations

+ + + + + + + + + + + {% for job in active_ops %} + + + + + + + {% endfor %} + +
Job IDTypeStatusUpdated
{{ (job.id or "—")[:8] }}{% if job.id %}…{% endif %}{{ job_type_label(job.job_type or "unknown") }}{{ status_text(job.state or "unknown") }}{{ job.updated_at or "—" }}
+ {% endif %} +
+ {% elif section == "reports" %} +
+
+

Event Summary

+ {% set event_summary = section_data.event_summary or {} %} +

Total: {{ event_summary.total or 0 }}

+

Critical: {{ event_summary.critical or 0 }}

+

Warning: {{ event_summary.warning or 0 }}

+

Info: {{ event_summary.info or 0 }}

+
+
+

Alert Summary

+ {% set alert_summary = section_data.alert_summary or {} %} +

Critical: {{ alert_summary.critical or 0 }}

+

Warning: {{ alert_summary.warning or 0 }}

+

Info: {{ alert_summary.info or 0 }}

+
+
+
+

Recent Events

+ {% set events = section_data.events or [] %} + {% if events %} + + + + + + + + + + + {% for event in events %} + + + + + + + {% endfor %} + +
TimestampSeverityComponentMessage
{{ event.timestamp or "—" }}{{ status_text(event.severity or "unknown") }}{{ event.component or "—" }}{{ event.message or "—" }}
+ {% else %} +
+

No events recorded

+

This device has no reportable events yet.

+
+ {% endif %} +
+ {% elif section == "settings" %} +
+

Device Settings

+
+ + + + +
+ +
+
+
+ {% endif %} +{% endblock %} diff --git a/openblade/web_flask/templates/devices/register.html b/openblade/web_flask/templates/devices/register.html new file mode 100644 index 0000000..6e95538 --- /dev/null +++ b/openblade/web_flask/templates/devices/register.html @@ -0,0 +1,59 @@ +{% extends "base.html" %} +{% block title %}Register Device · OpenBlade NAS{% endblock %} +{% block content %} +
+

Register Device

+

Add a connected Quantum library endpoint to the dashboard.

+ +
+ + + + + + + + + + + + + + +
+ + Cancel +
+ +
+
+
+{% endblock %} diff --git a/openblade/web_flask/templates/jobs/index.html b/openblade/web_flask/templates/jobs/index.html new file mode 100644 index 0000000..99b4cb1 --- /dev/null +++ b/openblade/web_flask/templates/jobs/index.html @@ -0,0 +1,89 @@ +{% extends "base.html" %} +{% block title %}Jobs · OpenBlade NAS{% endblock %} +{% block content %} + + +
+ Total: {{ jobs_summary.total }} + Running: {{ jobs_summary.running }} + Pending: {{ jobs_summary.pending }} + Failed: {{ jobs_summary.failed }} + Last update: initial +
+ + {% if jobs %} +
+ + + + + + + + + + + + + + {% for job in jobs %} + + + + + + + + + + {% endfor %} + +
Job IDTypeStatusLibraryCreatedUpdatedError
{{ (job.id or "—")[:8] }}{% if job.id %}…{% endif %}{{ job_type_label(job.job_type) }}{{ status_text(job.state) }}{{ job.library_id or "—" }}{{ job.created_at }}{{ job.updated_at }}{{ job.error or "" }}
+
+ {% else %} +
+

No jobs yet

+

Jobs are created when archive, restore, or inventory operations are triggered.

+ Go to Devices +
+ {% endif %} + + +{% endblock %} diff --git a/openblade/web_flask/templates/login.html b/openblade/web_flask/templates/login.html new file mode 100644 index 0000000..f3db2f8 --- /dev/null +++ b/openblade/web_flask/templates/login.html @@ -0,0 +1,23 @@ +{% extends "base.html" %} +{% block title %}Login · OpenBlade NAS{% endblock %} +{% block content %} +
+

Sign in

+

Use your Quantum-compatible credentials to access NAS operations.

+
+ + {% if next_path %} + + {% endif %} + + + +
+
+{% endblock %} diff --git a/openblade/web_flask/templates/reports/index.html b/openblade/web_flask/templates/reports/index.html new file mode 100644 index 0000000..70c37d8 --- /dev/null +++ b/openblade/web_flask/templates/reports/index.html @@ -0,0 +1,81 @@ +{% extends "base.html" %} +{% block title %}Reports · OpenBlade NAS{% endblock %} +{% block content %} + + + {% if active_device %} +
+
+

Event Log

+ {% if events %} + + + + + + + + + + + {% for event in events %} + + + + + + + {% endfor %} + +
TimestampSeverityComponentMessage
{{ event.timestamp or "—" }}{{ status_text(event.severity or "unknown") }}{{ event.component or "—" }}{{ event.message or "—" }}
+ {% else %} +
+

No events found

+

No events were recorded for this filter on {{ active_device.name }}.

+
+ {% endif %} +
+
+

Alert Summary

+
+ Critical: {{ (event_summary.critical or 0) + (alert_summary.critical or 0) }} + Warnings: {{ (event_summary.warning or 0) + (alert_summary.warning or 0) }} + Info: {{ (event_summary.info or 0) + (alert_summary.info or 0) }} +
+

Total events: {{ event_summary.total or 0 }}

+
+
+ {% else %} +
+

No devices connected

+

Register a device first to unlock report and alert views.

+ Register Device +
+ {% endif %} +{% endblock %} + diff --git a/openblade/web_flask/templates/storage/index.html b/openblade/web_flask/templates/storage/index.html new file mode 100644 index 0000000..05c42b7 --- /dev/null +++ b/openblade/web_flask/templates/storage/index.html @@ -0,0 +1,677 @@ +{% extends "base.html" %} +{% block title %}Storage · OpenBlade NAS{% endblock %} +{% block content %} + + + + + {% if section == "overview" %} +
+ {% set catalog = section_data.catalog_status or {} %} + Catalog files: {{ catalog.total_file_records or 0 }} + Datasets: {{ catalog.total_datasets or 0 }} + Cartridges: {{ catalog.total_cartridges or 0 }} + + Library: {{ status_text((section_data.library_status or {}).overall or 'unknown') }} + +
+
+
+

Catalog Health

+

DB reachable: {{ "Yes" if (catalog.db_reachable or false) else "No" }}

+

Path mappings: {{ catalog.total_path_mappings or 0 }}

+

Last rebuild: {{ catalog.last_rebuild_status or "unknown" }}

+
+
+

Media Pools

+ {% set pools = section_data.media_pools or [] %} + {% if pools %} + + {% else %} +

No media pools reported for this device.

+ {% endif %} +
+
+ {% elif section == "access" %} + {% set access = section_data.share_access or [] %} + {% set pools = section_data.nas_pools or [] %} + {% set gateway_status = section_data.gateway_status or {} %} + {% set gateway_config = section_data.gateway_config or {} %} +
+ Connect host: {{ section_data.connect_host or "unknown" }} + NAS shares: {{ access|length }} + Virtual mounts enabled: {{ pools|selectattr("virtual_mount_enabled")|list|length }} + + SFTP gateway: {{ status_text(gateway_status.status or gateway_config.status or "unknown") }} + +
+ +
+
+

Windows mapped drive (SMB)

+

Use a share UNC path in File Explorer or map a drive letter via command line.

+
    +
  1. Open File Explorer and choose Map network drive.
  2. +
  3. Enter UNC path \\{{ section_data.connect_host }}\{{ (access[0].name if access else "inbox") }}.
  4. +
  5. Authenticate with your OpenBlade account (or configured gateway credential).
  6. +
+ {% if access %} +
{{ access[0].windows_command }}
+ + {% endif %} +
+
+

Linux mount (CIFS/NFS)

+

Create mountpoint then run CIFS (Windows-compatible) or NFS command.

+ {% if access %} +
{{ access[0].linux_cifs_command }}
+ +
{{ access[0].linux_nfs_command }}
+ + {% else %} +

No NAS shares are currently configured.

+ {% endif %} +
+
+ +
+

SFTP fallback connection

+ {% set username = section_data.example_username or current_user or "admin" %} + {% set gateway_port = gateway_config.bind_port or 2222 %} + {% set sftp_command = "sftp -P " ~ gateway_port ~ " " ~ username ~ "@" ~ (section_data.connect_host or "127.0.0.1") %} +

If SMB/NFS is unavailable, use SFTP for direct transfer into ingest/restore paths.

+
{{ sftp_command }}
+ +
+ +
+

Per-share quick commands

+ {% if access %} + + + + {% for item in access %} + + + + + + + + {% endfor %} + +
SharePurposeModeWindows UNCLinux CIFS
{{ item.name }}{{ share_type_label(item.share_type) }}{{ "Read/Write" if item.writable else "Read-only" }} +
{{ item.windows_unc }}
+ +
+
{{ item.linux_cifs_command }}
+ +
+ {% else %} +
+

No NAS shares configured

+

Create share definitions in NAS configuration before mounting from clients.

+
+ {% endif %} +
+ {% elif section == "write-path" %} + {% set policies = section_data.policies or [] %} + {% set cache_drives = section_data.cache_drives or [] %} + {% set pools = section_data.nas_pools or [] %} + {% set shares = section_data.nas_shares or [] %} + {% set source_stream = section_data.source_stream or {} %} + {% set recommendations = section_data.recommendations or [] %} + {% set metrics = section_data.metrics or {} %} +
+ Pools: {{ pools|length }} + Policies: {{ policies|length }} + Cache drives: {{ cache_drives|length }} + + Source stream: {{ "Enabled" if source_stream.enabled else "Disabled" }} + + Replicated pools: {{ metrics.replicated_pools or 0 }} + Sharded policies: {{ metrics.sharded_policies or 0 }} + Folder rules: {{ metrics.folder_rules or 0 }} +
+ + + {% for policy in policies %} + + {% endfor %} + + + {% for pool in pools %} + + {% endfor %} + + +
+
+

Policy for multi-tape writes

+

Tune shard size and parallelism here, then map folders to pools below.

+
+ + + + + + + + + + + + + +
+ +
+
+
+ +
+

Cache drive staging

+
+ + + + + + + + + + + + +
+ +
+
+
+ +
+

Streaming direct-to-tape

+
+ + + + + + + + +
+ +
+
+
+ +
+

Folder-to-pool access mapping

+
+ + + + + + + + +
+ +
+
+
+
+ +
+

Optimization guidance

+ +
+ +
+

Current policy and tape write profile

+ {% if policies %} + + + + {% for policy in policies %} + + + + + + + + + + + {% endfor %} + +
PolicyTypeIngestCopiesShardingShard SizeParallelismAuto Clean
{{ policy.name or policy.id }}{{ status_text(policy.policy_type or "balanced") }}{{ status_text(policy.default_ingest_mode or "cache_drive") }}{{ policy.copies_required or 1 }}{{ "Enabled" if policy.allow_sharding else "Disabled" }}{{ policy.shard_size_bytes or "—" }}{{ policy.max_parallelism or 1 }}{{ "Enabled" if policy.auto_clean_before_archive else "Disabled" }}
+ {% else %} +

No storage policies configured yet.

+ {% endif %} +
+ +
+

Pool and cache alignment

+ + + + {% for pool in pools %} + + + + + + + + {% endfor %} + {% if not pools %} + + {% endif %} + +
PoolReplicationBackup OrderCache TargetRestore Path
{{ pool.name or pool.id }}{{ pool.replication_factor or 1 }}{{ status_text(pool.backup_order_mode or "sequential") }}{{ pool.cache_target_id or "—" }}{{ pool.restore_target_path or "—" }}
No pools defined.
+ {% if cache_drives %} + + + + {% for drive in cache_drives %} + + + + + + + + + {% endfor %} + +
Cache DriveRootCapacityMin FreeEvictionStatus
{{ drive.name or drive.id }}{{ drive.root_path }}{{ drive.max_bytes }}{{ drive.min_free_bytes }}{{ status_text(drive.eviction_policy or "after_verified") }}{{ "Enabled" if drive.enabled else "Disabled" }}
+ {% else %} +

No cache drives configured yet.

+ {% endif %} +
+ +
+

Current folder access mappings

+ {% if shares %} + + + + {% for share in shares %} + {% set pool_ids = share.pool_ids or [] %} + {% set folder_rules = share.folder_mappings or [] %} + + + + + + + + {% endfor %} + +
SharePathPoolsFolder RulesAccess
{{ share.name or "—" }}{{ share.path or "—" }}{{ (pool_ids|join(", ")) if pool_ids else "—" }} + {% if folder_rules %} +
    + {% for rule in folder_rules %} +
  • {{ rule.folder_path }} → {{ rule.pool_id }} ({{ status_text(rule.access_mode or "read_only") }})
  • + {% endfor %} +
+ {% else %} + — + {% endif %} +
{{ "Read/Write" if share.writable else "Read-only" }}
+ {% else %} +

No share mappings configured yet.

+ {% endif %} +
+ {% elif section == "archive" %} +
+

Archive Dataset

+

Submit archive jobs from NAS scope; execution is routed through the selected device context.

+
+ + + +
+ +
+
+
+
+

Recent Archive Jobs

+ {% set jobs = section_data.archive_jobs or [] %} + {% if jobs %} + + + + {% for job in jobs %} + + + + + + + {% endfor %} + +
Job IDStatusCreatedError
{{ (job.id or "—")[:8] }}{% if job.id %}…{% endif %}{{ status_text(job.state) }}{{ job.created_at }}{{ job.error or "" }}
+ {% else %} +
+

No archive jobs

+

No archive jobs have been submitted yet.

+
+ {% endif %} +
+ {% elif section == "catalog" %} + {% set catalog = section_data.catalog_status or {} %} + {% set datasets = section_data.datasets or [] %} + {% set verify_snapshot = section_data.verify_snapshot or {} %} +
+ Total files: {{ catalog.total_file_records or 0 }} + Datasets: {{ catalog.total_datasets or 0 }} + Tape records: {{ catalog.total_cartridges or 0 }} + Mapped paths: {{ catalog.total_path_mappings or 0 }} +
+ {% if verify_snapshot %} +
+

Last verify run

+

+ Dataset {{ verify_snapshot.dataset_id }} verified + {{ verify_snapshot.files_verified or 0 }} files, + corrupt {{ verify_snapshot.files_corrupt or 0 }}, + checksums updated {{ verify_snapshot.files_updated or 0 }}. +

+
+ {% endif %} +
+
+

Dataset verification workbench

+ +
+

Run tape checksum verification per dataset after archive or import operations.

+ {% if datasets %} + + + + {% for item in datasets %} + + + + + + + + + + {% endfor %} + +
DatasetStatusPoolFilesBytesSourceAction
+ {{ item.name or item.id }} +
{{ item.id }}
+
{{ status_text(item.status or "pending") }}{{ item.pool_id or "—" }}{{ item.file_count or 0 }}{{ item.total_bytes or 0 }}{{ item.source_path or "—" }} + {% if item.id %} +
+ + +
+ {% else %} + — + {% endif %} +
+ {% else %} +
+

No datasets available for verification

+

Archive files first, then verify checksums from this workbench.

+
+ {% endif %} +
+
+

Catalog Files

+ {% set files = (section_data.catalog or {}).files or [] %} + {% if files %} + + + + {% for item in files %} + + + + + + + {% endfor %} + +
PathSizeBarcodeShard
{{ item.path }}{{ item.size_bytes }}{{ item.primary_barcode or "—" }}{{ item.shard_count or 1 }}
+ {% else %} +
+

Catalog is empty

+

Archive datasets to populate catalog records.

+
+ {% endif %} +
+ {% elif section == "restore" %} +
+

Restore Dataset

+
+ + + +
+ +
+
+
+
+

Recent Restore Jobs

+ {% set jobs = section_data.restore_jobs or [] %} + {% if jobs %} + + + + {% for job in jobs %} + + + + + + + {% endfor %} + +
Job IDStatusCreatedError
{{ (job.id or "—")[:8] }}{% if job.id %}…{% endif %}{{ status_text(job.state) }}{{ job.created_at }}{{ job.error or "" }}
+ {% else %} +
+

No restore jobs

+

Submit a restore request to repopulate this queue.

+
+ {% endif %} +
+ {% endif %} +{% endblock %} diff --git a/openblade/web_flask/templates/system/index.html b/openblade/web_flask/templates/system/index.html new file mode 100644 index 0000000..ae1de78 --- /dev/null +++ b/openblade/web_flask/templates/system/index.html @@ -0,0 +1,121 @@ +{% extends "base.html" %} +{% block title %}System · OpenBlade NAS{% endblock %} +{% block content %} + + + +
+
+

Overall

+

+ {% set overall = system_status.overall or health.status or 'unknown' %} + {{ status_text(overall) }} +

+

Version: {{ config.version or "unknown" }}

+
+
+

Build & Runtime

+

Backend: {{ config.backend or "unknown" }}

+

Libraries: {{ config.library_count or 0 }}

+

Gateway: {{ "Enabled" if config.gateway_enabled else "Disabled" }}

+
+
+

Catalog

+

DB reachable: {{ "Yes" if catalog_status.db_reachable else "No" }}

+

Total files: {{ catalog_status.total_file_records or 0 }}

+

Total datasets: {{ catalog_status.total_datasets or 0 }}

+
+
+ +
+

Component Status

+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
ComponentStatus
CPU{{ status_text(system_status.cpu or "unknown") }}
Memory{{ status_text(system_status.memory or "unknown") }}
Disk{{ status_text(system_status.disk or "unknown") }}
Network{{ status_text(system_status.network or "unknown") }}
Services{{ status_text(system_status.services or "unknown") }}
+
+ +
+
+

Multi-user accounts

+ {% if users %} + + + + + + + + + + {% for user in users %} + + + + + + {% endfor %} + +
UserRolePassword Change
{{ user.name }}{{ aml_role_label(user.role) }}{{ "Required" if user.requirePasswordChange else "No" }}
+ {% else %} +

No user records are visible for this session.

+ {% endif %} +
+
+

Create user

+
+ + + + +
+ +
+
+
+
+{% endblock %} diff --git a/pyproject.toml b/pyproject.toml index d964fa5..5cf6af1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -28,6 +28,11 @@ dependencies = [ "httpx>=0.27", "pyotp>=2.9", "python-multipart>=0.0.9", + # openblade.web_flask: the NAS-first operator UI. It is a WSGI app SEPARATE from + # the FastAPI surface and reaches it over HTTP (web_flask.client.BackendClient), + # so Flask is a runtime dependency rather than a dev one: Dockerfile.web serves + # it under gunicorn, and a plain `pip install -e .` must be enough to import it. + "flask>=3.0", ] [project.optional-dependencies] @@ -52,7 +57,18 @@ dev = [ # so the pin costs zero churn). Bump deliberately, in its own commit, after # re-running `ruff check . && ruff format --check .`. "ruff==0.15.22", - "mypy>=1.11", + # EXACT pin, for the same reason as ruff above, now that backend-typecheck is a + # blocking gate: mypy adds error codes and tightens inference between minor + # releases, so a floating `>=` would let a new upload turn a green tree red with + # no commit behind it. Pinned to the version the zero-error baseline was cleared + # against (verified: `mypy openblade` exits 0 on 2.3.0). Bump deliberately, in + # its own commit, after re-running `mypy openblade`. + "mypy==2.3.0", + # Stubs for PyYAML (a transitive runtime dep via uvicorn[standard]), so + # openblade/nas/sidecar.py type-checks without a blanket import ignore. + # Pinned for the same reason as the two above: a stub release can introduce + # errors in unchanged code, and backend-typecheck now blocks. + "types-PyYAML==6.0.12.20260906", "asyncssh>=2.21", "pyfakefs>=5.6", ] @@ -70,13 +86,27 @@ target-version = "py312" [tool.ruff.lint] # EXPLICIT rule set — never rely on ruff's defaults, which widen between minor # releases (this is why the version above is pinned exactly as well). Anything -# not listed here is OFF, including rules referenced by `# noqa` comments in the -# tree: 23 `# noqa: BLE001` and 4 `# noqa: S3xx/S4xx` markers are currently inert -# documentation, because BLE and S are deliberately not selected yet. Enabling -# them is a scoped follow-up (BLE = 44 findings, S = 3,748, mostly S101 in -# tests) — see docs/decisions/2026-09-11-pin-ruff-toolchain.md. Do not delete those -# noqa markers to "clean up"; they record intent for that follow-up. -select = ["E", "F", "I", "B", "UP", "A", "SIM"] +# not listed here is OFF. +# +# BLE is now SELECTED, so every `# noqa: BLE001` in the tree is live and enforced +# rather than inert documentation. The policy behind those markers: a broad +# `except Exception` is legitimate only at a boundary whose job is to catch +# everything (job/step runners, CLI top level, health probes, content-negotiation +# fallbacks, best-effort probes and demo seeding, real-hardware test teardown). +# Each one carries a reason naming WHERE the error is logged and WHAT curated form +# escapes — `safe_job_error()` (openblade/domain/errors.py) for anything that +# reaches an unauthenticated field, a degraded status for health, a response +# header for content negotiation. A handler that can name its failure modes is +# narrowed instead; a handler that swallows silently is a bug, not a style issue. +# +# S stays deliberately UNSELECTED: 3,748 findings, overwhelmingly S101 (`assert` +# in tests, which is the point of tests). The `# noqa: S6xx/S3xx/S4xx` markers +# that used to sit in the tree pretending to be enforcement are now plain +# comments stating the safety property they documented (argv-list/`shell=False` +# discipline in scripts/deploy.py and the subprocess-based tests, the +# operator-supplied probe URL). Selecting S is still a separate, scoped decision +# — see docs/decisions/2026-09-11-pin-ruff-toolchain.md. +select = ["E", "F", "I", "B", "UP", "A", "SIM", "BLE"] # A002 (argument shadows a builtin) is ignored: FastAPI path/query params like # `id`, `type`, `filter` ARE the wire contract — renaming them would break the # /aml endpoints (see docs/tech-debt.md). diff --git a/scripts/campaign/restore_and_verify.py b/scripts/campaign/restore_and_verify.py index 68d8a1a..7d652bc 100644 --- a/scripts/campaign/restore_and_verify.py +++ b/scripts/campaign/restore_and_verify.py @@ -71,7 +71,10 @@ def main(argv: list[str] | None = None) -> int: path = record.path try: _, instance = catalog.get_latest_instance_for_path(path) - except Exception: + # A path with no resolvable archived instance is not a campaign target; + # skipping it is correct, but the skip is now visible in the run log. + except Exception as exc: # noqa: BLE001 - see above + print(f"skipping {path}: no archived instance ({type(exc).__name__})", file=sys.stderr) continue targets.append((path, instance.barcode)) diff --git a/scripts/deploy.py b/scripts/deploy.py index c6c0cfd..e63d355 100644 --- a/scripts/deploy.py +++ b/scripts/deploy.py @@ -16,7 +16,7 @@ import json import os import shlex -import subprocess # noqa: S404 - deploy command is operator-supplied, run without a shell +import subprocess # argv-list only, shell=False — see _deploy() import sys import urllib.error import urllib.request @@ -37,7 +37,9 @@ def _precheck() -> StageResult: def _deploy(cmd: list[str] | None) -> StageResult: if not cmd: return StageResult(Stage.DEPLOY, True, "skipped (no --deploy-cmd)") - completed = subprocess.run(cmd, check=False) # noqa: S603 - list form, no shell + # `cmd` is already an argv list (shlex.split of --deploy-cmd) and no shell=True: + # an operator string can therefore never become shell syntax. + completed = subprocess.run(cmd, check=False) ok = completed.returncode == 0 return StageResult(Stage.DEPLOY, ok, f"`{' '.join(cmd)}` exited {completed.returncode}") @@ -68,7 +70,10 @@ def _postcheck_live(base_url: str) -> StageResult: def probe(method: str, path: str) -> int: req = urllib.request.Request(base_url.rstrip("/") + path, method=method) try: - with urllib.request.urlopen(req, timeout=5) as resp: # noqa: S310 - operator-supplied URL + # The URL is operator-supplied (--postcheck-url), so the scheme is not + # constrained here; this runs as a deliberate deploy-time probe from the + # operator's own shell, never on behalf of a request. + with urllib.request.urlopen(req, timeout=5) as resp: return resp.status except urllib.error.HTTPError as exc: return exc.code diff --git a/scripts/mhvtl/_rig.sh b/scripts/mhvtl/_rig.sh index 5fd4fe9..62c8d1f 100755 --- a/scripts/mhvtl/_rig.sh +++ b/scripts/mhvtl/_rig.sh @@ -26,11 +26,74 @@ rig_die() { printf 'ERROR: %s\n' "$*" >&2; exit 1; } # resorting to a substring test - a loose match here could select a real # library, which is the whole thing this is guarding against. rig_serial_of() { - sg_inq "$1" 2>/dev/null \ + rig_sg_inq "$1" \ | awk -F': *' '/Unit serial number/{print $2; exit}' \ | awk '{print $NF}' } +# `sg_inq` on a device, escalating to `sudo -n` when the calling user cannot +# open the node. +# +# mhvtl's /dev/sgN nodes are mode 0660 root:tape. On the bare-metal rehearsal +# host the operator was in the `tape` group, so a plain `sg_inq` worked. A +# GitHub-hosted runner's `runner` user is NOT in `tape`, so every `sg_inq` +# failed with EACCES; because the serial lookup swallows stderr, that surfaced +# as an EMPTY serial and rig_changer reported "found medium changer(s) /dev/sg1 +# but none reporting unit serial 'OBLADE_L10'" on a perfectly healthy rig +# (mhvtl weekly run 35688121678). A permission problem must not be +# indistinguishable from "that is somebody else's library". +rig_sg_inq() { + local dev="$1" + if [ -r "$dev" ] && sg_inq "$dev" 2>/dev/null; then + return 0 + fi + [ "$(id -u)" -eq 0 ] && return 1 + command -v sudo >/dev/null || return 1 + sudo -n sg_inq "$dev" 2>/dev/null +} + +# True when this user cannot read the sg node directly and cannot escalate. +# Used to turn "no serial" into an accurate diagnosis instead of a wrong one. +rig_sg_unreadable() { + local dev="$1" + [ -r "$dev" ] && return 1 + [ "$(id -u)" -eq 0 ] && return 1 + sudo -n true 2>/dev/null && return 1 + return 0 +} + +# Echo the no-rewind tape node (/dev/nstN) for an sg node, using the same sysfs +# relationship openblade.hardware.discovery.resolve_sg_device() reads forwards. +# st and sg numbers are allocated independently (st0 -> sg1 and st2 -> sg4 are +# both real pairings on this rig), so this must be looked up, never derived. +# +# Failure means the kernel's `st` upper-level driver has not claimed the device +# — see the "upper-level SCSI drivers" section of setup.sh. +rig_nst_for_sg() { + local want="${1#/dev/}" tape + for tape in /sys/class/scsi_tape/nst*; do + [ -e "$tape/device/scsi_generic/$want" ] || continue + printf '/dev/%s' "$(basename "$tape")" + return 0 + done + return 1 +} + +# Why there is no /dev/nst node. Printed by both setup.sh and env.sh so the +# operator gets the same diagnosis whichever one they hit first. +rig_no_nst_hint() { + cat <<'EOF' +The kernel's SCSI tape upper-level driver ('st') has not attached to the rig's +drives: no /dev/nstN nodes exist and lsscsi prints '-' in the block column. +On Ubuntu's generic kernel st/ch ship inside linux-modules-$(uname -r) and udev +autoloads them, which is why the bare-metal rehearsal host never needed this. +On an Azure-tuned kernel (GitHub-hosted runners) they live in +linux-modules-extra-$(uname -r), which is NOT installed on the runner image. +Fix: sudo apt-get install -y "linux-modules-extra-$(uname -r)" + sudo modprobe st && sudo scripts/mhvtl/setup.sh +EOF +} + # Echo the /dev/sg node of THIS rig's medium changer. rig_changer() { local scsi dev serial candidates="" @@ -52,6 +115,14 @@ EOF rig_die "no medium changer with an sg node found - is the rig up? Run: sudo scripts/mhvtl/setup.sh" fi + for dev in $candidates; do + if rig_sg_unreadable "$dev"; then + rig_die "cannot read $dev to check its unit serial (it is mode 0660 + root:tape and this user is neither root, in the 'tape' group, nor able + to 'sudo -n'). This is a PERMISSION problem, not a wrong library. + Re-run as root, or: sudo usermod -aG tape \$USER (then re-login)." + fi + done rig_die "found medium changer(s)$candidates but none reporting unit serial '$RIG_CHANGER_SERIAL'. Refusing to guess: a real library attached to this host must not be driven by the rehearsal scripts. Check diff --git a/scripts/mhvtl/env.sh b/scripts/mhvtl/env.sh index 9451c0f..4ff06cb 100755 --- a/scripts/mhvtl/env.sh +++ b/scripts/mhvtl/env.sh @@ -29,25 +29,16 @@ changer_sg=$(rig_changer) # issuing SCSI commands on a rewinding node makes it rewind on close, which # fails on an empty drive and is a corruption footgun under LTFS. See Phase 0.3 # of docs/runbooks/real-i3-bringup-plan.md. -# Map an sg node back to its no-rewind tape node using the same sysfs -# relationship openblade.hardware.discovery.resolve_sg_device() reads forwards. -# st and sg numbers are allocated independently (st0 -> sg1 and st2 -> sg4 are -# both real pairings here), so this must be looked up, never derived. -nst_for_sg() { - local want="${1#/dev/}" tape - for tape in /sys/class/scsi_tape/nst*; do - [ -e "$tape/device/scsi_generic/$want" ] || continue - printf '/dev/%s' "$(basename "$tape")" - return 0 - done - return 1 -} +# The sg -> nst lookup lives in _rig.sh (rig_nst_for_sg) so setup.sh can verify +# it at bring-up time rather than leaving the test suite to discover it. drive_nst="" while read -r sg; do [ -n "$sg" ] || continue - node=$(nst_for_sg "$sg") \ - || rig_die "could not map $sg back to a /dev/nst node via sysfs" + if ! node=$(rig_nst_for_sg "$sg"); then + rig_no_nst_hint >&2 + rig_die "could not map $sg back to a /dev/nst node via sysfs (see above)" + fi drive_nst="${drive_nst:+$drive_nst,}${node}" done <&2; exit 1; } +# rig_changer / rig_drive_sgs / rig_nst_for_sg — the SAME helpers env.sh uses. +# Verifying the rig with them at the end of this script is deliberate: whatever +# would make env.sh refuse must fail HERE, where the diagnosis is in reach, not +# three steps later as an unbound-variable error inside the test runner. +# shellcheck source=scripts/mhvtl/_rig.sh +. "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/_rig.sh" + [ "$(id -u)" -eq 0 ] || fail "must run as root (needs modprobe + /etc/mhvtl)" # ---------------------------------------------------------------- packages @@ -115,6 +122,48 @@ else fi module_loaded || fail "mhvtl module is not loaded after modprobe" +# --------------------------------------------- upper-level SCSI drivers (st/ch) +# mhvtl registers SCSI *targets*. The /dev/nstN (tape) and /dev/schN (changer) +# nodes are created by the kernel's UPPER-LEVEL drivers `st` and `ch`, which are +# a separate matter from mhvtl entirely. +# +# Ubuntu's generic kernel ships st/ch inside linux-modules-$(uname -r) and udev +# autoloads them on device attach, which is why the bare-metal rehearsal host +# (5.15.0-174-generic) got /dev/st0-2 and /dev/sch0 for free and this script +# never mentioned them. An Azure-tuned kernel — i.e. every GitHub-hosted runner +# — puts them in linux-modules-extra-$(uname -r), which is NOT installed on the +# runner image. Evidence, mhvtl weekly run 35688121678: lsscsi printed all three +# tape rows with "-" in the block column, /sys/class/scsi_tape was empty, and +# env.sh could not map a single sg node to an nst node. +# +# `st` is REQUIRED: OPENBLADE_DRIVE_DEVICES is a list of /dev/nstN nodes and the +# hardware backend has nothing to talk to without them. `ch` is optional — the +# changer is driven through its sg node via mtx — so it is best-effort. +ensure_scsi_driver() { + local mod="$1" + # A built-in driver has no /proc/modules row; modprobe still exits 0 for it. + modprobe "$mod" 2>/dev/null && return 0 + [ "${SKIP_APT:-0}" = "1" ] && return 1 + log "'$mod' is not in this kernel's module set — trying linux-modules-extra-$(uname -r)" + DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends \ + "linux-modules-extra-$(uname -r)" || true + depmod -a || true + modprobe "$mod" 2>/dev/null +} + +log "Ensuring the SCSI tape ('st') and changer ('ch') drivers are loaded" +ensure_scsi_driver st || fail "the kernel's SCSI tape driver 'st' is unavailable + on this host, so no /dev/nstN nodes can ever appear and the OpenBlade + hardware backend has no drive to open. + Running kernel: $(uname -r) + Tried: modprobe st, then apt-get install linux-modules-extra-\$(uname -r). + Check 'apt-cache policy linux-modules-extra-$(uname -r)' and + 'modinfo st'." +ensure_scsi_driver ch \ + || printf "NOTE: the 'ch' changer driver is unavailable; no /dev/schN node.\n\ +This rig drives the changer through its /dev/sg node with mtx, so this is not\n\ +fatal — it only means lsscsi shows '-' in the changer's block column.\n" + # ---------------------------------------------------------------- userspace log "Building and installing mhvtl userspace daemons" make -C "$MHVTL_SRC" @@ -229,12 +278,25 @@ fi # /dev/sg nodes are created by udev a moment after that. Waiting only for the # mediumx ROW is not enough: lsscsi will happily print it with a "-" in the sg # column, and the rig then looks broken. Wait for the sg node itself. +# +# The /dev/nstN nodes are a THIRD, independent event: `st` binds the device and +# then udev creates the node. Waiting only for the sg nodes let setup.sh declare +# "Rig is up" on a rig whose drives had no tape node at all — so wait for the +# scsi_tape class entries too. log "Waiting for SCSI devices and udev to settle" for _ in $(seq 1 30); do sleep 1 changers_sg=$(lsscsi -g 2>/dev/null | awk '/ mediumx /{print $NF}' | grep -c '^/dev/sg' || true) drives_sg=$(lsscsi -g 2>/dev/null | awk '/ tape /{print $NF}' | grep -c '^/dev/sg' || true) - if [ "$changers_sg" -ge 1 ] && [ "$drives_sg" -ge 3 ]; then break; fi + # Counted with a glob rather than `ls | grep -c`: an unmatched glob stays + # literal, so the -e test is what distinguishes "no nodes" from "one node". + # The `if` rather than `&&` matters — a failing test as the last command in a + # loop body aborts the script under `set -e`. + tape_nodes=0 + for _t in /sys/class/scsi_tape/nst*; do + if [ -e "$_t" ]; then tape_nodes=$((tape_nodes + 1)); fi + done + if [ "$changers_sg" -ge 1 ] && [ "$drives_sg" -ge 3 ] && [ "$tape_nodes" -ge 3 ]; then break; fi done command -v udevadm >/dev/null && udevadm settle --timeout=30 || true @@ -250,7 +312,14 @@ printf '\nmediumx devices: %s (want >= 1)\ntape devices: %s (want >= 3)\n' \ [ "$changers" -ge 1 ] || fail "no medium changer appeared — check 'ps ax | grep vtl' and dmesg" [ "$drives" -ge 3 ] || fail "expected 3 tape devices, found $drives — check vtltape daemons" -changer_sg=$(lsscsi -g | awk '/ mediumx /{print $NF}' | head -1) +# Identify the changer the way env.sh and every other rig script does: by unit +# serial, never "first mediumx". Two reasons this is the right call HERE as well +# as there. It keeps a real i3 cabled to the same host from being selected +# (Phase 3 of the bring-up plan), and — the reason it moved into this script — +# it makes setup.sh fail on exactly the condition that made env.sh refuse. Run +# 35688121678 printed "Rig is up" and then died two steps later because the +# serial lookup returned nothing. +changer_sg=$(rig_changer) case "$changer_sg" in /dev/sg*) ;; *) fail "the changer has no /dev/sg node yet (lsscsi shows '$changer_sg'). @@ -265,12 +334,32 @@ mtx -f "$changer_sg" status # the "wrote to the wrong drive" trap from Phase 2 of the bring-up plan, so # print the correlation every time rather than letting anyone assume it. log "Drive correlation (SCSI address -> devices -> serial)" -printf '%-14s %-10s %-10s %s\n' "SCSI_ADDR" "BLOCK" "SG" "SERIAL" +printf '%-14s %-10s %-10s %-10s %s\n' "SCSI_ADDR" "BLOCK" "SG" "NST" "SERIAL" lsscsi -g | awk '/ tape /{print $1, $(NF-1), $NF}' | tr -d '[]' | while read -r addr blk sg; do - serial=$(sg_inq "$sg" 2>/dev/null | awk -F': *' '/Unit serial number/{print $2}') - printf '%-14s %-10s %-10s %s\n' "$addr" "$blk" "$sg" "${serial:-}" + serial=$(rig_serial_of "$sg") + nst=$(rig_nst_for_sg "$sg" || true) + printf '%-14s %-10s %-10s %-10s %s\n' \ + "$addr" "$blk" "$sg" "${nst:-}" "${serial:-}" done +# The nst nodes ARE the rig's usable surface: OPENBLADE_DRIVE_DEVICES is a list +# of them, so a rig without them cannot run a single drive-touching test. Assert +# it here, using the same lookup env.sh uses, so "the rig cannot work" fails at +# bring-up with a diagnosis instead of surfacing as an unbound variable in the +# CI step that runs pytest. +missing_nst="" +while read -r sg; do + [ -n "$sg" ] || continue + rig_nst_for_sg "$sg" >/dev/null || missing_nst="${missing_nst} $sg" +done <&2 + fail "no /dev/nst node for:${missing_nst} (see the diagnosis above). + The rig's drives are unusable in this state; refusing to report success." +fi + # ---------------------------------------------------------------- scratch media # The archive/restore jobs mount scratch media without formatting it first, so # blank cartridges fail the mount. Supply formatted scratch media here. diff --git a/scripts/security_agents/check_emulators.py b/scripts/security_agents/check_emulators.py index 879cb66..9a72865 100755 --- a/scripts/security_agents/check_emulators.py +++ b/scripts/security_agents/check_emulators.py @@ -22,10 +22,10 @@ try: h = requests.get(url + "/health", timeout=5) print(url + "/health", h.status_code) - except Exception as e: + except Exception as e: # noqa: BLE001 - operator probe script: an unreachable emulator is the result, printed, not a crash print("emulator", url, "unreachable", e) try: inv = requests.get(url + "/api/aml/inventory", timeout=5) print(url + "/api/aml/inventory", inv.status_code) - except Exception as e: + except Exception as e: # noqa: BLE001 - operator probe script: an unreachable endpoint is the result, printed, not a crash print("inventory failed for", url, e) diff --git a/tests/__pycache__/__init__.cpython-310.pyc b/tests/__pycache__/__init__.cpython-310.pyc deleted file mode 100644 index 2fefeeb..0000000 Binary files a/tests/__pycache__/__init__.cpython-310.pyc and /dev/null differ diff --git a/tests/__pycache__/conftest.cpython-310-pytest-8.3.2.pyc b/tests/__pycache__/conftest.cpython-310-pytest-8.3.2.pyc deleted file mode 100644 index ef8595c..0000000 Binary files a/tests/__pycache__/conftest.cpython-310-pytest-8.3.2.pyc and /dev/null differ diff --git a/tests/e2e/__pycache__/__init__.cpython-310.pyc b/tests/e2e/__pycache__/__init__.cpython-310.pyc deleted file mode 100644 index 2079408..0000000 Binary files a/tests/e2e/__pycache__/__init__.cpython-310.pyc and /dev/null differ diff --git a/tests/e2e/__pycache__/test_mock_archive_restore.cpython-310-pytest-8.3.2.pyc b/tests/e2e/__pycache__/test_mock_archive_restore.cpython-310-pytest-8.3.2.pyc deleted file mode 100644 index 2f7d6ef..0000000 Binary files a/tests/e2e/__pycache__/test_mock_archive_restore.cpython-310-pytest-8.3.2.pyc and /dev/null differ diff --git a/tests/fault/__pycache__/__init__.cpython-310.pyc b/tests/fault/__pycache__/__init__.cpython-310.pyc deleted file mode 100644 index 9efa5b1..0000000 Binary files a/tests/fault/__pycache__/__init__.cpython-310.pyc and /dev/null differ diff --git a/tests/fault/__pycache__/test_fault_injection.cpython-310-pytest-8.3.2.pyc b/tests/fault/__pycache__/test_fault_injection.cpython-310-pytest-8.3.2.pyc deleted file mode 100644 index d3f4ce2..0000000 Binary files a/tests/fault/__pycache__/test_fault_injection.cpython-310-pytest-8.3.2.pyc and /dev/null differ diff --git a/tests/hardware/conftest.py b/tests/hardware/conftest.py index 1616709..05df03c 100644 --- a/tests/hardware/conftest.py +++ b/tests/hardware/conftest.py @@ -81,7 +81,14 @@ def real_library_backend(real_hardware_guard): return backend_cls(changer_device=changer) except TypeError: return backend_cls(changer) - except Exception: + # Deliberately broad, and NOT narrowed to (ImportError, AttributeError): the + # contract of this fixture is "give me some real changer backend", and the + # construction of RealLibraryBackend can fail on a device open (OSError), a bad + # OPENBLADE_CHANGER_DEVICE string (ValueError) or a signature change + # (TypeError/AttributeError) — every one of which the mtx fallback handles. This + # only runs under the real-hardware marker, so narrowing it would change + # hardware-test setup behaviour on a rig this change cannot be tested against. + except Exception: # noqa: BLE001 - see above return MtxChangerBackend( device=changer, guard=_real_hardware_guard(), diff --git a/tests/hardware/test_ltfs_operations.py b/tests/hardware/test_ltfs_operations.py index c8eeff9..e644561 100644 --- a/tests/hardware/test_ltfs_operations.py +++ b/tests/hardware/test_ltfs_operations.py @@ -126,7 +126,7 @@ def test_ltfs_mount_formatted( finally: try: _unmount_ltfs(runner, tmp_mount_dir) - except Exception: + except Exception: # noqa: BLE001 - real-hardware teardown: a failed unmount must not mask the assertion that failed first, and the unload below still has to run pass _unload_barcode(changer_device, runner, slot_id) @@ -154,7 +154,7 @@ def test_ltfs_write_small_file( finally: try: _unmount_ltfs(runner, tmp_mount_dir) - except Exception: + except Exception: # noqa: BLE001 - real-hardware teardown: a failed unmount must not mask the assertion that failed first, and the unload below still has to run pass _unload_barcode(changer_device, runner, slot_id) @@ -184,7 +184,7 @@ def test_ltfs_read_small_file( finally: try: _unmount_ltfs(runner, tmp_mount_dir) - except Exception: + except Exception: # noqa: BLE001 - real-hardware teardown: a failed unmount must not mask the assertion that failed first, and the unload below still has to run pass _unload_barcode(changer_device, runner, slot_id) @@ -234,7 +234,7 @@ def test_ltfs_remount_persistence( finally: try: _unmount_ltfs(runner, tmp_mount_dir) - except Exception: + except Exception: # noqa: BLE001 - real-hardware teardown: a failed unmount must not mask the assertion that failed first, and the unload below still has to run pass _unload_barcode(changer_device, runner, slot_id) @@ -261,7 +261,7 @@ def test_ltfs_capacity_reporting( finally: try: _unmount_ltfs(runner, tmp_mount_dir) - except Exception: + except Exception: # noqa: BLE001 - real-hardware teardown: a failed unmount must not mask the assertion that failed first, and the unload below still has to run pass _unload_barcode(changer_device, runner, slot_id) diff --git a/tests/hardware/test_performance.py b/tests/hardware/test_performance.py index 45ea877..5076171 100644 --- a/tests/hardware/test_performance.py +++ b/tests/hardware/test_performance.py @@ -128,7 +128,7 @@ def test_single_drive_write_throughput( finally: try: _unmount_ltfs(runner, tmp_mount_dir) - except Exception: + except Exception: # noqa: BLE001 - real-hardware teardown: a failed unmount must not mask the assertion that failed first, and the unload below still has to run pass _unload_barcode(changer_device, runner, slot_id) @@ -162,7 +162,7 @@ def test_single_drive_read_throughput( finally: try: _unmount_ltfs(runner, tmp_mount_dir) - except Exception: + except Exception: # noqa: BLE001 - real-hardware teardown: a failed unmount must not mask the assertion that failed first, and the unload below still has to run pass _unload_barcode(changer_device, runner, slot_id) diff --git a/tests/integration/__pycache__/__init__.cpython-310.pyc b/tests/integration/__pycache__/__init__.cpython-310.pyc deleted file mode 100644 index 259eb5c..0000000 Binary files a/tests/integration/__pycache__/__init__.cpython-310.pyc and /dev/null differ diff --git a/tests/integration/__pycache__/test_catalog.cpython-310-pytest-8.3.2.pyc b/tests/integration/__pycache__/test_catalog.cpython-310-pytest-8.3.2.pyc deleted file mode 100644 index b078c70..0000000 Binary files a/tests/integration/__pycache__/test_catalog.cpython-310-pytest-8.3.2.pyc and /dev/null differ diff --git a/tests/integration/__pycache__/test_jobs.cpython-310-pytest-8.3.2.pyc b/tests/integration/__pycache__/test_jobs.cpython-310-pytest-8.3.2.pyc deleted file mode 100644 index 8be30c1..0000000 Binary files a/tests/integration/__pycache__/test_jobs.cpython-310-pytest-8.3.2.pyc and /dev/null differ diff --git a/tests/integration/__pycache__/test_simulator_backend.cpython-310-pytest-8.3.2.pyc b/tests/integration/__pycache__/test_simulator_backend.cpython-310-pytest-8.3.2.pyc deleted file mode 100644 index 2600108..0000000 Binary files a/tests/integration/__pycache__/test_simulator_backend.cpython-310-pytest-8.3.2.pyc and /dev/null differ diff --git a/tests/integration/__pycache__/test_simulator_stress.cpython-310-pytest-8.3.2.pyc b/tests/integration/__pycache__/test_simulator_stress.cpython-310-pytest-8.3.2.pyc deleted file mode 100644 index e32925a..0000000 Binary files a/tests/integration/__pycache__/test_simulator_stress.cpython-310-pytest-8.3.2.pyc and /dev/null differ diff --git a/tests/integration/test_drive_health_api.py b/tests/integration/test_drive_health_api.py new file mode 100644 index 0000000..b51962b --- /dev/null +++ b/tests/integration/test_drive_health_api.py @@ -0,0 +1,276 @@ +"""``GET /hardware/drive-health`` -- the HTTP half of ``openblade hardware drive-health``. + +Read-only and guarded. Two properties matter more than the payload shape: + +* with the simulator backend (the default) the route must refuse rather than go + looking for a device, and +* a drive without the TapeAlert log page must come back as a 200 saying so, not + as an error -- that is a property of the drive. + +The "real hardware" cases run with ``hardware_dry_run=True``, so the guard is +satisfied and ``SafeRunner`` returns the sample ``sg_inq``/``sg_logs`` output +captured from a real rig instead of executing anything. +""" + +from __future__ import annotations + +from dataclasses import replace +from pathlib import Path + +import pytest +from fastapi.testclient import TestClient + +from openblade.api.main import app +from openblade.bootstrap import create_context, get_context, reset_context +from openblade.config import BackendMode, OpenBladeConfig +from openblade.hardware import sg, tapealert +from openblade.hardware.runner import CommandResult, SafeRunner + + +@pytest.fixture() +def client(tmp_path: Path) -> TestClient: + context = create_context(OpenBladeConfig(db_url=f"sqlite:///{tmp_path / 'health.db'}")) + reset_context(context) + return TestClient(app) + + +@pytest.fixture() +def hardware_enabled() -> None: + """Claim real hardware while keeping the simulator backends. + + The route only consults ``context.config``; swapping the config alone + exercises the guarded path without building real backends in a test process. + """ + context = get_context() + reset_context( + replace( + context, + config=replace( + context.config, + backend=BackendMode.REAL, + real_hardware_enabled=True, + hardware_dry_run=True, + ), + ) + ) + + +def test_simulator_backend_refuses_with_the_curated_reason(client: TestClient) -> None: + response = client.get("/hardware/drive-health") + assert response.status_code == 503 + detail = response.json()["detail"] + # The refusal names both variables, because "unavailable" alone is unactionable. + assert "OPENBLADE_BACKEND=real" in detail + assert "OPENBLADE_REAL_HARDWARE_ENABLED=true" in detail + + +def test_one_named_device_reports_inquiry_and_tapealert( + client: TestClient, hardware_enabled: None +) -> None: + response = client.get("/hardware/drive-health", params={"device": "/dev/sg1"}) + assert response.status_code == 200 + drives = response.json()["drives"] + assert len(drives) == 1 + drive = drives[0] + assert drive["device"] == "/dev/sg1" + assert drive["inquiry"]["vendor"] + assert drive["inquiry"]["product"] + assert drive["tapeAlertSupported"] is True + # The clean sample sets no flags: every flag is read, none is active. + assert drive["flagsRead"] > 0 + assert drive["activeFlags"] == [] + assert drive["worstSeverity"] is None + assert drive["tapeAlertReason"] is None + + +def test_every_discovered_drive_is_reported_when_no_device_is_named( + client: TestClient, hardware_enabled: None +) -> None: + response = client.get("/hardware/drive-health") + assert response.status_code == 200 + drives = response.json()["drives"] + assert len(drives) >= 1 + assert len({drive["device"] for drive in drives}) == len(drives) + + +def test_active_flags_carry_their_severity( + client: TestClient, hardware_enabled: None, monkeypatch: pytest.MonkeyPatch +) -> None: + report = tapealert.parse_sg_logs_tapealert( + tapealert.SAMPLE_SG_LOGS_TAPEALERT_CLEAN, device="/dev/sg1" + ) + flags = tuple( + replace(flag, value=flag.number in {4, 3}) if flag.number in {4, 3} else flag + for flag in report.flags + ) + monkeypatch.setattr( + "openblade.api.routes_drive_health.read_tape_alerts", + lambda device, runner, guard: replace(report, flags=flags), + ) + + response = client.get("/hardware/drive-health", params={"device": "/dev/sg1"}) + assert response.status_code == 200 + drive = response.json()["drives"][0] + severities = {flag["number"]: flag["severity"] for flag in drive["activeFlags"]} + assert severities == {4: "critical", 3: "warning"} + # Worst-first, so a UI badge cannot report a critical drive as merely warned. + assert drive["worstSeverity"] == "critical" + + +def test_a_drive_without_the_log_page_is_a_200_with_a_reason( + client: TestClient, hardware_enabled: None, monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.setattr( + "openblade.api.routes_drive_health.read_tape_alerts", + lambda device, runner, guard: tapealert.TapeAlertReport( + device=device, + supported=False, + flags=(), + reason="no TapeAlert page in sg_logs output", + ), + ) + + response = client.get("/hardware/drive-health", params={"device": "/dev/sg1"}) + assert response.status_code == 200 + drive = response.json()["drives"][0] + assert drive["tapeAlertSupported"] is False + assert drive["tapeAlertReason"] == "no TapeAlert page in sg_logs output" + assert drive["activeFlags"] == [] + + +def test_the_route_is_read_only(client: TestClient, hardware_enabled: None) -> None: + # Not a formality: drive health is the one surface that talks SCSI to a drive + # a human is worried about, and a write here would be the worst possible bug. + for method in (client.post, client.put, client.delete, client.patch): + assert method("/hardware/drive-health").status_code == 405 + + +def test_it_issues_only_the_two_read_commands( + client: TestClient, hardware_enabled: None, monkeypatch: pytest.MonkeyPatch +) -> None: + """The 405s above are free from FastAPI; this asserts the actual SCSI traffic. + + Captured at the SafeRunner boundary with dry_run OFF, so every argv the route + would really execute is recorded. A LOG SELECT, a rewind, an `mt` command or a + format would all show up here. + """ + issued: list[list[str]] = [] + + def record(self: SafeRunner, args: list[str], **kwargs: object) -> CommandResult: + issued.append(list(args)) + sample = ( + sg.SAMPLE_SG_INQ + if args[0] == "sg_inq" + else tapealert.SAMPLE_SG_LOGS_TAPEALERT_CLEAN + if args[0] == "sg_logs" + else "" + ) + return CommandResult( + args=list(args), returncode=0, stdout=sample, stderr="", elapsed_seconds=0.0 + ) + + context = get_context() + reset_context(replace(context, config=replace(context.config, hardware_dry_run=False))) + monkeypatch.setattr(SafeRunner, "run", record) + + response = client.get("/hardware/drive-health", params={"device": "/dev/sg1"}) + assert response.status_code == 200, response.text + assert issued == [["sg_inq", "/dev/sg1"], ["sg_logs", "-p", "0x2e", "/dev/sg1"]] + + +def test_a_tool_failure_answers_502_without_the_argv_or_stderr( + client: TestClient, hardware_enabled: None, monkeypatch: pytest.MonkeyPatch +) -> None: + def explode(self: SafeRunner, args: list[str], **kwargs: object) -> CommandResult: + return CommandResult( + args=list(args), + returncode=2, + stdout="", + stderr="sg_inq: error opening file: /dev/sg9 (dsn=secret-token)", + elapsed_seconds=0.0, + ) + + context = get_context() + reset_context(replace(context, config=replace(context.config, hardware_dry_run=False))) + monkeypatch.setattr(SafeRunner, "run", explode) + + response = client.get("/hardware/drive-health", params={"device": "/dev/sg9"}) + # CommandError is not an OpenBladeError, so without the route's own handler + # this would escape as a bare 500 carrying the full argv and the tool's stderr. + assert response.status_code == 502 + detail = response.json()["detail"] + assert "secret-token" not in detail + assert "sg_inq:" not in detail + assert "sg3_utils" in detail + + +def test_the_tapealert_reason_never_carries_the_tools_stderr( + client: TestClient, hardware_enabled: None, monkeypatch: pytest.MonkeyPatch +) -> None: + # Curation happens AT THE SOURCE now: read_tape_alerts puts tool text in + # reason_detail (log/CLI only) and keeps reason curated. The route must + # serialize reason verbatim and reason_detail never. + monkeypatch.setattr( + "openblade.api.routes_drive_health.read_tape_alerts", + lambda device, runner, guard: tapealert.TapeAlertReport( + device=device, + supported=False, + flags=(), + reason="no TapeAlert page in sg_logs output (sg_logs exited 5; see server logs)", + reason_detail="log_sense: field in cdb illegal, /dev/sg1 dsn=secret", + ), + ) + + response = client.get("/hardware/drive-health", params={"device": "/dev/sg1"}) + assert response.status_code == 200 + body = response.text + assert "secret" not in body + assert "field in cdb illegal" not in body + reason = response.json()["drives"][0]["tapeAlertReason"] + assert "sg_logs exited 5" in reason + + +def test_paren_heavy_tool_stderr_never_reaches_the_wire( + client: TestClient, hardware_enabled: None, monkeypatch: pytest.MonkeyPatch +) -> None: + """Reviewer-repro'd leak: the old regex scrub over the COMBINED reason + string stopped at the first ')' inside real sg3_utils stderr, so + 'sense_key=0x5 (Invalid field in cdb) opening /dev/sg9 dsn=secret)' + leaked a device path and a DSN in a 200. Curation now happens at the + source: reason never contains tool text; reason_detail stays server-side. + This exercises read_tape_alerts itself against the hostile stderr.""" + hostile = "sense_key=0x5 (Invalid field in cdb) opening /dev/sg9 dsn=secret-token)" + + from openblade.hardware.sg import SAMPLE_SG_INQ_MODERN + + def hostile_run( + self: SafeRunner, + args: list[str], + timeout: int | None = None, + redact_args: list[int] | None = None, + ) -> CommandResult: + if args[0] == "sg_logs": + return CommandResult( + args=args, returncode=5, stdout="", stderr=hostile, elapsed_seconds=0.0 + ) + return CommandResult( + args=args, returncode=0, stdout=SAMPLE_SG_INQ_MODERN, stderr="", elapsed_seconds=0.0 + ) + + # The hardware_enabled fixture keeps hardware_dry_run=True, and dry-run + # short-circuits BEFORE runner.run — so patching run alone exercises + # nothing. Hand the route a non-dry runner whose run IS the hostile stub. + class HostileRunner(SafeRunner): + def __init__(self, dry_run: bool = False) -> None: + super().__init__(dry_run=False) + + monkeypatch.setattr(HostileRunner, "run", hostile_run) + monkeypatch.setattr("openblade.api.routes_drive_health.SafeRunner", HostileRunner) + response = client.get("/hardware/drive-health", params={"device": "/dev/sg1"}) + assert response.status_code == 200 + body = response.text + assert "dsn=secret-token" not in body + assert "sense_key" not in body + reason = response.json()["drives"][0]["tapeAlertReason"] + assert "see server logs" in reason + assert "sg_logs exited 5" in reason diff --git a/tests/integration/test_mailslot_api.py b/tests/integration/test_mailslot_api.py new file mode 100644 index 0000000..f5085bc --- /dev/null +++ b/tests/integration/test_mailslot_api.py @@ -0,0 +1,259 @@ +"""``/mailslot/*`` -- the HTTP half of ``openblade mailslot``. + +The interesting assertions are the refusal ones: the API must not let archived +data walk out of the library on an unforced request, and it must say what would +have left. See ``openblade/catalog/export_policy.py``. +""" + +from __future__ import annotations + +from pathlib import Path + +import pytest +from fastapi.testclient import TestClient + +from openblade.api.main import app +from openblade.bootstrap import create_context, get_context, reset_context +from openblade.config import OpenBladeConfig + +SERVICE_TOKEN = "openblade-controller-dev-token-do-not-expose" + + +@pytest.fixture() +def client(tmp_path: Path) -> TestClient: + context = create_context(OpenBladeConfig(db_url=f"sqlite:///{tmp_path / 'mailslot.db'}")) + reset_context(context) + return TestClient(app) + + +def _data_barcodes(limit: int) -> list[str]: + context = get_context() + return [ + str(slot.barcode) + for slot in context.library.inventory().slots + if slot.barcode is not None and not str(slot.barcode).startswith("CLN") + ][:limit] + + +def _admin_auth_headers(client: TestClient) -> dict[str, str]: + response = client.post("/aml/users/login", json={"name": "admin", "password": "password"}) + assert response.status_code == 200 + session_id = response.cookies.get("sessionID") + assert session_id is not None + return {"Cookie": f"sessionID={session_id}"} + + +def _format_and_assign(client: TestClient, volume_group: str, barcode: str) -> None: + auth_headers = _admin_auth_headers(client) + assert client.post("/volume-groups/", json={"name": volume_group}).status_code == 201 + assert ( + client.post(f"/volume-groups/{volume_group}/assign", json={"barcode": barcode}).status_code + == 200 + ) + dry_run = client.post(f"/cartridges/{barcode}/format/dry-run", headers=auth_headers) + assert dry_run.status_code == 200 + assert ( + client.post( + "/cartridges/format/confirm", + json={"barcode": barcode, "token": dry_run.json()["token"]}, + headers={**auth_headers, "X-Openblade-Service-Token": SERVICE_TOKEN}, + ).status_code + == 200 + ) + + +def _archive_one_file( + client: TestClient, source_dir: Path, volume_group: str, file_name: str +) -> None: + source_dir.mkdir() + (source_dir / file_name).write_text("mailslot export policy fixture") + response = client.post( + "/archive/", + json={"source_path": str(source_dir), "volume_group": volume_group}, + ) + assert response.status_code == 202 + + +def test_slots_lists_every_import_export_element(client: TestClient) -> None: + response = client.get("/mailslot/slots") + assert response.status_code == 200 + payload = response.json() + assert payload["supported"] is True + # The i3 profile declares two I/E elements; the listing reports all of them, + # empty ones included, because "empty" is the answer to "can I export?". + assert payload["slotCount"] == len(payload["importExportSlots"]) == 2 + assert payload["occupiedCount"] == 0 + assert all( + slot["occupied"] is False and slot["barcode"] is None + for slot in payload["importExportSlots"] + ) + + +def test_export_then_import_round_trips_a_scratch_cartridge(client: TestClient) -> None: + barcode = _data_barcodes(limit=1)[0] + + exported = client.post("/mailslot/export", json={"barcode": barcode}) + assert exported.status_code == 200, exported.text + export_payload = exported.json() + assert export_payload["barcode"] == barcode + assert export_payload["source"] == "storage_slot" + assert export_payload["destination"] == "import_export_slot" + # Nobody named a destination element, so the service picked one and says so. + assert export_payload["destinationSlotChosen"] is True + ie_slot = export_payload["destinationSlot"] + + occupied = client.get("/mailslot/slots").json() + assert occupied["occupiedCount"] == 1 + assert [slot["barcode"] for slot in occupied["importExportSlots"] if slot["occupied"]] == [ + barcode + ] + + imported = client.post("/mailslot/import", json={"ie_slot": ie_slot}) + assert imported.status_code == 200, imported.text + import_payload = imported.json() + assert import_payload["barcode"] == barcode + assert import_payload["source"] == "import_export_slot" + assert import_payload["sourceSlot"] == ie_slot + assert import_payload["destinationSlotChosen"] is True + assert client.get("/mailslot/slots").json()["occupiedCount"] == 0 + + +def test_import_honours_an_explicit_target_slot(client: TestClient) -> None: + barcode = _data_barcodes(limit=1)[0] + ie_slot = client.post("/mailslot/export", json={"barcode": barcode}).json()["destinationSlot"] + + context = get_context() + empty_slot = next( + slot.slot_id for slot in context.library.inventory().slots if slot.barcode is None + ) + imported = client.post("/mailslot/import", json={"ie_slot": ie_slot, "to_slot": empty_slot}) + assert imported.status_code == 200, imported.text + assert imported.json()["destinationSlot"] == empty_slot + assert imported.json()["destinationSlotChosen"] is False + + +def test_import_from_an_empty_element_is_a_409_naming_the_element(client: TestClient) -> None: + response = client.post("/mailslot/import", json={"ie_slot": 51}) + assert response.status_code == 409 + assert "51" in response.json()["detail"] + + +def test_import_from_an_unknown_element_says_which_exist(client: TestClient) -> None: + response = client.post("/mailslot/import", json={"ie_slot": 9999}) + assert response.status_code == 409 + assert "9999" in response.json()["detail"] + assert "does not exist" in response.json()["detail"] + + +def test_export_preview_reports_what_would_leave(client: TestClient, tmp_path: Path) -> None: + barcode = _data_barcodes(limit=1)[0] + _format_and_assign(client, "photos", barcode) + _archive_one_file(client, tmp_path / "photos-source", "photos", "a.txt") + + response = client.get(f"/mailslot/export-preview/{barcode}") + assert response.status_code == 200 + payload = response.json() + assert payload["barcode"] == barcode + assert payload["carriesData"] is True + assert payload["archivedFilesOnCartridge"] >= 1 + assert payload["bytesOnCartridge"] > 0 + assert payload["volumeGroup"] == "photos" + assert any(path.endswith("a.txt") for path in payload["samplePaths"]) + # A preview moves nothing: the cartridge is still in storage afterwards. + assert client.get("/mailslot/slots").json()["occupiedCount"] == 0 + + +def test_export_preview_of_a_scratch_cartridge_carries_no_data(client: TestClient) -> None: + barcode = _data_barcodes(limit=1)[0] + payload = client.get(f"/mailslot/export-preview/{barcode}").json() + assert payload["carriesData"] is False + assert payload["archivedFilesOnCartridge"] == 0 + assert payload["samplePaths"] == [] + + +def test_export_of_a_cartridge_carrying_data_is_refused_and_names_it( + client: TestClient, tmp_path: Path +) -> None: + barcode = _data_barcodes(limit=1)[0] + _format_and_assign(client, "photos", barcode) + _archive_one_file(client, tmp_path / "photos-source", "photos", "a.txt") + + response = client.post("/mailslot/export", json={"barcode": barcode}) + assert response.status_code == 409 + detail = response.json()["detail"] + assert barcode in detail + assert "still carries archived data" in detail + assert "photos" in detail + assert "a.txt" in detail + # Refused means nothing moved: the I/E station is still empty. + assert client.get("/mailslot/slots").json()["occupiedCount"] == 0 + + +def test_typed_confirmation_exports_the_same_cartridge(client: TestClient, tmp_path: Path) -> None: + barcode = _data_barcodes(limit=1)[0] + _format_and_assign(client, "photos", barcode) + _archive_one_file(client, tmp_path / "photos-source", "photos", "a.txt") + assert client.post("/mailslot/export", json={"barcode": barcode}).status_code == 409 + + forced = client.post("/mailslot/export", json={"barcode": barcode, "confirmBarcode": barcode}) + assert forced.status_code == 200, forced.text + payload = forced.json() + assert payload["barcode"] == barcode + # The response carries the assessment, so an audit of a forced export still + # records what left the library. + assert payload["exported"]["carriesData"] is True + assert payload["exported"]["archivedFilesOnCartridge"] >= 1 + assert client.get("/mailslot/slots").json()["occupiedCount"] == 1 + + +def test_export_of_an_unknown_barcode_is_a_404(client: TestClient) -> None: + response = client.post("/mailslot/export", json={"barcode": "NOSUCH1L8"}) + assert response.status_code == 404 + assert "NOSUCH1L8" in response.json()["detail"] + + +def test_export_into_an_occupied_element_is_refused(client: TestClient) -> None: + first, second = _data_barcodes(limit=2) + occupied_slot = client.post("/mailslot/export", json={"barcode": first}).json()[ + "destinationSlot" + ] + + response = client.post("/mailslot/export", json={"barcode": second, "ie_slot": occupied_slot}) + assert response.status_code == 409 + assert first in response.json()["detail"] + + +def test_export_refuses_once_every_element_is_full(client: TestClient) -> None: + barcodes = _data_barcodes(limit=3) + slot_count = client.get("/mailslot/slots").json()["slotCount"] + for barcode in barcodes[:slot_count]: + assert client.post("/mailslot/export", json={"barcode": barcode}).status_code == 200 + + response = client.post("/mailslot/export", json={"barcode": barcodes[slot_count]}) + assert response.status_code == 409 + assert "empty the mailslot" in response.json()["detail"] + + +def test_export_preview_of_an_unknown_barcode_is_a_404_not_a_cheerful_zero( + client: TestClient, +) -> None: + # `assess_export` answers "nothing on it" for a barcode that does not exist, + # which is true and useless: next to an Export button a typo would read as + # "safe to export". + response = client.get("/mailslot/export-preview/NOSUCH1L8") + assert response.status_code == 404 + assert "NOSUCH1L8" in response.json()["detail"] + + +def test_a_mistyped_confirm_barcode_is_refused(client: TestClient, tmp_path: Path) -> None: + """Typed confirmation means EXACT match — a near-miss must not export. + (This is the API-side twin of the CLI's --confirm-barcode guard.)""" + barcode = _data_barcodes(limit=1)[0] + _format_and_assign(client, "photos", barcode) + _archive_one_file(client, tmp_path / "photos-source", "photos", "a.txt") + wrong = client.post( + "/mailslot/export", json={"barcode": barcode, "confirmBarcode": barcode.lower()} + ) + assert wrong.status_code == 409, wrong.text + # Nothing moved on a near-miss. + assert client.get("/mailslot/slots").json()["occupiedCount"] == 0 diff --git a/tests/integration/test_scalar_http_drive_device_emulator.py b/tests/integration/test_scalar_http_drive_device_emulator.py index d3bdbae..a2973a2 100644 --- a/tests/integration/test_scalar_http_drive_device_emulator.py +++ b/tests/integration/test_scalar_http_drive_device_emulator.py @@ -76,7 +76,8 @@ def emulator_url(tmp_path_factory: pytest.TempPathFactory) -> Generator[str, Non "OPENBLADE_DB_URL": f"sqlite:///{db_path}", "PYTHONPATH": str(REPO_ROOT), } - process = subprocess.Popen( # noqa: S603 - fixed argv, no shell + # Fixed argv list, shell=False: nothing here is caller-controlled. + process = subprocess.Popen( [ sys.executable, "-m", diff --git a/tests/integration/test_tree_restore_api.py b/tests/integration/test_tree_restore_api.py new file mode 100644 index 0000000..108df1f --- /dev/null +++ b/tests/integration/test_tree_restore_api.py @@ -0,0 +1,252 @@ +"""``POST /restore/tree`` -- the HTTP half of ``openblade restore tree``. + +The payload is ``TreeRestoreResult.to_dict()``, so the CLI and the API cannot +drift. What is asserted here is the behaviour a UI depends on: a dry run moves +nothing, a real run writes the tree under the destination preserving the source +layout, and a short result names the files it did not restore. +""" + +from __future__ import annotations + +import asyncio +from pathlib import Path + +import pytest +from fastapi.testclient import TestClient + +from openblade.api.main import app +from openblade.bootstrap import create_context, get_context, reset_context +from openblade.config import OpenBladeConfig +from openblade.domain.errors import CartridgeOfflineError +from openblade.jobs import tree_restore as tree_restore_module + +SERVICE_TOKEN = "openblade-controller-dev-token-do-not-expose" + + +@pytest.fixture() +def client(tmp_path: Path) -> TestClient: + context = create_context(OpenBladeConfig(db_url=f"sqlite:///{tmp_path / 'tree.db'}")) + reset_context(context) + return TestClient(app) + + +def _data_barcodes(limit: int) -> list[str]: + context = get_context() + return [ + str(slot.barcode) + for slot in context.library.inventory().slots + if slot.barcode is not None and not str(slot.barcode).startswith("CLN") + ][:limit] + + +def _admin_auth_headers(client: TestClient) -> dict[str, str]: + response = client.post("/aml/users/login", json={"name": "admin", "password": "password"}) + assert response.status_code == 200 + session_id = response.cookies.get("sessionID") + assert session_id is not None + return {"Cookie": f"sessionID={session_id}"} + + +def _format_and_assign(client: TestClient, volume_group: str, barcode: str) -> None: + auth_headers = _admin_auth_headers(client) + assert client.post("/volume-groups/", json={"name": volume_group}).status_code == 201 + assert ( + client.post(f"/volume-groups/{volume_group}/assign", json={"barcode": barcode}).status_code + == 200 + ) + dry_run = client.post(f"/cartridges/{barcode}/format/dry-run", headers=auth_headers) + assert dry_run.status_code == 200 + assert ( + client.post( + "/cartridges/format/confirm", + json={"barcode": barcode, "token": dry_run.json()["token"]}, + headers={**auth_headers, "X-Openblade-Service-Token": SERVICE_TOKEN}, + ).status_code + == 200 + ) + + +def _archive_tree(client: TestClient, source_dir: Path, volume_group: str) -> None: + (source_dir / "nested").mkdir(parents=True) + (source_dir / "top.txt").write_text("top level file") + (source_dir / "nested" / "deep.txt").write_text("nested file") + response = client.post( + "/archive/", + json={"source_path": str(source_dir), "volume_group": volume_group}, + ) + assert response.status_code == 202 + + +@pytest.fixture() +def archived(client: TestClient, tmp_path: Path) -> str: + barcode = _data_barcodes(limit=1)[0] + _format_and_assign(client, "photos", barcode) + _archive_tree(client, tmp_path / "photos-source", "photos") + return barcode + + +def test_dry_run_plans_without_writing_anything( + client: TestClient, tmp_path: Path, archived: str +) -> None: + destination = tmp_path / "planned" + response = client.post( + "/restore/tree", + json={"catalog_prefix": "/photos", "dest_dir": str(destination), "dry_run": True}, + ) + assert response.status_code == 200, response.text + payload = response.json() + assert payload["dryRun"] is True + assert payload["catalogPrefix"] == "/photos" + assert payload["filesRestored"] == 2 + assert payload["bytesRestored"] > 0 + assert payload["tapesUsed"] == [archived] + assert payload["perTapeCounts"][archived] == 2 + assert payload["failures"] == [] + assert payload["status"] == "completed" + # A preview that leaves files behind is the whole point of a preview. + assert not destination.exists() + + +def test_run_restores_the_tree_preserving_the_source_layout( + client: TestClient, tmp_path: Path, archived: str +) -> None: + destination = tmp_path / "restored" + response = client.post( + "/restore/tree", + json={"catalog_prefix": "/photos", "dest_dir": str(destination), "dry_run": False}, + ) + assert response.status_code == 200, response.text + payload = response.json() + assert payload["dryRun"] is False + assert payload["filesRestored"] == 2 + assert payload["status"] == "completed" + assert (destination / "top.txt").read_text() == "top level file" + assert (destination / "nested" / "deep.txt").read_text() == "nested file" + + +def test_the_job_row_records_the_tree_restore( + client: TestClient, tmp_path: Path, archived: str +) -> None: + payload = client.post( + "/restore/tree", + json={"catalog_prefix": "/photos", "dest_dir": str(tmp_path / "out"), "dry_run": False}, + ).json() + job = client.get(f"/jobs/{payload['jobId']}") + assert job.status_code == 200 + assert job.json()["job_type"] == "restore" + + +def test_a_prefix_with_nothing_archived_is_an_honest_empty_result( + client: TestClient, tmp_path: Path +) -> None: + response = client.post( + "/restore/tree", + json={"catalog_prefix": "/nothing-here", "dest_dir": str(tmp_path / "out")}, + ) + assert response.status_code == 200 + payload = response.json() + assert payload["filesRestored"] == 0 + assert payload["filesSkipped"] == 0 + assert payload["tapesUsed"] == [] + # dry_run defaults to False: an absent flag must not silently plan-only. + assert payload["dryRun"] is False + + +def test_a_blank_prefix_or_destination_is_rejected(client: TestClient, tmp_path: Path) -> None: + assert ( + client.post( + "/restore/tree", json={"catalog_prefix": "", "dest_dir": str(tmp_path / "out")} + ).status_code + == 422 + ) + assert ( + client.post("/restore/tree", json={"catalog_prefix": "/photos", "dest_dir": ""}).status_code + == 422 + ) + + +def test_a_failure_answers_with_curated_text_and_fails_the_job( + client: TestClient, tmp_path: Path, archived: str, monkeypatch: pytest.MonkeyPatch +) -> None: + class Boom(RuntimeError): + """Stands in for a tool failure carrying argv and device paths.""" + + def explode(*_args: object, **_kwargs: object) -> None: + raise Boom("mkltfs /dev/sg3 failed: password=hunter2") + + monkeypatch.setattr("openblade.api.routes_restore.run_tree_restore", explode) + response = client.post( + "/restore/tree", + json={"catalog_prefix": "/photos", "dest_dir": str(tmp_path / "out")}, + ) + assert response.status_code == 500 + detail = response.json()["detail"] + # Untyped failures never reach the wire verbatim (safe_job_error). + assert "hunter2" not in detail + assert "/dev/sg3" not in detail + assert "Boom" in detail + + +def test_the_restore_does_not_run_on_the_event_loop( + client: TestClient, tmp_path: Path, archived: str, monkeypatch: pytest.MonkeyPatch +) -> None: + """A tree restore is minutes of blocking work; the loop must stay free. + + Asserted by capturing the thread it runs on: on the loop thread every other + request in the process -- `/health`, which the container health check gives + 5s, and the AML parity surface -- waits for the whole restore. + """ + seen: dict[str, object] = {} + real = tree_restore_module.run_tree_restore + + def capture(*args: object, **kwargs: object) -> object: + # `get_running_loop()` is thread-local: it succeeds only when called from + # the thread the loop runs in. NOT `threading.main_thread()` -- the + # TestClient drives the app from its own portal thread, so a main-thread + # check passes even when the work is sitting on the loop (verified: that + # version of this test survived reverting the fix). + try: + asyncio.get_running_loop() + seen["on_event_loop"] = True + except RuntimeError: + seen["on_event_loop"] = False + return real(*args, **kwargs) + + monkeypatch.setattr("openblade.api.routes_restore.run_tree_restore", capture) + response = client.post( + "/restore/tree", + json={"catalog_prefix": "/photos", "dest_dir": str(tmp_path / "off-loop")}, + ) + assert response.status_code == 200, response.text + assert seen["on_event_loop"] is False + + +def test_a_typed_domain_error_keeps_its_own_status( + client: TestClient, tmp_path: Path, archived: str, monkeypatch: pytest.MonkeyPatch +) -> None: + def offline(*_args: object, **_kwargs: object) -> None: + raise CartridgeOfflineError("Cartridge PHO001L8 is exported and cannot be read") + + monkeypatch.setattr("openblade.api.routes_restore.run_tree_restore", offline) + response = client.post( + "/restore/tree", + json={"catalog_prefix": "/photos", "dest_dir": str(tmp_path / "out")}, + ) + # 409 from the app's OpenBladeError handler, not a flattened 500: the message + # is operator-written and the status is part of its meaning. + assert response.status_code == 409 + assert "exported" in response.json()["detail"] + + +def test_tree_restore_serializes_with_archive_media_work(monkeypatch) -> None: + """Off-loop execution made concurrency real; the worker must hold the + same process-wide media lock the archive routes do, or drive loads + interleave (plain files always target drive 0). Source-shape assertion: + the lock acquisition must wrap run_tree_restore in the worker.""" + import inspect + + from openblade.api import routes_restore as mod + + src = inspect.getsource(mod) + worker = src.split("def _run()")[1].split("await to_thread")[0] + assert "_ARCHIVE_REQUEST_LOCK" in worker diff --git a/tests/property/__pycache__/__init__.cpython-310.pyc b/tests/property/__pycache__/__init__.cpython-310.pyc deleted file mode 100644 index 396d80c..0000000 Binary files a/tests/property/__pycache__/__init__.cpython-310.pyc and /dev/null differ diff --git a/tests/property/__pycache__/test_robot_transitions.cpython-310-pytest-8.3.2.pyc b/tests/property/__pycache__/test_robot_transitions.cpython-310-pytest-8.3.2.pyc deleted file mode 100644 index b4c15c0..0000000 Binary files a/tests/property/__pycache__/test_robot_transitions.cpython-310-pytest-8.3.2.pyc and /dev/null differ diff --git a/tests/safety/__pycache__/__init__.cpython-310.pyc b/tests/safety/__pycache__/__init__.cpython-310.pyc deleted file mode 100644 index 7dbd6d7..0000000 Binary files a/tests/safety/__pycache__/__init__.cpython-310.pyc and /dev/null differ diff --git a/tests/unit/__pycache__/__init__.cpython-310.pyc b/tests/unit/__pycache__/__init__.cpython-310.pyc deleted file mode 100644 index cc4c115..0000000 Binary files a/tests/unit/__pycache__/__init__.cpython-310.pyc and /dev/null differ diff --git a/tests/unit/__pycache__/test_domain_models.cpython-310-pytest-8.3.2.pyc b/tests/unit/__pycache__/test_domain_models.cpython-310-pytest-8.3.2.pyc deleted file mode 100644 index 26d0626..0000000 Binary files a/tests/unit/__pycache__/test_domain_models.cpython-310-pytest-8.3.2.pyc and /dev/null differ diff --git a/tests/unit/__pycache__/test_fuse_filesystem.cpython-310-pytest-8.3.2.pyc b/tests/unit/__pycache__/test_fuse_filesystem.cpython-310-pytest-8.3.2.pyc deleted file mode 100644 index 0197880..0000000 Binary files a/tests/unit/__pycache__/test_fuse_filesystem.cpython-310-pytest-8.3.2.pyc and /dev/null differ diff --git a/tests/unit/__pycache__/test_hardware_parsers.cpython-310-pytest-8.3.2.pyc b/tests/unit/__pycache__/test_hardware_parsers.cpython-310-pytest-8.3.2.pyc deleted file mode 100644 index 7724fed..0000000 Binary files a/tests/unit/__pycache__/test_hardware_parsers.cpython-310-pytest-8.3.2.pyc and /dev/null differ diff --git a/tests/unit/__pycache__/test_hardware_safety.cpython-310-pytest-8.3.2.pyc b/tests/unit/__pycache__/test_hardware_safety.cpython-310-pytest-8.3.2.pyc deleted file mode 100644 index efdc4e3..0000000 Binary files a/tests/unit/__pycache__/test_hardware_safety.cpython-310-pytest-8.3.2.pyc and /dev/null differ diff --git a/tests/unit/__pycache__/test_safety_gates.cpython-310-pytest-8.3.2.pyc b/tests/unit/__pycache__/test_safety_gates.cpython-310-pytest-8.3.2.pyc deleted file mode 100644 index 411ff5b..0000000 Binary files a/tests/unit/__pycache__/test_safety_gates.cpython-310-pytest-8.3.2.pyc and /dev/null differ diff --git a/tests/unit/__pycache__/test_state_transitions.cpython-310-pytest-8.3.2.pyc b/tests/unit/__pycache__/test_state_transitions.cpython-310-pytest-8.3.2.pyc deleted file mode 100644 index e7c440a..0000000 Binary files a/tests/unit/__pycache__/test_state_transitions.cpython-310-pytest-8.3.2.pyc and /dev/null differ diff --git a/tests/unit/test_archive_job_logging.py b/tests/unit/test_archive_job_logging.py new file mode 100644 index 0000000..eec1105 --- /dev/null +++ b/tests/unit/test_archive_job_logging.py @@ -0,0 +1,73 @@ +"""Regression: the archive-job completion log must not crash when INFO is on. + +`run_archive_job` ended with structlog-style kwargs on a *stdlib* logger +(`logger.info("...", job_id=..., files_archived=...)`), which raises TypeError +inside `Logger._log()`. `Logger.info` checks `isEnabledFor(INFO)` first, so the +call was a no-op in a test suite that never enables INFO -- and blew up on the +SUCCESS path of a real archive job in any deployment that logs at INFO, after +the catalog had already been marked completed. + +The test therefore has to raise the level explicitly; running the archive job +without doing so proves nothing. +""" + +from __future__ import annotations + +import logging +from pathlib import Path + +import pytest + +from openblade.catalog.db import get_session, init_db +from openblade.catalog.repository import CatalogRepository +from openblade.domain.policies import FormatConfirmation, SafetyToken +from openblade.jobs.archive import ArchiveRequest, run_archive_job +from openblade.simulator.library import MockLibraryBackend +from openblade.simulator.ltfs_volume import MockLTFSBackend +from openblade.simulator.scenarios import one_drive_twenty_slots_five_cartridges + +_ARCHIVE_LOGGER = "openblade.jobs.archive" + + +def _formatted_stack() -> tuple[CatalogRepository, MockLibraryBackend, MockLTFSBackend]: + init_db("sqlite:///:memory:") + catalog = CatalogRepository(get_session()) + library, ltfs = one_drive_twenty_slots_five_cartridges() + barcode = str(library.inventory().slots[0].barcode) + library.load(1, 0) + ltfs.format(barcode, FormatConfirmation(barcode, SafetyToken.generate("format", barcode))) + library.unload(0, 1) + group = catalog.create_volume_group("photos") + catalog.add_barcode_to_volume_group(group.id, barcode) + return catalog, library, ltfs + + +def test_archive_job_completion_log_survives_info_level( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + catalog, library, ltfs = _formatted_stack() + source = tmp_path / "source" + source.mkdir() + (source / "a.txt").write_text("hello archive") + job = catalog.create_job("archive", {"source_path": str(source), "volume_group": "photos"}) + + with caplog.at_level(logging.INFO, logger=_ARCHIVE_LOGGER): + result = run_archive_job( + ArchiveRequest(source_path=source, volume_group_name="photos"), + library, + ltfs, + catalog, + job.id, + ) + + assert result.files_archived == 1 + completion = [ + record + for record in caplog.records + if record.name == _ARCHIVE_LOGGER and record.message == "archive job completed" + ] + assert len(completion) == 1 + # The structured fields survive the fix -- they moved into `extra`, which + # attaches them to the record rather than being passed to `_log()`. + assert completion[0].job_id == job.id + assert completion[0].files_archived == 1 diff --git a/tests/unit/test_catalog_backup_restore_cli.py b/tests/unit/test_catalog_backup_restore_cli.py new file mode 100644 index 0000000..fdd3021 --- /dev/null +++ b/tests/unit/test_catalog_backup_restore_cli.py @@ -0,0 +1,363 @@ +"""CLI surface for `openblade catalog backup` and `catalog restore-backup`. + +Covers: the online-backup round trip (including a concurrent writer, which is +the entire point of using sqlite3.Connection.backup() instead of a file copy), +`--keep` pruning, both destructive-restore refusal guards, and refusing a +corrupt backup before the live catalog is ever touched. +""" + +from __future__ import annotations + +import gzip +import json +import shutil +import sqlite3 +import threading +import time +from pathlib import Path + +import pytest +from typer.testing import CliRunner + +from openblade.cli import main as cli_main + +runner = CliRunner() + + +@pytest.fixture +def cli_home(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path: + """Point the CLI's ~/.openblade at a scratch dir, on the mock backend.""" + home = tmp_path / "home" + home.mkdir() + state_dir = home / ".openblade" + monkeypatch.setattr(cli_main, "_STATE_DIR", state_dir) + monkeypatch.setattr(cli_main, "_STATE_PATH", state_dir / "mock_state.json") + monkeypatch.setattr(cli_main, "_DB_PATH", state_dir / "openblade.db") + for name in ( + "OPENBLADE_BACKEND", + "OPENBLADE_REAL_HARDWARE_ENABLED", + "OPENBLADE_DB_URL", + "OPENBLADE_CACHE_DIR", + "OPENBLADE_STAGING_DIR", + "OPENBLADE_RESTORE_DIR", + ): + monkeypatch.delenv(name, raising=False) + monkeypatch.setenv("OPENBLADE_DB_URL", f"sqlite:///{state_dir / 'openblade.db'}") + monkeypatch.setenv("OPENBLADE_CACHE_DIR", str(state_dir / "cache")) + monkeypatch.setenv("OPENBLADE_STAGING_DIR", str(state_dir / "staging")) + monkeypatch.setenv("OPENBLADE_RESTORE_DIR", str(state_dir / "restore")) + return home + + +def invoke(*args: str): + return runner.invoke(cli_main.app, list(args), catch_exceptions=False) + + +def stdout_json(result) -> object: + assert result.stdout.strip(), "command produced no stdout" + return json.loads(result.stdout) + + +def _init_live_db(cli_home: Path) -> tuple[Path, str]: + """Force the live catalog DB into existence and return (db_path, canary id). + + The canary is the "seed" volume group's own id. `_get_context()` (called by + every CLI command, including a refused one) legitimately reseeds admin/ + service accounts and an inventory job on each invocation, so comparing the + whole file's bytes before/after a refusal is not a valid "untouched" check + -- it would fail even with no bug present. Checking that this specific, + pre-existing row survives unchanged is the right invariant: a real restore + (or any bug that let one slip past a guard) would replace or lose it. + """ + result = invoke("volume-group", "seed") + assert result.exit_code == 0, result.output + canary_id = stdout_json(result)["id"] + assert isinstance(canary_id, str) + return cli_home / ".openblade" / "openblade.db", canary_id + + +def _assert_canary_untouched(db_path: Path, canary_id: str) -> None: + with sqlite3.connect(str(db_path)) as conn: + row = conn.execute("SELECT name FROM volume_groups WHERE id = ?", (canary_id,)).fetchone() + assert row is not None, "canary volume group is gone -- the live catalog was replaced" + assert row[0] == "seed" + + +class TestBackupRoundTrip: + def test_backup_survives_a_concurrent_writer_and_restores_cleanly( + self, cli_home: Path, tmp_path: Path + ) -> None: + db_path, _canary_id = _init_live_db(cli_home) + stop = threading.Event() + errors: list[BaseException] = [] + committed_ids: list[str] = [] + + def writer() -> None: + # Deliberately gentle: this repo's SQLite catalog is not in WAL + # mode, so a writer and a concurrent reader/writer contend for the + # same file lock like any default-journal-mode SQLite. A tight + # write loop starves `catalog backup`'s own setup queries (which + # is a real limitation of rollback-journal SQLite, not something + # this test is trying to prove) and makes the test flaky rather + # than demonstrating the property under test: that an online + # *backup* mid-write is safe, not that arbitrary concurrent + # writers never see `database is locked`. + conn = sqlite3.connect(str(db_path), timeout=30) + try: + i = 0 + while not stop.is_set(): + job_id = f"writer-job-{i}" + try: + conn.execute( + "INSERT INTO jobs (id, job_type, state, metadata_json, " + "error, created_at, updated_at) VALUES (?, 'archive', " + "'completed', '{}', NULL, datetime('now'), datetime('now'))", + (job_id,), + ) + conn.commit() + except sqlite3.OperationalError: + conn.rollback() + time.sleep(0.05) + continue + committed_ids.append(job_id) + i += 1 + time.sleep(0.05) + except BaseException as exc: # noqa: BLE001 — worker thread's ONLY channel back to the asserting test; anything narrower hides the failure # pragma: no cover - failure path + errors.append(exc) + finally: + conn.close() + + thread = threading.Thread(target=writer) + thread.start() + time.sleep(0.1) # let the writer get going before the backup starts + + backup_dest = tmp_path / "backups" + result = invoke("catalog", "backup", "--dest", str(backup_dest)) + + stop.set() + thread.join(timeout=10) + assert not thread.is_alive() + assert not errors, f"writer thread raised: {errors}" + assert result.exit_code == 0, result.output + assert len(committed_ids) > 0, "writer never got a row committed" + + payload = stdout_json(result) + backup_file = Path(payload["backupFile"]) + assert backup_file.exists() + assert backup_file.suffix == ".gz" + + # The backup is a point-in-time snapshot taken *during* the writer loop: + # it must be internally consistent (passes integrity_check) and must not + # contain more committed rows than were ever committed -- a torn/partial + # read would show up as a corrupt file or a phantom row, not as "some + # subset of what was actually committed". + with gzip.open(backup_file, "rb") as gz_src, open(tmp_path / "check.db", "wb") as dst: + shutil.copyfileobj(gz_src, dst) + with sqlite3.connect(str(tmp_path / "check.db")) as conn: + assert conn.execute("PRAGMA integrity_check").fetchone()[0] == "ok" + rows = { + row[0] + for row in conn.execute( + "SELECT id FROM jobs WHERE id LIKE 'writer-job-%'" + ).fetchall() + } + assert rows <= set(committed_ids) + + # Now actually restore it over the live DB and confirm the destructive + # path leaves a healthy, queryable catalog behind. + restore_result = invoke( + "catalog", "restore-backup", str(backup_file), "--confirm-db-path", str(db_path) + ) + assert restore_result.exit_code == 0, restore_result.output + restore_payload = stdout_json(restore_result) + assert restore_payload["restored"] is True + checks = {c["name"]: c["ok"] for c in restore_payload["checks"]} + assert all(checks.values()), checks + + with sqlite3.connect(str(db_path)) as conn: + assert conn.execute("PRAGMA integrity_check").fetchone()[0] == "ok" + restored_rows = { + row[0] + for row in conn.execute( + "SELECT id FROM jobs WHERE id LIKE 'writer-job-%'" + ).fetchall() + } + assert restored_rows == rows + + def test_keep_prunes_older_backups(self, cli_home: Path, tmp_path: Path) -> None: + _init_live_db(cli_home) + backup_dest = tmp_path / "backups" + backup_files = [] + for _ in range(5): + result = invoke("catalog", "backup", "--dest", str(backup_dest), "--keep", "3") + assert result.exit_code == 0, result.output + backup_files.append(Path(stdout_json(result)["backupFile"])) + time.sleep(1.1) # filenames are second-resolution timestamps + + remaining = sorted(backup_dest.glob("openblade-*.db.gz")) + assert len(remaining) == 3 + # The three most-recently-created backups survive; earlier ones are gone. + assert set(remaining) == set(backup_files[-3:]) + for pruned in backup_files[:-3]: + assert not pruned.exists() + + def test_keep_zero_still_keeps_the_backup_just_taken( + self, cli_home: Path, tmp_path: Path + ) -> None: + # Regression: `--keep 0` used to mean "prune down to 0 backups + # total", which deleted the file this very invocation just wrote and + # then crashed reporting its size. `--keep N` means "keep N OLDER + # backups alongside the one just taken" -- the backup you just made + # is never a candidate for its own prune. + _init_live_db(cli_home) + backup_dest = tmp_path / "backups" + + result = invoke("catalog", "backup", "--dest", str(backup_dest), "--keep", "0") + + assert result.exit_code == 0, result.output + payload = stdout_json(result) + backup_file = Path(payload["backupFile"]) + assert backup_file.exists() + assert payload["pruned"] == [] + assert payload["retained"] == 1 + assert sorted(backup_dest.glob("openblade-*.db.gz")) == [backup_file] + + def test_keep_zero_prunes_all_older_backups(self, cli_home: Path, tmp_path: Path) -> None: + _init_live_db(cli_home) + backup_dest = tmp_path / "backups" + first = invoke("catalog", "backup", "--dest", str(backup_dest), "--keep", "99") + assert first.exit_code == 0, first.output + first_file = Path(stdout_json(first)["backupFile"]) + time.sleep(1.1) + + second = invoke("catalog", "backup", "--dest", str(backup_dest), "--keep", "0") + + assert second.exit_code == 0, second.output + payload = stdout_json(second) + second_file = Path(payload["backupFile"]) + assert str(first_file) in payload["pruned"] + assert not first_file.exists() + assert second_file.exists() + assert sorted(backup_dest.glob("openblade-*.db.gz")) == [second_file] + + +class TestRestoreBackupStaging: + def test_stages_the_decompressed_backup_next_to_the_live_db( + self, cli_home: Path, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + # Regression: the decompression workdir used to be created in the + # system temp dir (plain `tempfile.mkdtemp()`), which is frequently a + # different filesystem/mount from the live catalog's directory (e.g. + # the container's writable layer vs. a `/data` volume). The final + # `os.replace()` is only atomic -- or even possible -- when both + # paths share a filesystem, so the workdir must be created WITH + # `dir=`. This would still pass on a dev machine + # where both happen to be under /tmp, so we assert the call's `dir` + # kwarg directly rather than relying on an actual cross-device mount. + import tempfile as tempfile_module + + db_path, _canary_id = _init_live_db(cli_home) + backup_dest = tmp_path / "backups" + backup = invoke("catalog", "backup", "--dest", str(backup_dest)) + backup_file = Path(stdout_json(backup)["backupFile"]) + + seen_dirs: list[str | None] = [] + real_mkdtemp = tempfile_module.mkdtemp + + def spy_mkdtemp(*args: object, **kwargs: object) -> str: + seen_dirs.append(kwargs.get("dir")) # type: ignore[arg-type] + return real_mkdtemp(*args, **kwargs) # type: ignore[arg-type] + + monkeypatch.setattr(tempfile_module, "mkdtemp", spy_mkdtemp) + + result = invoke( + "catalog", "restore-backup", str(backup_file), "--confirm-db-path", str(db_path) + ) + + assert result.exit_code == 0, result.output + assert str(db_path.parent) in seen_dirs + + +class TestRestoreBackupGuards: + def test_refuses_when_confirm_path_does_not_match_live_db( + self, cli_home: Path, tmp_path: Path + ) -> None: + db_path, canary_id = _init_live_db(cli_home) + backup_dest = tmp_path / "backups" + backup = invoke("catalog", "backup", "--dest", str(backup_dest)) + backup_file = Path(stdout_json(backup)["backupFile"]) + + result = invoke( + "catalog", + "restore-backup", + str(backup_file), + "--confirm-db-path", + str(tmp_path / "not-the-live-db.db"), + ) + + assert result.exit_code != 0 + assert "Refusing restore" in result.output + _assert_canary_untouched(db_path, canary_id) + + def test_refuses_while_a_job_is_pending_or_running( + self, cli_home: Path, tmp_path: Path + ) -> None: + db_path, canary_id = _init_live_db(cli_home) + with sqlite3.connect(str(db_path)) as conn: + conn.execute( + "INSERT INTO jobs (id, job_type, state, metadata_json, error, " + "created_at, updated_at) VALUES ('still-running', 'restore', " + "'running', '{}', NULL, datetime('now'), datetime('now'))" + ) + conn.commit() + + backup_dest = tmp_path / "backups" + backup = invoke("catalog", "backup", "--dest", str(backup_dest)) + backup_file = Path(stdout_json(backup)["backupFile"]) + + result = invoke( + "catalog", "restore-backup", str(backup_file), "--confirm-db-path", str(db_path) + ) + + assert result.exit_code != 0 + assert "Refusing restore" in result.output + assert "running" in result.output + _assert_canary_untouched(db_path, canary_id) + with sqlite3.connect(str(db_path)) as conn: + state = conn.execute("SELECT state FROM jobs WHERE id = 'still-running'").fetchone() + assert state == ("running",) + + def test_refuses_a_corrupt_gzip_before_touching_the_live_db( + self, cli_home: Path, tmp_path: Path + ) -> None: + db_path, canary_id = _init_live_db(cli_home) + + bad_backup = tmp_path / "corrupt.db.gz" + bad_backup.write_bytes(b"not actually gzip") + + result = invoke( + "catalog", "restore-backup", str(bad_backup), "--confirm-db-path", str(db_path) + ) + + assert result.exit_code != 0 + assert "Corrupt backup file" in result.output + _assert_canary_untouched(db_path, canary_id) + + def test_refuses_a_backup_that_fails_integrity_verification( + self, cli_home: Path, tmp_path: Path + ) -> None: + db_path, canary_id = _init_live_db(cli_home) + + # A well-formed gzip stream around bytes that are not a valid SQLite + # database at all -- distinct from the "not gzip" corruption case above. + bad_backup = tmp_path / "bad-content.db.gz" + with gzip.open(bad_backup, "wb") as gz: + gz.write(b"this is not a sqlite database" * 10) + + result = invoke( + "catalog", "restore-backup", str(bad_backup), "--confirm-db-path", str(db_path) + ) + + assert result.exit_code != 0 + assert "failed integrity verification" in result.output + _assert_canary_untouched(db_path, canary_id) diff --git a/tests/unit/test_cli_new_commands.py b/tests/unit/test_cli_new_commands.py index 0b20a6e..1264829 100644 --- a/tests/unit/test_cli_new_commands.py +++ b/tests/unit/test_cli_new_commands.py @@ -176,7 +176,7 @@ def test_a_failed_file_exits_non_zero_with_the_curated_error(self, cli_home, tmp seed_tree(source) invoke("archive", "--volume-group", "photos", "--path", str(source)) # Take the tape out of the library, forcibly. - invoke("mailslot", "export", "MCK00001", "--force") + invoke("mailslot", "export", "MCK00001", "--confirm-barcode", "MCK00001") result = invoke("restore", "tree", "/photos", "--dest", str(tmp_path / "out")) @@ -393,13 +393,15 @@ def test_export_refuses_a_cartridge_with_data_and_names_it(self, cli_home, tmp_p assert result.stdout.strip() == "" assert stdout_json(invoke("mailslot", "list"))["occupiedCount"] == 0 - def test_export_with_force_reports_what_went_out_of_the_door(self, cli_home, tmp_path) -> None: + def test_export_with_matching_confirm_barcode_reports_what_went_out_of_the_door( + self, cli_home, tmp_path + ) -> None: bootstrap(cli_home) source = tmp_path / "src" seed_tree(source) invoke("archive", "--volume-group", "photos", "--path", str(source)) - result = invoke("mailslot", "export", "MCK00001", "--force") + result = invoke("mailslot", "export", "MCK00001", "--confirm-barcode", "MCK00001") assert result.exit_code == 0, result.output payload = stdout_json(result) @@ -407,6 +409,33 @@ def test_export_with_force_reports_what_went_out_of_the_door(self, cli_home, tmp assert payload["exported"]["archivedFilesOnCartridge"] == 3 assert payload["exported"]["volumeGroup"] == "photos" + def test_export_refuses_a_mistyped_confirm_barcode(self, cli_home, tmp_path) -> None: + """``--confirm-barcode`` must equal the cartridge exactly -- a typo, or + the barcode of a different cartridge, must still refuse.""" + bootstrap(cli_home) + source = tmp_path / "src" + seed_tree(source) + invoke("archive", "--volume-group", "photos", "--path", str(source)) + + result = invoke("mailslot", "export", "MCK00001", "--confirm-barcode", "MCK00002") + + assert result.exit_code == 1 + assert "Export refused" in result.stderr + assert result.stdout.strip() == "" + + def test_export_no_longer_accepts_a_force_flag(self, cli_home, tmp_path) -> None: + """``--force`` was replaced by typed ``--confirm-barcode``; it must be gone.""" + bootstrap(cli_home) + source = tmp_path / "src" + seed_tree(source) + invoke("archive", "--volume-group", "photos", "--path", str(source)) + + result = invoke("mailslot", "export", "MCK00001", "--force") + + assert result.exit_code != 0 + combined = (result.stderr + result.output).lower() + assert "no such option" in combined + def test_import_from_an_empty_element_exits_one_with_a_typed_error(self, cli_home) -> None: bootstrap(cli_home) result = invoke("mailslot", "import", "8") diff --git a/tests/unit/test_cli_version.py b/tests/unit/test_cli_version.py new file mode 100644 index 0000000..adf2234 --- /dev/null +++ b/tests/unit/test_cli_version.py @@ -0,0 +1,38 @@ +"""`openblade --version`. + +Version is single-sourced in pyproject.toml's [project].version; the CLI +reads it back via importlib.metadata rather than duplicating the string. +""" + +from __future__ import annotations + +import tomllib +from pathlib import Path + +from typer.testing import CliRunner + +from openblade.cli import main as cli_main + +runner = CliRunner() + +REPO_ROOT = Path(__file__).resolve().parents[2] + + +def test_version_matches_pyproject() -> None: + pyproject = tomllib.loads((REPO_ROOT / "pyproject.toml").read_text()) + expected = pyproject["project"]["version"] + + result = runner.invoke(cli_main.app, ["--version"], catch_exceptions=False) + + assert result.exit_code == 0, result.output + assert result.output.strip() == expected + + +def test_version_is_eager_and_short_circuits_other_options() -> None: + # --version must win even when paired with a nonsense subcommand: it is an + # is_eager callback, so it must fire (and exit 0) before typer tries to + # resolve "not-a-real-command" as a subcommand. + result = runner.invoke( + cli_main.app, ["--version", "not-a-real-command"], catch_exceptions=False + ) + assert result.exit_code == 0, result.output diff --git a/tests/unit/test_hardware_safety.py b/tests/unit/test_hardware_safety.py index 79e890a..3054723 100644 --- a/tests/unit/test_hardware_safety.py +++ b/tests/unit/test_hardware_safety.py @@ -307,7 +307,8 @@ def test_procfs_scan_finds_a_real_process_holding_the_mount(self, tmp_path: Path # `executable=` lets us choose argv[0] independently of the binary, so # the child genuinely presents as `ltfs ` in procfs - a # shebang script would show up as /bin/sh and prove nothing. - process = subprocess.Popen( # noqa: S603 + # Fixed argv list, shell=False. + process = subprocess.Popen( ["ltfs", "-c", "import time; time.sleep(30)", mount_point], executable=sys.executable, stdout=subprocess.DEVNULL, diff --git a/tests/unit/test_hardware_validation.py b/tests/unit/test_hardware_validation.py index e92506d..5d01193 100644 --- a/tests/unit/test_hardware_validation.py +++ b/tests/unit/test_hardware_validation.py @@ -38,6 +38,152 @@ def test_connect_quantum_i3_returns_inventory_report_in_dry_run(tmp_path: Path) assert report.drive_correlation[0] == {"driveId": 0, "device": "/dev/st0", "serial": ""} +class TestConnectQuantumI3TransportAwareness: + """``connect-i3`` used to hardcode ``RealLibraryBackend`` regardless of + ``OPENBLADE_ROBOTICS_TRANSPORT``, so it could not diagnose a Scalar reached + only over Web Services -- it built the SCSI (``mtx``) backend and then + crashed reading ``library.correlation``/``library.changer``, neither of + which ``ScalarHttpLibraryBackend`` has (see its class docstring). These + pin: (1) the SCSI path is unchanged, (2) the webservices path builds the + real Web Services backend via the same bootstrap builder the app uses, and + (3) fields that are genuinely transport-absent degrade to a named + "not applicable" report instead of raising ``AttributeError``. + """ + + @staticmethod + def _webservices_config(tmp_path: Path) -> OpenBladeConfig: + return OpenBladeConfig( + backend=BackendMode.REAL, + real_hardware_enabled=True, + hardware_dry_run=True, + ltfs_mount_root=str(tmp_path / "ltfs"), + robotics_transport="webservices", + scalar_url="https://library.example/", + scalar_user="admin", + scalar_password="password", + ) + + def test_scsi_transport_is_unaffected(self, tmp_path: Path) -> None: + """MUTATION ANCHOR (partial): the default/explicit ``scsi`` transport + must still build ``RealLibraryBackend`` and report a real correlation + summary, not the webservices "not applicable" degrade path.""" + config = OpenBladeConfig( + backend=BackendMode.REAL, + real_hardware_enabled=True, + hardware_dry_run=True, + ltfs_mount_root=str(tmp_path / "ltfs"), + robotics_transport="scsi", + ) + + report = connect_quantum_i3(config, runner=SafeRunner(dry_run=True)) + + assert report.changer_device == "/dev/sg0" + assert report.drive_correlation_source == "dry_run" + assert report.drive_correlation[0] == {"driveId": 0, "device": "/dev/st0", "serial": ""} + + def test_webservices_transport_builds_the_scalar_http_backend(self, tmp_path: Path) -> None: + """Verified by reading ``connect_quantum_i3``'s call to the same + ``openblade.bootstrap._create_scalar_http_library`` bootstrap uses for + a running app -- stubbed here (per ``tests/i3/test_scalar_http_*``'s + pattern) with a fake session so no real network call happens.""" + import openblade.bootstrap as bootstrap_module + from openblade.hardware.scalar_http import ScalarHttpLibraryBackend + + built: dict[str, object] = {} + + class _StubSession: + def get_json(self, path: str) -> dict[str, object]: + if path == "/aml/physicalLibrary/elements": + return { + "elementList": { + "element": [ + {"type": "slot", "address": 1, "barcode": "OB0001L8"}, + {"type": "slot", "address": 2, "barcode": None}, + {"type": "drive", "address": 1, "barcode": None, "state": "Empty"}, + ] + } + } + raise AssertionError(f"unexpected GET {path}") + + def _fake_create_scalar_http_library( + config: OpenBladeConfig, runner: SafeRunner, guard: object + ) -> ScalarHttpLibraryBackend: + library = ScalarHttpLibraryBackend( + _StubSession(), # type: ignore[arg-type] # structural stand-in for ScalarHttpSession + library_id="stub-i3", + ) + built["library"] = library + return library + + original = bootstrap_module._create_scalar_http_library + bootstrap_module._create_scalar_http_library = _fake_create_scalar_http_library + try: + report = connect_quantum_i3( + self._webservices_config(tmp_path), runner=SafeRunner(dry_run=True) + ) + finally: + bootstrap_module._create_scalar_http_library = original + + assert isinstance(built["library"], ScalarHttpLibraryBackend) + assert report.library_id == "stub-i3" + assert report.slot_count == 2 + assert report.drive_count == 1 + assert report.occupied_slot_count == 1 + + def test_webservices_transport_degrades_absent_capabilities_instead_of_crashing( + self, tmp_path: Path + ) -> None: + """MUTATION ANCHOR: remove the ``getattr(library, "correlation", None)`` + / ``getattr(library, "changer", None)`` guards (go back to + ``library.correlation``/``library.changer`` directly) and this test + fails with ``AttributeError`` instead of asserting the "not + applicable" report.""" + import openblade.bootstrap as bootstrap_module + from openblade.hardware.scalar_http import ScalarHttpLibraryBackend + + class _StubSession: + def get_json(self, path: str) -> dict[str, object]: + return { + "elementList": { + "element": [ + {"type": "drive", "address": 1, "barcode": None, "state": "Empty"}, + ] + } + } + + def _fake_create_scalar_http_library( + config: OpenBladeConfig, runner: SafeRunner, guard: object + ) -> ScalarHttpLibraryBackend: + # No correlation_factory: drive_device() refuses (typed error) -- + # exactly the "no OPENBLADE_DRIVE_SERIAL_MAP declared" case, which + # must be caught, not propagated, same as the SCSI path. + return ScalarHttpLibraryBackend( + _StubSession(), # type: ignore[arg-type] # structural stand-in for ScalarHttpSession + library_id="stub-i3", + ) + + original = bootstrap_module._create_scalar_http_library + bootstrap_module._create_scalar_http_library = _fake_create_scalar_http_library + try: + report = connect_quantum_i3( + self._webservices_config(tmp_path), runner=SafeRunner(dry_run=True) + ) + finally: + bootstrap_module._create_scalar_http_library = original + + assert report.changer_device == ( + "not applicable on webservices transport (the changer is driven over " + "AML Web Services; there is no local changer device)" + ) + assert report.drive_correlation_source == "not_applicable" + assert report.drive_correlation_serials_verified is False + assert report.drive_correlation_warnings + assert report.drive_correlation == [] + # No OPENBLADE_DRIVE_SERIAL_MAP -> drive_device() refuses; the refusal + # is swallowed into "" the same way the SCSI backend's is. + assert report.drive_devices == [""] + + def test_validate_ltfs_capabilities_reports_device_list_and_plan(tmp_path: Path) -> None: report = validate_ltfs_capabilities( _config(tmp_path), diff --git a/tests/unit/test_job_error_sanitization.py b/tests/unit/test_job_error_sanitization.py index 6ae2b22..7b9a572 100644 --- a/tests/unit/test_job_error_sanitization.py +++ b/tests/unit/test_job_error_sanitization.py @@ -4,6 +4,8 @@ import contextlib +import pytest + from openblade.domain.errors import OpenBladeError, safe_job_error from openblade.hardware.runner import CommandError @@ -63,3 +65,59 @@ def test_aml_move_error_detail_is_sanitized() -> None: src = inspect.getsource(mod) assert "detail=str(exc)" not in src assert src.count("detail=safe_job_error(exc)") >= 2 + + +def test_remote_library_probe_error_is_curated() -> None: + """The remote-library probe reports "offline" for every failure mode, but the + text it returns used to be str(exc) on a bare `except Exception`: httpx carries + the probe URL, and ConnectError wraps OS-level text. The class name is the whole + payload the client gets.""" + import asyncio + + import httpx + + from openblade.api.routes_proxy import RemoteLibraryProbeRequest, probe_remote_library + + class _ExplodingClient: + def __init__(self, *args, **kwargs) -> None: + pass + + async def __aenter__(self): + return self + + async def __aexit__(self, *exc_info) -> None: + return None + + async def post(self, *args, **kwargs): + raise RuntimeError("connect to http://remote:8000 failed: SECRET-DETAIL") + + monkeypatch = pytest.MonkeyPatch() + monkeypatch.setattr(httpx, "AsyncClient", _ExplodingClient) + try: + result = asyncio.run( + probe_remote_library( + "lib-1", + RemoteLibraryProbeRequest(host="remote", port=8000, username="u", password="p"), + None, + ) + ) + finally: + monkeypatch.undo() + + assert result["status"] == "offline" + assert "SECRET-DETAIL" not in result["error"] + assert "remote:8000" not in result["error"] + assert "RuntimeError" in result["error"] + + +def test_decode_password_hash_rejects_malformed_hashes_without_swallowing_defects() -> None: + """_decode_password_hash() narrowed from `except Exception` to the three ways a + STORED hash can be malformed (non-base64, non-ASCII, NULL column). All three + still answer "not a hash" rather than raising into the login path.""" + from openblade.api.aml_state import is_password_hash + + assert is_password_hash("not base64!!!") is False + assert is_password_hash("\u00e9\u00e9\u00e9") is False + assert is_password_hash("") is False + assert is_password_hash(None) is False # type: ignore[arg-type] + assert is_password_hash("aGVsbG8=") is False # valid base64, wrong length diff --git a/tests/unit/test_mailslot_service.py b/tests/unit/test_mailslot_service.py index 8f1137b..ce1a840 100644 --- a/tests/unit/test_mailslot_service.py +++ b/tests/unit/test_mailslot_service.py @@ -110,7 +110,7 @@ def test_honours_an_explicit_destination_slot(self) -> None: def test_clears_the_exported_flag_so_restores_work_again(self) -> None: repo, library, service = make_service() archive_one_file(repo, library, "OB0001L8", "/photos/a.txt") - service.export_cartridge("OB0001L8", force=True) + service.export_cartridge("OB0001L8", confirm_barcode="OB0001L8") assert repo.get_cartridge("OB0001L8").state == "exported" service.import_cartridge(7) @@ -165,7 +165,7 @@ def test_export_refuses_a_cartridge_carrying_data(self) -> None: Verified by deleting the ``if assessment.carries_data: raise`` block in ``TapeOperationOrchestrator._guard_export``: this test then fails while - ``test_export_with_force_moves_the_cartridge_anyway`` still passes. + ``test_export_with_matching_confirm_barcode_moves_the_cartridge_anyway`` still passes. """ repo, library, service = make_service() archive_one_file(repo, library, "OB0001L8", "/photos/holiday.jpg") @@ -190,8 +190,8 @@ def test_a_blank_tape_in_an_active_group_exports_without_a_fight(self) -> None: cartridge holding part of a striped file has instances of its own and the real check catches it. A cartridge with zero instances carries nothing -- and refusing it with the sentence "still carries archived - data" was both false and a good way to teach operators that `--force` - is the normal way to export. (Adversarial review finding.) + data" was both false and a good way to teach operators that + `--confirm-barcode` is the normal way to export. (Adversarial review finding.) """ repo, library, service = make_service() group = repo.create_volume_group("shards") @@ -220,11 +220,11 @@ def test_export_refuses_while_a_write_to_the_cartridge_is_in_flight(self) -> Non assert library.find_slot_by_barcode("OB0001L8") == 1 - def test_export_with_force_moves_the_cartridge_anyway(self) -> None: + def test_export_with_matching_confirm_barcode_moves_the_cartridge_anyway(self) -> None: repo, library, service = make_service() archive_one_file(repo, library, "OB0001L8", "/photos/holiday.jpg") - result = service.export_cartridge("OB0001L8", force=True) + result = service.export_cartridge("OB0001L8", confirm_barcode="OB0001L8") assert result.destination_slot == 7 assert library.find_slot_by_barcode("OB0001L8") is None @@ -232,11 +232,30 @@ def test_export_with_force_moves_the_cartridge_anyway(self) -> None: assert result.assessment is not None assert result.assessment.archived_files_on_cartridge == 1 + def test_confirm_barcode_must_match_exactly(self) -> None: + """MUTATION ANCHOR. A mismatched (or merely truthy) confirm_barcode must + not bypass the guard -- that would just be ``--force`` wearing a string. + + Verified by changing the equality check in + ``TapeOperationOrchestrator._guard_export`` to accept any non-empty + value (e.g. ``if confirm_barcode:``): this test then fails while + ``test_export_with_matching_confirm_barcode_moves_the_cartridge_anyway`` + still passes. + """ + repo, library, service = make_service() + archive_one_file(repo, library, "OB0001L8", "/photos/holiday.jpg") + + with pytest.raises(ExportRefusedError): + service.export_cartridge("OB0001L8", confirm_barcode="OB0002L8") + + assert library.find_slot_by_barcode("OB0001L8") == 1 + assert repo.get_cartridge("OB0001L8").state != "exported" + def test_export_marks_the_catalog_cartridge_offline(self) -> None: repo, library, service = make_service() archive_one_file(repo, library, "OB0001L8", "/photos/holiday.jpg") - service.export_cartridge("OB0001L8", force=True) + service.export_cartridge("OB0001L8", confirm_barcode="OB0001L8") # This is the flag jobs/restore.py and jobs/sharded_restore.py read. assert repo.get_cartridge("OB0001L8").state == "exported" @@ -255,7 +274,7 @@ def test_export_marks_a_cartridge_the_catalog_has_never_seen(self) -> None: repo.mark_instance_archived(instance.id) assert repo.get_cartridge("OB0001L8") is None - service.export_cartridge("OB0001L8", force=True) + service.export_cartridge("OB0001L8", confirm_barcode="OB0001L8") assert repo.get_cartridge("OB0001L8").state == "exported" @@ -269,7 +288,7 @@ def test_exported_data_actually_becomes_unrestorable(self) -> None: service = MailslotService(repo, library, ltfs) archive_one_file(repo, library, "OB0001L8", "/photos/holiday.jpg") - service.export_cartridge("OB0001L8", force=True) + service.export_cartridge("OB0001L8", confirm_barcode="OB0001L8") job = repo.create_job("restore", {}) with pytest.raises(CartridgeOfflineError): @@ -409,7 +428,7 @@ def test_export_to_a_non_ie_element_is_refused(self) -> None: TapeOpRequest( op_type=TapeOpType.EXPORT, barcode="OB0002L8", - extras={"ie_slot": 3, "force": True}, # 3 is a STORAGE slot + extras={"ie_slot": 3, "confirmBarcode": "OB0002L8"}, # 3 is a STORAGE slot ), ) assert record.status is TapeOpStatus.FAILED @@ -500,7 +519,7 @@ def test_the_catalog_flag_is_written_by_the_orchestrator_not_the_service( TapeOpRequest( op_type=TapeOpType.EXPORT, barcode="OB0001L8", - extras={"ie_slot": 5, "force": True}, + extras={"ie_slot": 5, "confirmBarcode": "OB0001L8"}, ), ) assert record.status is TapeOpStatus.COMPLETED diff --git a/tests/unit/test_nas_hydration.py b/tests/unit/test_nas_hydration.py index 9e58e72..cd15a57 100644 --- a/tests/unit/test_nas_hydration.py +++ b/tests/unit/test_nas_hydration.py @@ -432,3 +432,29 @@ def get_dataset(self, dataset_id): ) assert "hydration tape read failed" in joined assert any(r.exc_info is not None for r in caplog.records), "no traceback captured" + + +def test_per_file_error_message_is_sanitized(monkeypatch) -> None: + """NasRestoreJob.error_message is served by GET /nas/restore-jobs/{id}, a route + with no auth dependency. A CommandError there would publish mtx/mkltfs argv and + raw tool stderr, which is exactly what safe_job_error() exists to prevent on + jobs.error. The relative path stays — it names which file failed and is already + readable through the dataset listing.""" + from openblade.hardware.runner import CommandError + + service, job = setup_restore_fixture(paths=["photos/a.jpg"], tapes=["VOL001L9"]) + executor = hydration_executor(service) + + def leaky(record: NasFileRecord) -> bytes: + raise CommandError(["mtx", "-f", "/dev/sg0", "status"], 1, "raw stderr LEAK") + + monkeypatch.setattr(executor, "_simulate_content", leaky) + + restored = executor.run(job.id) + + assert restored.error_message is not None + assert "LEAK" not in restored.error_message + assert "/dev/sg0" not in restored.error_message + assert "mtx" not in restored.error_message + assert "CommandError" in restored.error_message + assert "photos/a.jpg" in restored.error_message diff --git a/tests/unit/test_nas_ingest.py b/tests/unit/test_nas_ingest.py index a0cb7ae..ea3e2c3 100644 --- a/tests/unit/test_nas_ingest.py +++ b/tests/unit/test_nas_ingest.py @@ -437,3 +437,111 @@ def test_ingest_with_no_plan_returns_400(tmp_path: Path) -> None: ) assert response.status_code == 400 + + +def test_ingest_run_failure_error_is_sanitized(tmp_path: Path, monkeypatch) -> None: + """IngestJob.errors is served by GET /nas/ingest/{job_id}, a route with no auth + dependency. run()'s catch-all owns the dataset's terminal state, so it must stay + broad — but the text it records goes through safe_job_error(), or a CommandError + publishes mkltfs/mtx argv and stderr on an unauthenticated surface.""" + from openblade.hardware.runner import CommandError + from openblade.nas.ingest import CacheDriveIngest + + service, cache_root = _setup_service(tmp_path) + plan = register_archive_plan(_make_plan(cache_root)) + job = start_ingest_job( + plan=plan, + dataset_name="dataset-a", + pool_id="pool-1", + nas_service=service, + cache_drive_id="cache-1", + ) + + def leaky(self): + raise CommandError(["mkltfs", "-d", "/dev/nst0"], 1, "raw stderr LEAK") + + monkeypatch.setattr(CacheDriveIngest, "_prepare_files", leaky) + + result = _run_job(service, job.job_id) + + assert result.status is DatasetStatus.FAILED + joined = " ".join(result.errors) + assert "LEAK" not in joined + assert "/dev/nst0" not in joined + assert "mkltfs" not in joined + assert "CommandError" in joined + + +def test_ingest_per_file_error_is_sanitized(tmp_path: Path, monkeypatch) -> None: + """The per-file handler records into the same unauthenticated IngestJob.errors, + so it goes through safe_job_error() too. The relative path stays: it names which + file failed, which the dataset listing already shows.""" + from openblade.hardware.runner import CommandError + + service, cache_root = _setup_service(tmp_path) + plan = register_archive_plan(_make_plan(cache_root)) + job = start_ingest_job( + plan=plan, + dataset_name="dataset-a", + pool_id="pool-1", + nas_service=service, + cache_drive_id="cache-1", + ) + + context = get_context() + + def leaky_write(handle, dest, content, **kwargs): + raise CommandError(["mkltfs", "-d", "/dev/nst0"], 1, "raw stderr LEAK") + + monkeypatch.setattr(context.ltfs, "write_bytes", leaky_write) + + result = _run_job(service, job.job_id) + + joined = " ".join(result.errors) + assert result.files_failed == 2 + assert "LEAK" not in joined + assert "/dev/nst0" not in joined + assert "a.txt" in joined + assert "CommandError" in joined + + +def test_preflight_refusals_keep_their_curated_text(tmp_path: Path) -> None: + """IngestRefusedError is typed so safe_job_error passes it through: sanitizing + the run() boundary must not degrade the messages an operator acts on.""" + from openblade.domain.errors import safe_job_error + from openblade.nas.ingest import IngestRefusedError + + exc = IngestRefusedError("Cache drive cache-1 cannot reserve 10 bytes") + assert safe_job_error(exc) == "Cache drive cache-1 cannot reserve 10 bytes" + assert isinstance(exc, RuntimeError) + + +def test_ingest_per_file_failure_is_logged(tmp_path: Path, monkeypatch, caplog) -> None: + """The per-file handler is exempt from BLE001 only because ruff ignores except + bodies containing a `raise` — and that raise is unreachable on the default + continue-on-error path. The cause must still reach the log.""" + import logging + + service, cache_root = _setup_service(tmp_path) + plan = register_archive_plan(_make_plan(cache_root)) + job = start_ingest_job( + plan=plan, + dataset_name="dataset-a", + pool_id="pool-1", + nas_service=service, + cache_drive_id="cache-1", + ) + + context = get_context() + + def boom(handle, dest, content, **kwargs): + raise RuntimeError("simulated per-file failure") + + monkeypatch.setattr(context.ltfs, "write_bytes", boom) + + with caplog.at_level(logging.WARNING, logger="openblade.nas.ingest"): + result = _run_job(service, job.job_id) + + assert result.files_failed == 2 + assert caplog.records, "the per-file failure was not logged at all" + assert any(r.exc_info is not None for r in caplog.records), "no traceback captured" diff --git a/tests/unit/test_web_flask_app.py b/tests/unit/test_web_flask_app.py new file mode 100644 index 0000000..85ff8aa --- /dev/null +++ b/tests/unit/test_web_flask_app.py @@ -0,0 +1,1674 @@ +from __future__ import annotations + +from dataclasses import replace +from ipaddress import ip_address + +import pytest + +from openblade.web_flask import app as web_flask_app +from openblade.web_flask.app import _validate_device_url, create_app +from openblade.web_flask.client import BackendClient, BackendError +from openblade.web_flask.models import Device + + +class FakeBackendClient(BackendClient): + def __init__(self) -> None: + super().__init__(base_url="http://backend") + self.devices = [ + Device( + id=1, + name="Quantum i3 A", + emulator_url="http://device-a:8010", + model="Scalar i3", + role="primary", + status="online", + enabled=True, + sort_order=1, + ) + ] + self.jobs = [ + { + "id": "job-1", + "state": "running", + "job_type": "archive", + "error": None, + "created_at": "2026-05-30T00:00:00Z", + "updated_at": "2026-05-30T00:01:00Z", + "library_id": 1, + } + ] + self.mounts = [ + { + "id": "mount-1", + "barcode": "ABC123L7", + "drive": "DRV001", + "state": "mounted", + "mountTime": "2026-05-30T00:01:30Z", + } + ] + self.inventory_status = {"state": "idle"} + self.import_status = {"state": "idle"} + self.export_status = {"state": "idle"} + self.last_move: dict[str, object] | None = None + self.last_export: dict[str, object] | None = None + self.last_restore: dict[str, object] | None = None + self.probe_calls: list[dict[str, object]] = [] + self.last_pool: dict[str, object] | None = None + self.last_share: dict[str, object] | None = None + self.last_magazine_action: tuple[str, str] | None = None + self.last_policy: dict[str, object] | None = None + self.last_cache_drive: dict[str, object] | None = None + self.last_source_stream: dict[str, object] | None = None + self.last_user: dict[str, object] | None = None + self.users = [ + {"name": "admin", "role": 0, "requirePasswordChange": False}, + {"name": "operator1", "role": 1, "requirePasswordChange": False}, + ] + self.policies = [ + { + "id": "balanced", + "name": "Balanced", + "policy_type": "balanced", + "default_ingest_mode": "cache_drive", + "copies_required": 1, + "allow_sharding": False, + "shard_size_bytes": None, + "max_parallelism": 1, + "auto_clean_before_archive": True, + } + ] + self.cache_drives = [ + { + "id": "cache-primary", + "name": "Primary Cache", + "root_path": "/openblade/cache", + "max_bytes": 107374182400, + "min_free_bytes": 5368709120, + "eviction_policy": "after_verified", + "enabled": True, + } + ] + self.source_stream = { + "enabled": True, + "require_source_online_for_entire_job": True, + "preflight_read_check": True, + "checksum_mode": "precompute_and_post_verify", + "max_retries": 3, + "fail_on_source_change": True, + "allow_partial_dataset_success": False, + } + self.volume_groups = [{"id": "vg-default", "name": "Default", "barcodes": ["ABC123L7"]}] + self.pools = [ + { + "id": "pool-1", + "name": "Primary Pool", + "replication_factor": 1, + "backup_order_mode": "sequential", + "access_mode": "read_only", + "volume_group_ids": ["vg-default"], + } + ] + self.shares = [ + { + "path": "/openblade/inbox", + "name": "inbox", + "share_type": "inbox", + "pool_ids": ["pool-1"], + "folder_mappings": [ + {"folder_path": "/inbox", "pool_id": "pool-1", "access_mode": "read_write"} + ], + "writable": True, + } + ] + self.magazines = [ + { + "id": "MAG-1", + "location": "left", + "status": "inserted", + "slotCount": 10, + "occupiedSlots": 2, + "tapes": ["ABC123L7"], + }, + ] + self.magazine_slots = { + "MAG-1": [ + { + "id": "slot-1", + "address": "1,1,1", + "state": "occupied", + "barcode": "ABC123L7", + "type": "magazine", + } + ] + } + self.datasets = [ + { + "id": "dataset-1", + "name": "finance-q1", + "pool_id": "pool-1", + "source_path": "/openblade/inbox/finance-q1", + "file_count": 2, + "total_bytes": 4096, + "status": "archived", + } + ] + self.last_dataset_verify_id: str | None = None + self.last_archive: dict[str, object] | None = None + #: AML role the fake reports from /aml/users/me. 0 == Administrator, which + #: is what most tests need; set it to 1 or 2 to exercise the write gates. + self.current_role = 0 + self.logout_calls: list[str] = [] + + def login(self, *, username: str, password: str) -> str: + if username == "admin" and password == "admin": + return "session-token" + raise BackendError(status_code=401, detail="invalid credentials") + + def get_current_user(self, token: str) -> dict[str, object]: + assert token == "session-token" + return { + "name": "admin", + "role": self.current_role, + "requirePasswordChange": False, + } + + def logout(self, token: str) -> None: + self.logout_calls.append(token) + + def list_devices(self, token: str) -> list[Device]: + assert token == "session-token" + return self.devices + + def get_device(self, token: str, device_id: int) -> Device | None: + assert token == "session-token" + for device in self.devices: + if device.id == device_id: + return device + return None + + def create_device(self, token: str, payload: dict[str, object]) -> Device: + assert token == "session-token" + next_device = replace( + self.devices[0], + id=2, + name=str(payload["name"]), + emulator_url=str(payload["emulator_url"]), + model=str(payload["model"]), + role=str(payload["role"]), + sort_order=int(payload["sort_order"]), + ) + self.devices.append(next_device) + return next_device + + def probe_device_endpoint( + self, + *, + connection_url: str, + username: str | None = None, + password: str | None = None, + ) -> dict[str, object]: + self.probe_calls.append( + {"connection_url": connection_url, "username": username, "password": password} + ) + if "invalid" in connection_url: + raise BackendError(status_code=400, detail="Unable to reach device health endpoint") + return {"reachable": True, "authenticated": bool(username)} + + def update_device(self, token: str, device_id: int, payload: dict[str, object]) -> Device: + assert token == "session-token" + for index, device in enumerate(self.devices): + if device.id == device_id: + updated = replace( + device, + name=str(payload["name"]), + emulator_url=str(payload["emulator_url"]), + role=str(payload["role"]), + enabled=bool(payload["enabled"]), + ) + self.devices[index] = updated + return updated + raise AssertionError("device not found") + + def list_jobs( + self, + token: str, + *, + library_id: int | None = None, + state: str | None = None, + job_type: str | None = None, + ) -> list[dict[str, object]]: + assert token == "session-token" + jobs: list[dict[str, object]] = self.jobs + if library_id is not None: + jobs = [item for item in jobs if int(item["library_id"]) == library_id] + if state is not None: + jobs = [item for item in jobs if str(item["state"]) == state] + if job_type is not None: + jobs = [item for item in jobs if str(item["job_type"]) == job_type] + return jobs + + def list_media( + self, token: str, *, library_id: int, limit: int = 100 + ) -> list[dict[str, object]]: + assert token == "session-token" + assert library_id == 1 + return [ + { + "barcode": "ABC123L7", + "type": "LTO7", + "state": "online", + "partition": "p1", + "slot": "1,1,1", + } + ] + + def get_file_versions(self, token: str, *, file_path: str) -> list[dict[str, object]]: + assert token == "session-token" + assert file_path + return [ + { + "file_path": file_path, + "tape_barcode": "ABC123L7", + "tape_slot": "1,1,1", + "checksum": "abc123", + } + ] + + def list_drives(self, token: str, *, library_id: int) -> list[dict[str, object]]: + assert token == "session-token" + assert library_id == 1 + return [ + {"serialNumber": "DRV001", "type": "LTO7", "state": "online", "loadedMedia": "ABC123L7"} + ] + + def list_partitions(self, token: str, *, library_id: int) -> list[dict[str, object]]: + assert token == "session-token" + assert library_id == 1 + return [{"name": "p1", "type": "classic", "state": "online", "slotCount": 24}] + + def trigger_inventory(self, token: str, *, library_id: int) -> dict[str, object]: + assert token == "session-token" + assert library_id == 1 + return {"message": "Inventory started"} + + def move_media_operation( + self, + token: str, + *, + library_id: int, + source: str, + destination: str, + barcode: str | None = None, + ) -> dict[str, object]: + assert token == "session-token" + assert library_id == 1 + self.last_move = {"source": source, "destination": destination, "barcode": barcode} + return {"resultCode": "0", "resultMsg": "Move queued"} + + def mount_media_operation( + self, + token: str, + *, + library_id: int, + barcode: str, + drive: str, + partition: str | None = None, + ) -> dict[str, object]: + assert token == "session-token" + assert library_id == 1 + assert barcode + assert drive + return {"resultCode": "0", "resultMsg": "Mount queued"} + + def unmount_media_operation( + self, + token: str, + *, + library_id: int, + barcode: str, + drive: str, + ) -> dict[str, object]: + assert token == "session-token" + assert library_id == 1 + assert barcode + assert drive + return {"resultCode": "0", "resultMsg": "Unmount queued"} + + def start_import_operation( + self, + token: str, + *, + library_id: int, + partition: str, + ie_station: str, + ) -> dict[str, object]: + assert token == "session-token" + assert library_id == 1 + assert partition + assert ie_station + return {"resultCode": "0", "resultMsg": "Import queued"} + + def start_export_operation( + self, + token: str, + *, + library_id: int, + barcodes: list[str], + ie_station: str, + ) -> dict[str, object]: + assert token == "session-token" + assert library_id == 1 + assert barcodes + assert ie_station + self.last_export = {"barcodes": barcodes, "ie_station": ie_station} + return {"resultCode": "0", "resultMsg": "Export queued"} + + def list_mounts(self, token: str, *, library_id: int) -> list[dict[str, object]]: + assert token == "session-token" + assert library_id == 1 + return self.mounts + + def list_ie_stations(self, token: str, *, library_id: int) -> list[dict[str, object]]: + assert token == "session-token" + assert library_id == 1 + return [{"id": "IE-1", "status": "closed"}] + + def get_inventory_status(self, token: str, *, library_id: int) -> dict[str, object]: + assert token == "session-token" + assert library_id == 1 + return self.inventory_status + + def get_import_status(self, token: str, *, library_id: int) -> dict[str, object]: + assert token == "session-token" + assert library_id == 1 + return self.import_status + + def get_export_status(self, token: str, *, library_id: int) -> dict[str, object]: + assert token == "session-token" + assert library_id == 1 + return self.export_status + + def list_media_pools(self, token: str, *, library_id: int) -> list[dict[str, object]]: + assert token == "session-token" + assert library_id == 1 + return [{"name": "default", "type": "archive"}] + + def list_nas_shares(self, token: str) -> list[dict[str, object]]: + assert token == "session-token" + return self.shares + + def list_nas_pools(self, token: str) -> list[dict[str, object]]: + assert token == "session-token" + return self.pools + + def list_aml_users(self, token: str) -> list[dict[str, object]]: + assert token == "session-token" + return self.users + + def create_aml_user(self, token: str, payload: dict[str, object]) -> dict[str, object]: + assert token == "session-token" + self.last_user = payload + record = { + "name": str(payload["name"]), + "role": int(payload["role"]), + "requirePasswordChange": False, + } + self.users.append(record) + return record + + def list_nas_policies(self, token: str) -> list[dict[str, object]]: + assert token == "session-token" + return self.policies + + def create_or_update_nas_policy( + self, token: str, payload: dict[str, object] + ) -> dict[str, object]: + assert token == "session-token" + self.last_policy = payload + self.policies = [payload] + return payload + + def list_cache_drives(self, token: str) -> list[dict[str, object]]: + assert token == "session-token" + return self.cache_drives + + def create_or_update_cache_drive( + self, token: str, payload: dict[str, object] + ) -> dict[str, object]: + assert token == "session-token" + self.last_cache_drive = payload + self.cache_drives = [payload] + return payload + + def get_source_stream_config(self, token: str) -> dict[str, object]: + assert token == "session-token" + return self.source_stream + + def update_source_stream_config( + self, token: str, payload: dict[str, object] + ) -> dict[str, object]: + assert token == "session-token" + self.last_source_stream = payload + self.source_stream = payload + return payload + + def list_volume_groups(self, token: str) -> list[dict[str, object]]: + assert token == "session-token" + return self.volume_groups + + def create_nas_pool(self, token: str, payload: dict[str, object]) -> dict[str, object]: + assert token == "session-token" + self.last_pool = payload + self.pools = [payload] + return payload + + def create_nas_share(self, token: str, payload: dict[str, object]) -> dict[str, object]: + assert token == "session-token" + self.last_share = payload + self.shares.append(payload) + return payload + + def list_magazines(self, token: str, *, library_id: int) -> list[dict[str, object]]: + assert token == "session-token" + assert library_id == 1 + return self.magazines + + def list_magazine_slots( + self, token: str, *, library_id: int, magazine_id: str + ) -> list[dict[str, object]]: + assert token == "session-token" + assert library_id == 1 + return self.magazine_slots.get(magazine_id, []) + + def eject_magazine(self, token: str, *, library_id: int, magazine_id: str) -> dict[str, object]: + assert token == "session-token" + assert library_id == 1 + self.last_magazine_action = ("eject", magazine_id) + return {"resultCode": "0"} + + def insert_magazine( + self, token: str, *, library_id: int, magazine_id: str + ) -> dict[str, object]: + assert token == "session-token" + assert library_id == 1 + self.last_magazine_action = ("insert", magazine_id) + return {"resultCode": "0"} + + def get_gateway_config(self, token: str) -> dict[str, object]: + assert token == "session-token" + return {"bind_host": "0.0.0.0", "bind_port": 2222, "status": "running"} + + def get_gateway_status(self, token: str) -> dict[str, object]: + assert token == "session-token" + return {"status": "running", "active_sessions": 1, "total_sessions": 3} + + def list_gateway_credentials(self, token: str) -> list[dict[str, object]]: + assert token == "session-token" + return [{"username": "ops-user", "enabled": True}] + + def get_event_summary(self, token: str, *, library_id: int) -> dict[str, object]: + assert token == "session-token" + assert library_id == 1 + return {"total": 3, "critical": 1, "warning": 1, "info": 1} + + def get_alert_summary(self, token: str, *, library_id: int) -> dict[str, object]: + assert token == "session-token" + assert library_id == 1 + return {"critical": 1, "warning": 0, "info": 1} + + def list_events( + self, + token: str, + *, + library_id: int, + severity: str | None = None, + limit: int = 50, + ) -> list[dict[str, object]]: + assert token == "session-token" + assert library_id == 1 + events = [ + { + "timestamp": "2026-05-30T00:00:00Z", + "severity": "critical", + "component": "drive", + "message": "Drive alert", + } + ] + if severity and severity != "all": + return [item for item in events if item["severity"] == severity] + return events[:limit] + + def get_system_status(self, token: str, *, library_id: int | None = None) -> dict[str, object]: + assert token == "session-token" + return { + "overall": "good", + "cpu": "good", + "memory": "warning", + "disk": "good", + "network": "good", + "services": "good", + } + + def get_health(self, token: str) -> dict[str, object]: + assert token == "session-token" + return {"status": "healthy"} + + def get_system_config(self, token: str) -> dict[str, object]: + assert token == "session-token" + return { + "version": "0.1.0", + "backend": "simulator", + "library_count": 1, + "gateway_enabled": True, + } + + def get_catalog_status(self, token: str) -> dict[str, object]: + assert token == "session-token" + return { + "db_reachable": True, + "total_file_records": 12, + "total_datasets": 2, + "total_cartridges": 4, + "total_path_mappings": 12, + "last_rebuild_status": "completed", + } + + def get_library_status(self, token: str, *, library_id: int) -> dict[str, object]: + assert token == "session-token" + assert library_id == 1 + return {"overall": "good"} + + def list_catalog_files(self, token: str, *, limit: int = 25) -> dict[str, object]: + assert token == "session-token" + return { + "files": [ + { + "path": "/vg/file1", + "size_bytes": 10, + "primary_barcode": "ABC123L7", + "shard_count": 1, + } + ], + "total": 1, + } + + def list_nas_datasets( + self, + token: str, + *, + status: str | None = None, + limit: int = 100, + ) -> list[dict[str, object]]: + assert token == "session-token" + datasets = self.datasets[:limit] + if status is not None: + datasets = [item for item in datasets if str(item.get("status")) == status] + return datasets + + def verify_nas_dataset(self, token: str, *, dataset_id: str) -> dict[str, object]: + assert token == "session-token" + self.last_dataset_verify_id = dataset_id + for dataset in self.datasets: + if str(dataset.get("id")) == dataset_id: + dataset["status"] = "verified" + return { + "dataset_id": dataset_id, + "files_verified": 2, + "files_corrupt": 0, + "files_updated": 1, + "checksums": {"sample.bin": "abc123"}, + } + + def create_archive_job( + self, token: str, *, source_path: str, volume_group: str + ) -> dict[str, object]: + assert token == "session-token" + assert source_path + assert volume_group + self.last_archive = {"source_path": source_path, "volume_group": volume_group} + return {"job_id": "job-archive", "status": "pending"} + + def create_restore_job( + self, token: str, *, catalog_path: str, dest_path: str + ) -> dict[str, object]: + assert token == "session-token" + assert catalog_path + assert dest_path + self.last_restore = {"catalog_path": catalog_path, "destination_path": dest_path} + return {"job_id": "job-restore", "status": "pending"} + + +@pytest.fixture +def app_client(monkeypatch: pytest.MonkeyPatch): + monkeypatch.setenv("FLASK_SECRET_KEY", "test-secret-key") + # _validate_device_url resolves device hostnames and fails closed on names that + # do not resolve. The fixture's device names ("device-b") are fictional, so stub + # the resolver to a public address rather than making the suite depend on live + # DNS. The deny-list behaviour itself is covered directly against + # _validate_device_url in the SSRF tests below. + monkeypatch.setattr( + web_flask_app, "_resolve_device_host", lambda hostname: [ip_address("198.51.100.10")] + ) + app = create_app(client_factory=FakeBackendClient) + app.config["TESTING"] = True + return app.test_client() + + +def _backend_from_client(client) -> FakeBackendClient: + app = client.application + backend = app.extensions["backend_client"] + assert isinstance(backend, FakeBackendClient) + return backend + + +def _csrf(client) -> str: + with client.session_transaction() as sess: + return str(sess["_csrf_token"]) + + +def _login(client) -> None: + client.get("/login") + client.post( + "/login", + data={"username": "admin", "password": "admin", "csrf_token": _csrf(client)}, + follow_redirects=False, + ) + + +def test_requires_auth_for_devices_page(app_client) -> None: + response = app_client.get("/devices") + assert response.status_code == 302 + assert "/login" in response.headers["Location"] + + +def test_login_and_devices_page(app_client) -> None: + _login(app_client) + response = app_client.get("/devices") + assert response.status_code == 200 + body = response.get_data(as_text=True) + assert "Connected Devices" in body + assert "Fleet" not in body + assert "Manage" in body + assert "Reports" in body + assert "System" in body + + +def test_login_rejects_protocol_relative_next_redirect(app_client) -> None: + app_client.get("/login?next=//evil.example/path") + response = app_client.post( + "/login", + data={ + "username": "admin", + "password": "admin", + "next": "//evil.example/path", + "csrf_token": _csrf(app_client), + }, + follow_redirects=False, + ) + assert response.status_code == 302 + assert response.headers["Location"].endswith("/devices") + + +def test_login_rejects_control_chars_in_next_redirect(app_client) -> None: + app_client.get("/login") + response = app_client.post( + "/login", + data={ + "username": "admin", + "password": "admin", + "next": "/devices\x00//evil.example", + "csrf_token": _csrf(app_client), + }, + follow_redirects=False, + ) + assert response.status_code == 302 + assert response.headers["Location"].endswith("/devices") + + +def test_login_session_is_permanent(app_client) -> None: + _login(app_client) + with app_client.session_transaction() as sess: + assert sess.permanent is True + + +def test_logout_requires_post_and_csrf(app_client) -> None: + _login(app_client) + get_response = app_client.get("/logout") + assert get_response.status_code == 405 + post_response = app_client.post( + "/logout", data={"csrf_token": _csrf(app_client)}, follow_redirects=False + ) + assert post_response.status_code == 302 + assert post_response.headers["Location"].endswith("/login") + + +def test_login_rate_limit_blocks_after_repeated_failures(app_client) -> None: + app_client.get("/login") + for _ in range(8): + response = app_client.post( + "/login", + data={"username": "admin", "password": "bad-password", "csrf_token": _csrf(app_client)}, + follow_redirects=False, + ) + assert response.status_code == 401 + blocked = app_client.post( + "/login", + data={"username": "admin", "password": "bad-password", "csrf_token": _csrf(app_client)}, + follow_redirects=False, + ) + assert blocked.status_code == 429 + + +def test_login_rate_limit_ignores_forged_forwarded_for(app_client) -> None: + """A rotating ``X-Forwarded-For`` must not mint fresh rate-limit identities. + + The header is caller-controlled, so if ``_client_ip`` honoured it an attacker + would defeat the login lockout entirely by sending a new value each attempt. + Mirrors the rule documented on ``openblade.api.routes_assist.client_key``. + + Mutation check: restore the ``X-Forwarded-For`` branch in ``_client_ip`` and + this test fails — every attempt lands in a distinct bucket, so the ninth POST + returns 401 instead of 429. + """ + app_client.get("/login") + for attempt in range(8): + response = app_client.post( + "/login", + data={ + "username": "admin", + "password": "bad-password", + "csrf_token": _csrf(app_client), + }, + headers={"X-Forwarded-For": f"10.9.9.{attempt}"}, + follow_redirects=False, + ) + assert response.status_code == 401 + blocked = app_client.post( + "/login", + data={"username": "admin", "password": "bad-password", "csrf_token": _csrf(app_client)}, + headers={"X-Forwarded-For": "10.9.9.200"}, + follow_redirects=False, + ) + assert blocked.status_code == 429 + + +def test_register_device_requires_csrf(app_client) -> None: + _login(app_client) + response = app_client.post( + "/devices/register", + data={ + "name": "Quantum i3 B", + "connection_url": "http://device-b:8010", + "serial_number": "SN-2", + "model": "Scalar i3", + }, + ) + assert response.status_code == 400 + + +def test_register_device_flow(app_client) -> None: + _login(app_client) + app_client.get("/devices/register") + response = app_client.post( + "/devices/register", + data={ + "name": "Quantum i3 B", + "connection_url": "http://device-b:8010", + "serial_number": "SN-2", + "model": "Scalar i3", + "csrf_token": _csrf(app_client), + }, + follow_redirects=False, + ) + assert response.status_code == 302 + assert response.headers["Location"].endswith("/devices") + backend = _backend_from_client(app_client) + assert backend.probe_calls[-1]["connection_url"] == "http://device-b:8010" + + +def test_register_page_hides_storage_role_and_supports_probe_auth(app_client) -> None: + _login(app_client) + response = app_client.get("/devices/register") + assert response.status_code == 200 + body = response.get_data(as_text=True) + assert "Storage role" not in body + assert "Device Username (optional)" in body + assert "Device Password (optional)" in body + + +def test_devices_index_hides_role_metadata(app_client) -> None: + _login(app_client) + response = app_client.get("/devices") + assert response.status_code == 200 + body = response.get_data(as_text=True) + assert "Role:" not in body + + +def test_probe_endpoint_validates_auth_connectivity(app_client) -> None: + _login(app_client) + app_client.get("/devices/register") + response = app_client.post( + "/devices/probe", + data={ + "connection_url": "http://device-b:8010", + "device_username": "operator", + "device_password": "secret", + "csrf_token": _csrf(app_client), + }, + ) + assert response.status_code == 200 + payload = response.get_json() + assert payload is not None + assert payload["ok"] is True + assert payload["probe"]["reachable"] is True + assert payload["probe"]["authenticated"] is True + backend = _backend_from_client(app_client) + assert backend.probe_calls[-1]["username"] == "operator" + + +def test_manage_page_is_device_scoped(app_client) -> None: + _login(app_client) + response = app_client.get("/devices/1/manage", follow_redirects=True) + assert response.status_code == 200 + body = response.get_data(as_text=True) + assert "Inventory" in body + assert "Drives" in body + assert "Quantum i3 A" in body + + +def test_jobs_reports_and_system_pages_render(app_client) -> None: + _login(app_client) + jobs_response = app_client.get("/jobs") + reports_response = app_client.get("/reports") + system_response = app_client.get("/system") + assert jobs_response.status_code == 200 + assert reports_response.status_code == 200 + assert system_response.status_code == 200 + assert "Jobs" in jobs_response.get_data(as_text=True) + assert "Reports" in reports_response.get_data(as_text=True) + assert "System Health" in system_response.get_data(as_text=True) + + +def test_system_page_lists_users_and_creation_controls(app_client) -> None: + _login(app_client) + response = app_client.get("/system") + assert response.status_code == 200 + body = response.get_data(as_text=True) + assert "Multi-user accounts" in body + assert "admin" in body + assert "Create user" in body + + +def test_system_create_user_flow(app_client) -> None: + _login(app_client) + app_client.get("/system") + response = app_client.post( + "/system/users", + data={ + "username": "operator2", + "password": "operator-password", + "role": "1", + "csrf_token": _csrf(app_client), + }, + follow_redirects=False, + ) + assert response.status_code == 302 + assert response.headers["Location"].endswith("/system") + backend = _backend_from_client(app_client) + assert backend.last_user is not None + assert backend.last_user["name"] == "operator2" + assert backend.last_user["role"] == 1 + + +def test_storage_paths_reject_parent_traversal(app_client) -> None: + _login(app_client) + app_client.get("/storage/archive") + response = app_client.post( + "/storage/archive", + data={ + "source_path": "/data/../etc", + "volume_group": "default", + "csrf_token": _csrf(app_client), + }, + follow_redirects=False, + ) + assert response.status_code == 302 + assert response.headers["Location"].endswith("/storage/archive") + + +def test_storage_access_page_shows_windows_and_linux_mount_commands(app_client) -> None: + _login(app_client) + response = app_client.get("/storage/access") + assert response.status_code == 200 + body = response.get_data(as_text=True) + assert "Windows mapped drive" in body + assert "net use Z:" in body + assert "mount -t cifs" in body + assert "sftp -P 2222 ops-user@" in body + + +def test_storage_write_path_page_renders_controls(app_client) -> None: + _login(app_client) + response = app_client.get("/storage/write-path") + assert response.status_code == 200 + body = response.get_data(as_text=True) + assert "Policy for multi-tape writes" in body + assert "Cache drive staging" in body + assert "Streaming direct-to-tape" in body + assert "Folder-to-pool access mapping" in body + assert "Optimization guidance" in body + + +def test_storage_catalog_page_renders_dataset_verification_workbench(app_client) -> None: + _login(app_client) + response = app_client.get("/storage/catalog") + assert response.status_code == 200 + body = response.get_data(as_text=True) + assert "Dataset verification workbench" in body + assert "finance-q1" in body + assert "Verify now" in body + + +def test_storage_catalog_verify_dataset_flow(app_client) -> None: + _login(app_client) + app_client.get("/storage/catalog") + response = app_client.post( + "/storage/datasets/dataset-1/verify", + data={"csrf_token": _csrf(app_client)}, + follow_redirects=False, + ) + assert response.status_code == 302 + assert response.headers["Location"].endswith("/storage/catalog") + backend = _backend_from_client(app_client) + assert backend.last_dataset_verify_id == "dataset-1" + + catalog = app_client.get("/storage/catalog") + body = catalog.get_data(as_text=True) + assert "Last verify run" in body + assert "dataset-1" in body + + +def test_storage_write_path_actions_flow(app_client) -> None: + _login(app_client) + app_client.get("/storage/write-path") + policy_response = app_client.post( + "/storage/policies", + data={ + "policy_id": "critical-fast", + "name": "Critical Fast", + "policy_type": "critical_sequential", + "default_ingest_mode": "source_stream", + "copies_required": "2", + "max_parallelism": "4", + "shard_size_bytes": "1048576", + "shard_strategy": "round_robin", + "allow_sharding": "on", + "verify_before_archive": "on", + "verify_after_archive": "on", + "auto_clean_before_archive": "on", + "csrf_token": _csrf(app_client), + }, + follow_redirects=False, + ) + assert policy_response.status_code == 302 + backend = _backend_from_client(app_client) + assert backend.last_policy is not None + assert backend.last_policy["id"] == "critical-fast" + assert backend.last_policy["copies_required"] == 2 + assert backend.last_policy["shard_size_bytes"] == 1048576 + assert backend.last_policy["auto_clean_before_archive"] is True + + app_client.get("/storage/write-path") + cache_response = app_client.post( + "/storage/cache-drives", + data={ + "drive_id": "cache-b", + "name": "Cache B", + "root_path": "/openblade/cache/b", + "max_bytes": "1000000", + "min_free_bytes": "1000", + "retention_days": "7", + "stabilization_seconds": "3", + "eviction_policy": "after_verified", + "enabled": "on", + "verify_before_archive": "on", + "verify_after_archive": "on", + "csrf_token": _csrf(app_client), + }, + follow_redirects=False, + ) + assert cache_response.status_code == 302 + assert backend.last_cache_drive is not None + assert backend.last_cache_drive["id"] == "cache-b" + assert backend.last_cache_drive["root_path"] == "/openblade/cache/b" + + app_client.get("/storage/write-path") + source_response = app_client.post( + "/storage/source-stream", + data={ + "checksum_mode": "streaming", + "max_retries": "2", + "enabled": "on", + "require_source_online_for_entire_job": "on", + "preflight_read_check": "on", + "fail_on_source_change": "on", + "csrf_token": _csrf(app_client), + }, + follow_redirects=False, + ) + assert source_response.status_code == 302 + assert backend.last_source_stream is not None + assert backend.last_source_stream["checksum_mode"] == "streaming" + assert backend.last_source_stream["max_retries"] == 2 + + app_client.get("/storage/write-path") + share_response = app_client.post( + "/storage/shares", + data={ + "share_path": "/openblade/finance", + "share_name": "finance", + "share_type": "pool", + "default_policy_id": "critical-fast", + "pool_ids": "pool-1,pool-2", + "folder_mappings": "/finance/reports|pool-1|read_only\n/finance/ops|pool-2|read_write", + "writable": "on", + "csrf_token": _csrf(app_client), + }, + follow_redirects=False, + ) + assert share_response.status_code == 302 + assert backend.last_share is not None + assert backend.last_share["path"] == "/openblade/finance" + assert backend.last_share["pool_ids"] == ["pool-1", "pool-2"] + assert len(backend.last_share["folder_mappings"]) == 2 + + +def test_device_operations_actions_flow(app_client) -> None: + _login(app_client) + app_client.get("/devices/1/manage/operations") + response = app_client.post( + "/devices/1/operations/move", + data={ + "source": "1,1,1", + "destination": "DRV-1", + "barcode": "ABC123L7", + "csrf_token": _csrf(app_client), + }, + follow_redirects=False, + ) + assert response.status_code == 302 + assert response.headers["Location"].endswith("/devices/1/manage/operations") + backend = _backend_from_client(app_client) + assert backend.last_move == {"source": "1,1,1", "destination": "DRV-1", "barcode": "ABC123L7"} + + +def test_operations_section_includes_pool_magazine_and_search_ui(app_client) -> None: + _login(app_client) + response = app_client.get("/devices/1/manage/operations") + assert response.status_code == 200 + body = response.get_data(as_text=True) + assert "Pool Controls" in body + assert "Tape and Content Search" in body + assert "Magazine Control" in body + + +def test_create_pool_and_map_share_actions(app_client) -> None: + _login(app_client) + app_client.get("/devices/1/manage/operations") + pool_response = app_client.post( + "/devices/1/operations/pool", + data={ + "pool_id": "pool-2", + "pool_name": "Primary Pool", + "volume_group_ids": "vg-default", + "restore_target_path": "/openblade/restore", + "replication_factor": "2", + "backup_order_mode": "sequential", + "access_mode": "read_write", + "csrf_token": _csrf(app_client), + }, + follow_redirects=False, + ) + assert pool_response.status_code == 302 + backend = _backend_from_client(app_client) + assert backend.last_pool is not None + assert backend.last_pool["name"] == "Primary Pool" + assert backend.last_pool["replication_factor"] == 2 + + app_client.get("/devices/1/manage/operations") + share_response = app_client.post( + "/devices/1/operations/share", + data={ + "share_name": "poolshare", + "pool_id": "pool-1", + "writable": "on", + "csrf_token": _csrf(app_client), + }, + follow_redirects=False, + ) + assert share_response.status_code == 302 + assert backend.last_share is not None + assert backend.last_share["path"] == "/pools/pool-1" + assert backend.last_share["share_type"] == "pool" + + +def test_hydrate_restore_action_uses_latest_tape_copy(app_client) -> None: + _login(app_client) + app_client.get("/devices/1/manage/operations") + response = app_client.post( + "/devices/1/operations/restore", + data={ + "catalog_path": "/archive/file-a.bin", + # /openblade/restore, not a bare /restore: dest_path is a real + # filesystem write in jobs/restore.py, so it must sit under an allowed + # storage root. This matches tests/i3/test_09_restore_cycle.py. + "dest_path": "/openblade/restore", + "csrf_token": _csrf(app_client), + }, + follow_redirects=False, + ) + assert response.status_code == 302 + backend = _backend_from_client(app_client) + assert backend.last_restore is not None + assert backend.last_restore["destination_path"] == "/openblade/restore" + + +def test_magazine_eject_action_calls_backend(app_client) -> None: + _login(app_client) + app_client.get("/devices/1/manage/operations") + response = app_client.post( + "/devices/1/operations/magazine/MAG-1/eject", + data={"csrf_token": _csrf(app_client)}, + follow_redirects=False, + ) + assert response.status_code == 302 + backend = _backend_from_client(app_client) + assert backend.last_magazine_action == ("eject", "MAG-1") + + +def test_export_operation_validates_barcodes(app_client) -> None: + _login(app_client) + app_client.get("/devices/1/manage/operations") + response = app_client.post( + "/devices/1/operations/export", + data={ + "barcodes": "GOOD123 BAD*CODE", + "ie_station": "IE-1", + "csrf_token": _csrf(app_client), + }, + follow_redirects=False, + ) + assert response.status_code == 302 + backend = _backend_from_client(app_client) + assert backend.last_export is None + + +def test_jobs_sse_stream_returns_snapshot(app_client) -> None: + _login(app_client) + response = app_client.get("/events/jobs?once=1") + assert response.status_code == 200 + assert response.content_type.startswith("text/event-stream") + body = response.get_data(as_text=True) + assert "event: jobs.snapshot" in body + assert '"running": 1' in body + + +def test_devices_sse_stream_returns_snapshot(app_client) -> None: + _login(app_client) + response = app_client.get("/events/devices?once=1") + assert response.status_code == 200 + assert response.content_type.startswith("text/event-stream") + body = response.get_data(as_text=True) + assert "event: devices.snapshot" in body + assert "Quantum i3 A" in body + + +def test_security_headers_present(app_client) -> None: + _login(app_client) + response = app_client.get("/devices") + assert response.headers["X-Content-Type-Options"] == "nosniff" + assert "default-src 'self'" in response.headers["Content-Security-Policy"] + + +def test_headers_send_native_api_token_as_bearer_and_session_as_cookie() -> None: + """With native auth on, Authorization must carry the NATIVE token, not the session. + + openblade.api.api_auth gates the native surface on the static + OPENBLADE_API_TOKEN, while /aml/* authenticates per-user via + routes_aml_auth.require_auth, which checks the ``sessionID`` cookie before the + bearer. Sending the AML session token as the only bearer 401s every native + endpoint (verified against the live app: /jobs, /nas/policies, /api/libraries + and /volume-groups all returned 401). + + Mutation check: drop ``self.api_token or`` from BackendClient._headers and the + bearer assertion below fails. + """ + client = BackendClient(base_url="http://backend", api_token="native-tok") + headers = client._headers("aml-session-token") + assert headers["Authorization"] == "Bearer native-tok" + assert headers["Cookie"] == "sessionID=aml-session-token" + + +def test_headers_fall_back_to_session_bearer_when_no_native_token() -> None: + """No OPENBLADE_API_TOKEN configured => unchanged pre-existing behaviour. + + Native auth is disabled in that case, so the session token stays in + Authorization and nothing about the previous wiring changes. + """ + client = BackendClient(base_url="http://backend") + headers = client._headers("aml-session-token") + assert headers["Authorization"] == "Bearer aml-session-token" + assert headers["Cookie"] == "sessionID=aml-session-token" + + +@pytest.mark.parametrize( + "url", + [ + # Loopback, spelled every way that defeats a string deny-list. Each of the + # first three was PROVEN to reach a live listener on 127.0.0.1 before the + # resolve-and-check fix. + "http://127.0.0.1:9911", + "http://127.1:9911", + "http://2130706433:9911", + "http://localhost.:9911", + "http://[::1]:9911", + "http://0/", + # Cloud metadata, by name and as a packed integer. + "http://169.254.169.254/", + "http://2852039166/", + "http://metadata.google.internal/", + # Other non-routable classes. + "http://224.0.0.1/", + "http://240.0.0.1/", + ], +) +def test_validate_device_url_blocks_ssrf_targets(url: str) -> None: + """A device URL must never be pointed at loopback, metadata, or multicast. + + Mutation check: delete the `_is_unsafe_ip` call from `_validate_device_url` (or + drop `is_loopback`/`is_link_local` from `_is_unsafe_ip`) and these fail. The + previous implementation compared HOST SPELLINGS against a set, which is why + "127.1" and "2130706433" both sailed through it. + """ + assert _validate_device_url(url) is not None + + +@pytest.mark.parametrize( + "url", + [ + # RFC1918 stays allowed ON PURPOSE: a real Scalar i3 lives on the operator's + # LAN, so blocking private space would refuse every legitimate device. + "http://10.0.0.5:8010/", + "http://192.168.1.10:8010/", + "http://172.16.0.1:8010/", + ], +) +def test_validate_device_url_allows_lan_devices(url: str) -> None: + assert _validate_device_url(url) is None + + +def test_validate_device_url_rejects_unresolvable_host() -> None: + """Fail closed: an unresolvable name is refused, not probed.""" + assert _validate_device_url("http://no-such-host.invalid:8010/") is not None + + +def test_validate_device_url_checks_resolved_address_not_spelling( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A benign-looking name that RESOLVES to loopback must be refused. + + This is the case a deny-list can never catch, because the attacker owns the DNS + record. Mutation check: make _validate_device_url skip the resolved-address loop + and this fails. + """ + monkeypatch.setattr( + web_flask_app, "_resolve_device_host", lambda hostname: [ip_address("127.0.0.1")] + ) + assert _validate_device_url("http://totally-normal.example.com:8010/") is not None + + +def test_validate_device_url_rejects_embedded_credentials() -> None: + assert _validate_device_url("http://user:pw@10.0.0.5:8010/") is not None + + +def test_validate_device_url_override_allows_loopback(monkeypatch: pytest.MonkeyPatch) -> None: + """The documented development escape hatch still works.""" + monkeypatch.setenv("OPENBLADE_WEB_ALLOW_UNSAFE_DEVICE_TARGETS", "true") + assert _validate_device_url("http://127.0.0.1:8010/") is None + + +def test_probe_does_not_follow_redirects(monkeypatch: pytest.MonkeyPatch) -> None: + """The probe must not chase a redirect to an unvetted host. + + The vetted URL is only the FIRST hop; a redirect is a second destination nobody + checked, and httpx replays the POST body on 307/308, which would hand the + submitted device password to whatever host the redirect names. + + Mutation check: set follow_redirects=True in probe_device_endpoint and this test + fails, because the 302 would be chased to a 200. + """ + captured: dict[str, object] = {} + + class FakeResponse: + status_code = 302 + headers = {"Location": "http://127.0.0.1:9913/healthz"} + + class FakeHttpxClient: + def __init__(self, **kwargs: object) -> None: + captured.update(kwargs) + + def __enter__(self) -> FakeHttpxClient: + return self + + def __exit__(self, *exc: object) -> None: + return None + + def get(self, *args: object, **kwargs: object) -> FakeResponse: + return FakeResponse() + + def post(self, *args: object, **kwargs: object) -> FakeResponse: + return FakeResponse() + + import openblade.web_flask.client as client_module + + monkeypatch.setattr(client_module.httpx, "Client", FakeHttpxClient) + client = BackendClient(base_url="http://backend") + with pytest.raises(BackendError): + client.probe_device_endpoint(connection_url="http://10.0.0.5:8010") + assert captured.get("follow_redirects") is False + + +@pytest.mark.parametrize( + ("path", "field", "payload", "recorder"), + [ + ("/storage/archive", "source_path", {"volume_group": "vg-default"}, "last_archive"), + ("/storage/restore", "dest_path", {"catalog_path": "/archive/f.bin"}, "last_restore"), + ( + "/storage/cache-drives", + "root_path", + {"drive_id": "c1", "name": "C"}, + "last_cache_drive", + ), + ], +) +def test_storage_writes_reject_paths_outside_allowed_roots( + app_client, path: str, field: str, payload: dict[str, str], recorder: str +) -> None: + """Operator paths are confined to allowed roots, not merely traversal-checked. + + These values reach real filesystem sinks in the API container: source_path is + rglob'd by routes_archive, dest_path is written by jobs/restore, root_path + becomes a cache-drive root. Those backend routes have no per-user authorization, + so /etc, /root and /proc/self/environ -- which holds OPENBLADE_API_TOKEN -- must + be unreachable from this form even though none of them contains "..". + + Mutation check: swap _is_allowed_storage_root back to _is_safe_storage_path at + the call site and these fail, because "/etc/passwd" passes a traversal check. + """ + _login(app_client) + backend = _backend_from_client(app_client) + app_client.get("/storage/overview") + for bad in ("/etc/passwd", "/root/.ssh", "/proc/self/environ", "/"): + data = {**payload, field: bad, "csrf_token": _csrf(app_client)} + app_client.post(path, data=data, follow_redirects=False) + assert getattr(backend, recorder) is None, f"{bad} reached the backend via {field}" + + +def test_archive_accepts_path_under_allowed_root(app_client) -> None: + """The confinement must not block the legitimate case.""" + _login(app_client) + app_client.get("/storage/overview") + response = app_client.post( + "/storage/archive", + data={ + "source_path": "/openblade/inbox/finance", + "volume_group": "vg-default", + "csrf_token": _csrf(app_client), + }, + follow_redirects=False, + ) + assert response.status_code == 302 + assert response.headers["Location"].endswith("/jobs") + + +def test_archive_rejects_unvalidated_volume_group(app_client) -> None: + """volume_group was the one field reaching the backend with no allowlist. + + It becomes a catalog path component via PurePosixPath("/") / volume_group, and + PurePosixPath does not normalise, so ".." would persist into catalog keys. + """ + _login(app_client) + app_client.get("/storage/overview") + response = app_client.post( + "/storage/archive", + data={ + "source_path": "/openblade/inbox/finance", + "volume_group": "../../etc", + "csrf_token": _csrf(app_client), + }, + follow_redirects=False, + ) + assert response.status_code == 302 + assert response.headers["Location"].endswith("/storage/archive") + + +@pytest.mark.parametrize("role", [1, 2]) +def test_config_writes_require_admin_role(app_client, role: int) -> None: + """Non-admin sessions cannot rewrite storage configuration. + + This gate lives in the UI because the backend's config surface has none: + /nas/policies, /nas/shares, /nas/pools, /nas/cache-drives and + /nas/source-stream carry no require_auth. Since BackendClient presents the + instance-wide OPENBLADE_API_TOKEN as the native bearer, every session would + otherwise act with full native authority regardless of its AML role. + + Mutation check: remove the _require_admin() calls and these fail -- role 1 and 2 + reach the backend and last_policy/last_cache_drive get set. + """ + backend = _backend_from_client(app_client) + backend.current_role = role + _login(app_client) + app_client.get("/storage/overview") + for path, data in ( + ("/storage/policies", {"policy_id": "p1", "name": "P", "policy_type": "balanced"}), + ("/storage/cache-drives", {"drive_id": "c1", "name": "C", "root_path": "/openblade/cache"}), + ("/storage/source-stream", {"checksum_mode": "streaming", "max_retries": "2"}), + ( + "/storage/shares", + {"share_path": "/openblade/x", "share_name": "x", "share_type": "pool"}, + ), + ): + response = app_client.post( + path, data={**data, "csrf_token": _csrf(app_client)}, follow_redirects=False + ) + assert response.status_code == 302 + assert backend.last_policy is None + assert backend.last_cache_drive is None + assert backend.last_source_stream is None + assert backend.last_share is None + + +def test_config_writes_allowed_for_admin_role(app_client) -> None: + """The admin path still works -- the gate is not a blanket refusal.""" + _login(app_client) + backend = _backend_from_client(app_client) + app_client.get("/storage/write-path") + response = app_client.post( + "/storage/policies", + data={ + "policy_id": "critical-fast", + "name": "Critical Fast", + "policy_type": "critical_sequential", + "copies_required": "2", + "max_parallelism": "1", + "csrf_token": _csrf(app_client), + }, + follow_redirects=False, + ) + assert response.status_code == 302 + assert backend.last_policy is not None + + +def test_unknown_role_fails_closed(app_client) -> None: + """If /aml/users/me cannot be read, writes are denied rather than allowed. + + A role we could not determine must never default to Administrator. + """ + backend = _backend_from_client(app_client) + + def boom(token: str) -> dict[str, object]: + raise BackendError(status_code=503, detail="unavailable") + + backend.get_current_user = boom # type: ignore[method-assign] + _login(app_client) + app_client.get("/storage/overview") + response = app_client.post( + "/storage/policies", + data={"policy_id": "p1", "name": "P", "csrf_token": _csrf(app_client)}, + follow_redirects=False, + ) + assert response.status_code == 302 + assert backend.last_policy is None + + +def test_logout_revokes_the_backend_session(app_client) -> None: + """Clearing the Flask cookie is not enough; the AML sessionID must be revoked.""" + _login(app_client) + backend = _backend_from_client(app_client) + app_client.post("/logout", data={"csrf_token": _csrf(app_client)}, follow_redirects=False) + assert backend.logout_calls == ["session-token"] + + +@pytest.mark.parametrize("value", ["..", ".", "..."]) +def test_identifier_allowlist_rejects_dot_only_segments(app_client, value: str) -> None: + """Dot-only identifiers are refused before they reach a URL or a path. + + _POOL_ID_ALLOWED permits ".", so ".." matched the alphabet. These values are + interpolated into backend URLs and into /pools/, and both httpx and + POSIX normalisation collapse a ".." segment -- verified: POST + /storage/datasets/../verify rewrote the backend URL to /nas/verify. + + Mutation check: call pattern.fullmatch directly instead of _is_safe_identifier + and these fail. + """ + _login(app_client) + backend = _backend_from_client(app_client) + app_client.get("/storage/catalog") + app_client.post( + f"/storage/datasets/{value}/verify", + data={"csrf_token": _csrf(app_client)}, + follow_redirects=False, + ) + assert backend.last_dataset_verify_id is None + + +def test_redirect_target_rejects_backslash() -> None: + """ "/\\evil.example" must not be accepted as a local redirect. + + urlparse reports no netloc for it, so without an explicit check the only thing + preventing an off-site redirect is Werkzeug percent-encoding the backslash in + the Location header -- someone else's escaping doing our security. + """ + from openblade.web_flask.app import _is_safe_redirect_target + + assert _is_safe_redirect_target("/devices") is True + assert _is_safe_redirect_target("/\\evil.example") is False + assert _is_safe_redirect_target("//evil.example") is False + + +def test_backend_error_does_not_echo_non_json_body() -> None: + """A non-JSON upstream body must not be relayed into the operator's browser. + + _handle_backend_error flashes BackendError.detail, so echoing response.text put + proxy/gunicorn HTML error pages -- internal paths, hostnames, stack frames -- on + screen. Only the backend's own curated JSON detail is surfaced. + + Mutation check: restore `detail = response.text` and this fails. + """ + + class FakeResponse: + status_code = 500 + text = "Traceback: /srv/app/secret.py line 3, password=hunter2" + + def json(self) -> object: + raise ValueError("not json") + + client = BackendClient(base_url="http://backend") + with pytest.raises(BackendError) as excinfo: + client._raise_for_error(FakeResponse()) # type: ignore[arg-type] + assert "hunter2" not in excinfo.value.detail + assert "Traceback" not in excinfo.value.detail + assert "500" in excinfo.value.detail + + +def test_backend_error_surfaces_curated_json_detail() -> None: + """The curated FastAPI detail IS still shown -- error quality is not degraded.""" + + class FakeResponse: + status_code = 400 + text = "" + + def json(self) -> object: + return {"detail": "Barcode ABC123L9 is not in the library"} + + client = BackendClient(base_url="http://backend") + with pytest.raises(BackendError) as excinfo: + client._raise_for_error(FakeResponse()) # type: ignore[arg-type] + assert excinfo.value.detail == "Barcode ABC123L9 is not in the library" + + +def test_login_bucket_is_capped(monkeypatch: pytest.MonkeyPatch) -> None: + """The pre-auth login-attempt dict must not grow without bound. + + It is keyed partly by a caller-supplied username, and _is_login_blocked sweeps + the whole dict on every attempt, so an unbounded dict is both a memory leak and + a self-amplifying CPU cost (measured: 50k keys -> ~13ms per login attempt). + """ + monkeypatch.setenv("FLASK_SECRET_KEY", "test-secret-key") + app = create_app(client_factory=FakeBackendClient) + for index in range(web_flask_app._LOGIN_BUCKET_MAX_KEYS + 500): + web_flask_app._record_login_failure(app, f"1.2.3.4:user{index}") + bucket = web_flask_app._attempt_bucket(app) + assert len(bucket) <= web_flask_app._LOGIN_BUCKET_MAX_KEYS + + +def test_headers_omit_credentials_when_unauthenticated() -> None: + """The login call itself must not present a session cookie.""" + client = BackendClient(base_url="http://backend") + headers = client._headers() + assert "Authorization" not in headers + assert "Cookie" not in headers + + +def test_compose_service_token_default_matches_backend_default() -> None: + """docker-compose.yml repeats service_auth's dev token; keep them in step. + + web-flask must send the same service token `api` falls back to, or /aml/mount + and /aml/unmount 403 in the default dev stack. The literal is duplicated into + compose out of necessity (compose cannot import Python), so this test is what + stops the two copies drifting. + """ + from pathlib import Path + + from openblade.api.service_auth import _DEFAULT_SERVICE_TOKEN + + compose = Path(__file__).resolve().parents[2] / "docker-compose.yml" + text = compose.read_text(encoding="utf-8") + assert ( + f"OPENBLADE_SERVICE_TOKEN: ${{OPENBLADE_SERVICE_TOKEN:-{_DEFAULT_SERVICE_TOKEN}}}" in text + ) + + +def test_compose_keeps_react_web_service_alongside_flask() -> None: + """The Flask UI is additive. `web` (React) must not have been repurposed. + + The parked change rewired `web` itself to Dockerfile.web, which would have + retired the React SPA as a side effect while frontend-build-test still gated + it. Both surfaces are named in CLAUDE.md, so both stay. + """ + from pathlib import Path + + compose = Path(__file__).resolve().parents[2] / "docker-compose.yml" + text = compose.read_text(encoding="utf-8") + assert "context: ./frontend" in text + assert "dockerfile: Dockerfile.web" in text