Repository navigation
Gap closure: frontend auth+parity, Flask NAS UI landed, mypy zero, BLE001 policy, canary green, release+backup - #45
Merged
Conversation
66 __pycache__/*.pyc files were committed despite .gitignore already covering *.py[cod] and __pycache__/. Untrack them with git rm --cached; bytecode regenerates fine on next import.
Evidence from run 35688121678 (and identically 34930111187). All three documented assumptions HELD — headers 6.17.0-1022-azure installed, "SecureBoot disabled" + sig_enforce N, PID 1 is systemd, module built and loaded, mhvtl registered 1 mediumx + 3 tape and mtx status listed all 12 slots. setup.sh printed "=== Rig is up ===". The real chain was three separate things, none of them the ones we expected: 1. No /dev/nst nodes. lsscsi showed "-" in the block column for every tape row and the evidence step reported "ls: cannot access '/dev/nst*': No such file or directory". mhvtl only registers SCSI targets; /dev/nstN comes from the kernel's `st` upper-level driver, which the Azure kernel keeps in linux-modules-extra-$(uname -r) (not installed on the runner image) rather than in linux-modules-* as the generic kernel does. A fourth assumption nobody had written down. setup.sh now modprobes st (apt-installing linux-modules-extra as a fallback) and hard-fails if it cannot, tries `ch` best-effort, waits for the scsi_tape class entries, and asserts an nst node per drive before claiming success. 2. env.sh reported the WRONG cause. "found medium changer(s) /dev/sg1 but none reporting unit serial 'OBLADE_L10'" on a rig whose device.conf says exactly OBLADE_L10. /dev/sg* is 0660 root:tape; the runner user is not in `tape`, so sg_inq returned EACCES, whose stderr the serial lookup swallowed — an empty serial is indistinguishable from someone else's library. rig_sg_inq now escalates via `sudo -n`, and rig_changer reports a permission problem as a permission problem. 3. LTFS build failed on a missing dependency, not on the documented icu or submodule blockers: "configure: error: Package requirements (net-snmp >= 5.3) were not met" after "net-snmp-config: command not found". Added libsnmp-dev. Runtime libs are now installed one per apt-get call because noble renamed libfuse2 -> libfuse2t64 and one unknown name in a batched install silently installs nothing behind `|| true`. Loudness: the suite step's `eval "$(scripts/mhvtl/env.sh)"` swallowed the failure — a failing command substitution does not fail the enclosing command under set -e, so env.sh's refusal became `eval ""` and the first red line was `OPENBLADE_BACKEND: unbound variable` well after the real diagnosis. Resolution is now its own step that checks the exit status and ::error::s. setup.sh identifies the changer with rig_changer (serial-keyed) so whatever makes env.sh refuse fails at bring-up instead.
--force let a scripted caller wave a data-carrying export through with a bare boolean, never naming the cartridge it was letting out the door. `openblade mailslot export` now requires --confirm-barcode <BARCODE> matching the cartridge exactly (extras: confirmBarcode on POST /tape-ops/execute); a mismatch still refuses. Blank/scratch-tape exports are unaffected -- they were never gated by --force. Mutation-checked: relaxing the equality check to a truthiness check makes test_confirm_barcode_must_match_exactly and test_export_refuses_a_mistyped_confirm_barcode fail; restored after confirming. Regenerated docs/wiki/reference/cli.md (tools/gen_wiki_reference.py) for the flag rename, and updated docs/runbooks/mailslot-and-bulk-restore.md which had documented this as open tech debt.
`src/lib/apiToken.ts` is the browser half of `openblade/api/api_auth.py`: it stores the token, mirrors the backend's native-vs-AML path classification (including the `/aml/proxy` exception), and decides which 401s belong to the token gate rather than the AML session. Two invariants keep auth-disabled deployments identical: no stored token means no header is ever added, and nothing prompts until a real 401 arrives. Session storage is the default slot so a token is forgotten when the tab closes; "remember" is the explicit localStorage opt-in. Tests mutation-checked: forcing `isNativeApiUrl` to true fails the "never attaches the native token to an AML request" case. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013S1qbQs31YSox1azQChqs2
Appends §7 to the rehearsal runbook from runs 35688121678 (red) and 36222156436 / 36222322745 (green, cold-build and cache-hit respectively — both 51 passed, 11 skipped, 0 failed, LTFS included). States plainly that all three predicted assumptions HELD, quotes the evidence for the fourth nobody had written down (st/ch live in linux-modules-extra on an Azure kernel, "modinfo: ERROR: Module st not found", "Installed: (none)"), the fifth (runner user is not in the `tape` group, so sg_inq EACCES masqueraded as a wrong unit serial), and the LTFS dependency §1 omitted because this host already had it (net-snmp >= 5.3). Also records the runner's non-monotonic sg/st mapping (target 2 -> st2/sg4, target 3 -> st1/sg3) as independent confirmation of §3.4's "must be read, not derived", the loudness rule the red run bought, what stays degraded (only the LTFS soft cut) and why a scheduled run cannot reuse a branch cache.
`apiRequest` in `api/client.ts` is the chokepoint nearly every api module
already goes through, so it gains the header attachment and the 401
handling. Three transports bypass it and get the smallest possible
insertion instead:
- `api/nas.ts` — `nasRequest`'s raw fetch to `/api/nas/*`
- `api/filestation.ts` — the upload `XMLHttpRequest` and the download fetch
A native 401 is routed by which gate answered it: the token gate
(`WWW-Authenticate: Bearer` / `{"error": "Unauthorized"}`) clears the bad
token and raises the token prompt, while an AML-session 401 keeps the
existing sign-in redirect. AML/iBlade requests are untouched and never
carry the token.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013S1qbQs31YSox1azQChqs2
`ApiTokenGate` renders nothing until the API client reports a 401 from the token gate, so a deployment with `OPENBLADE_API_TOKEN` unset sees no login wall and no extra request. On submit the token is stored (session-only by default, localStorage when "remember on this device" is checked) and the page reloads, which replays the navigation that failed. The input is masked and the token is never rendered back, logged, or put in a message — the curated copy distinguishes "no token yet" from "the stored token was rejected and has been discarded". Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013S1qbQs31YSox1azQChqs2
Adds a "Web UI" section to the API authentication guide: what the operator sees when auth is on, where the token is stored and for how long, and that nothing changes when auth is off. Corrects the rollout note and the troubleshooting entry that said the frontend has nowhere to put a token. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013S1qbQs31YSox1azQChqs2
shellcheck SC2010 on the nst wait-loop counter added in ea6457e — the only new finding the change introduced. `if` rather than `&&` because a failing test as the last command in a loop body aborts the script under set -e.
…kend openblade hardware connect-i3 hardcoded RealLibraryBackend regardless of OPENBLADE_ROBOTICS_TRANSPORT, so it could not diagnose a Scalar reached only over Web Services -- and library.correlation / library.changer, which ScalarHttpLibraryBackend does not have, would raise AttributeError. connect_quantum_i3 now builds the backend via the same openblade.bootstrap._create_scalar_http_library the app uses when robotics_transport == "webservices", and degrades genuinely transport-absent capabilities (a local changer device, the up-front correlation summary) to a named "not applicable" report instead of crashing. Per-drive host devices (drive_devices) already worked on both backends via the shared drive_device() method and are unchanged. Mutation-checked: reverting the getattr(...) guards back to direct attribute access makes both new webservices tests fail with AttributeError; restored after confirming. SCSI-transport behavior is pinned unchanged by test_scsi_transport_is_unaffected.
Three green dispatches: 2m41s cold, 2m14s cached, 45-minute timeout. The cold LTFS build is 40s, so the workflow header's premise that LTFS is 'the slowest thing here by far' is wrong — setup.sh is, at 55s, now that it pulls a 51MB linux-modules-extra.
Resurrects openblade/web_flask/ from the 2026-09-11 parked state (see docs/decisions/2026-09-11-parked-flask-nas-ci-readme.md). The parked note could not land it because the NAS/AML endpoints it consumes were untracked; all 46 of them now exist on master with matching methods, verified by diffing the client's paths against app.openapi(). The package needed no functional port -- 31/31 of its tests passed unchanged, mypy --strict is clean, and ruff check passed; only `ruff format` drift (the repo reformatted since) had to be applied. A separate WSGI app, not a second implementation of anything: it owns no domain logic and reaches the FastAPI control plane over HTTP via BackendClient. It sits alongside the React SPA rather than replacing it, per CLAUDE.md's "React frontend + Flask NAS UI". Two auth fixes, both found by probing the real backend rather than reading: 1. Native-surface auth was entirely broken. api_auth gates /api, /jobs, /nas, /catalog, /status, /system, /volume-groups, /archive and /restore on the static OPENBLADE_API_TOKEN bearer, while /aml/* authenticates per-user via routes_aml_auth.require_auth. The client sent its AML *session* token as the only bearer, so with native auth enabled all 21 native endpoints -- the whole Storage/Jobs/Reports/System surface -- returned 401. Fixed in one place: require_auth checks the sessionID cookie BEFORE the bearer, so the session token now travels as that cookie and Authorization carries the native token. Falls back to the old behaviour when no native token is configured. 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. Now peer-address only, matching the rule already documented on openblade.api.routes_assist.client_key. flask moves into the runtime dependencies because Dockerfile.web serves this under gunicorn and `pip install -e .` must be enough to import it. Both fixes carry mutation-checked regression tests: removing either guard makes the new test fail (verified), while the pre-existing lockout test passed against the vulnerable code -- it was vacuous for the forged-header attack. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013S1qbQs31YSox1azQChqs2
…ervice Dockerfile.web and the compose wiring, from the same parked state. Three corrections to what was parked, each verified by running the built image rather than reasoning about it: - `-k gevent` was dead. gunicorn.workers.ggevent imports `packaging`, which neither gunicorn nor gevent declares, so the container died at start with ModuleNotFoundError while `docker compose build` stayed green. Installed explicitly. This is exactly the bug the parked CI job would have shipped: its only image check was a build. - The image baked OPENBLADE_ENV=development, 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 to development for the local plain-HTTP stack. Verified: the image now exits with "FLASK_SECRET_KEY is required in production". - gunicorn ran as root. Now drops to the non-root openblade user, mirroring Dockerfile. Verified uid=999 for every process in the running container, which then serves /login 200 with X-Frame-Options: DENY. The compose change is additive where the parked one was destructive: it rewired `web` itself to Dockerfile.web, which would have retired the React SPA as a side effect while frontend-build-test still gated it. `web` stays React; the Flask UI is a new `web-flask` service behind a profile, following the `emulator` precedent, so `docker compose up` is byte-for-byte unchanged. Host port 5175: 5173 is React and 5174 is the standalone emulator UI, so both would have clashed under `make fleet-up`. Both services now declare 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, keeping default behaviour. The service token's dev default has to be repeated as a literal (compose cannot import Python) because /aml/mount and /aml/unmount require it, so a test asserts the two copies agree. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013S1qbQs31YSox1azQChqs2
Resurrects the parked ci.yml job with its path filter corrected. 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) and adding it here only tacked a docker build onto Dependabot traffic. 'docker-compose.yml' IS kept, because it defines the service this job builds. The job's unique value, given the existing layering: backend-lint already runs ruff tree-wide and backend-tests already runs tests/unit, but `mypy openblade` in backend-typecheck must stay advisory 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 -- named explicitly, since a bare `docker compose build` skips profiled services. Registered in ci-gate's needs and its check list, without which a failure would not have blocked anything. README: adds the Flask UI section, its configuration table (including that OPENBLADE_API_TOKEN must match the backend), and the web-flask-smoke row in the layered-CI table. Deliberately NOT taking two of the parked README hunks: the one deleting the `cd frontend && npm ...` quickstart line, since the React frontend is not being retired and the CI table still lists frontend-build-test; and the reword of goal 4, since landing a UI is no reason to restate the project's goals or to drop "without out-of-scope feature leakage". Verified: workflow YAML parses, `yamllint -c .yamllint` passes, and `gen_wiki_reference.py --check` reports the generated pages up to date. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013S1qbQs31YSox1azQChqs2
Ends the parked state opened on 2026-09-11 with a per-component decision, and marks the old note superseded so its now-obsolete "How to recover" section cannot mislead. The headline finding is that two months moved the ground under the parked note: the NAS/AML endpoints the Flask UI needed have all landed (46 present, 0 missing against app.openapi()), and the parity generator plus its artifacts landed independently in ce89749 -- master's generator is the same file, only ruff formatted. So the parity half retires at zero loss rather than needing the T3 emulator_contract review the old note anticipated. Includes the exact cleanup commands for the integrator, deliberately NOT run here: dropping a stash is irreversible and belongs to whoever owns that working tree. They carry two cautions -- the parity files look untracked in the main checkout only because it sits 36 commits behind origin/master, and ~45 unrelated files from the same abandoned period are explicitly out of scope. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013S1qbQs31YSox1azQChqs2
Adopting BLE001 means judging each `except Exception` individually. These six
handlers can name what they are catching, so the blind catch was costing us the
ability to see a defect in the same block:
- `_decode_password_hash`: b64decode raises binascii.Error/UnicodeEncodeError,
both ValueError subclasses; TypeError/AttributeError cover a NULL password
column. A bug in the KDF constants now surfaces instead of reading as
"password does not verify".
- AML move fast-path `int(...)` x2 -> (TypeError, ValueError); the 422 text and
`from None` are unchanged.
- AML move `result.data.get("barcode")` x2 -> (AttributeError, TypeError): a
non-mapping backend payload means "no barcode to name".
- `_parse_json_report` -> (OSError, ValueError): a vanished or truncated pytest
report means "no counts", nothing else.
- `real_library_backend` fixture -> (ImportError, AttributeError): the optional
hardware module. A constructor that raises for any other reason must not be
silently swapped for a different backend.
Behaviour is preserved at every site (same fall-through value, same status
codes); the narrowing only changes which exceptions escape.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013S1qbQs31YSox1azQChqs2
…rror fields
Three blind handlers put `str(exc)` into a field that is served to clients. This
is the leak class docs/ and test_job_error_sanitization.py already guard on
jobs.error ("CommandError carries argv + tool stderr"), reached through routes
nobody had audited:
- `HydrationExecutor._hydrate_record` -> NasRestoreJob.error_message, served by
GET /nas/restore-jobs/{id} (nas_config.router has no auth dependency).
- `_BaseIngest.run` and its per-file handler -> IngestJob.errors, served by
GET /nas/ingest/{job_id}.
- `probe_remote_library` -> {"status": "offline", "error": str(exc)}; httpx
exception text carries the probe URL and wraps OS-level detail.
All three stay broad — per-file isolation and "a probe reports, it does not
500" are the reason they exist — and are now logged with exc_info, with
safe_job_error()/a class-name string on the wire. The failing file's relative
path stays: it names what failed and the dataset listing already shows it.
Not degrading error quality to satisfy the sanitizer: ingest's curated preflight
refusals ("cache drive cannot reserve N bytes", "source_path is required",
"Source changed during source-stream ingest") were bare RuntimeErrors, which
safe_job_error reduces to a class name. They are now IngestRefusedError
(OpenBladeError, RuntimeError) so the message passes through, following the
ShardedRestoreFailed precedent in jobs/tree_restore.py; RuntimeError stays in
the bases so existing `except RuntimeError` callers are unaffected. The two
existing tests asserting that text pass unchanged.
Each leak has a regression test, mutation-checked: restoring `str(exc)` /
`{exc}` fails all four.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013S1qbQs31YSox1azQChqs2
`except Exception: pass|continue|<default>` with no log is the category BLE001
is actually worth enabling for: the handler is defensible, but the evidence is
gone. Each of these keeps its behaviour (and its broad catch, with a reason) and
gains a warning with exc_info:
- CatalogRebuildPlanner: disaster recovery discarded the only evidence of why a
cartridge could not be recovered; the operator saw "failed to recover catalog
data for VOL001L9" and nothing else.
- POST /nas/datasets/{id}/verify: a per-file read failure is counted as CORRUPT.
Logged, and the misclassification (an unavailable drive or failed mount is not
corruption) is now flagged at the site — narrowing it needs a new
NasFileState, so it is deliberately not changed here.
- ArchiveLifecycleManager step dispatcher: which step failed was reported, why
was not.
- GET /ltfs/status: "unknown" with no trace of why the backend went dark.
- AML move barcode inference: the handler body was `barcode_raw = barcode_raw`,
a no-op standing in for a comment. Inner aml_state scan narrowed to
(AttributeError, TypeError, ValueError).
- bootstrap demo LTFS seeding (x2): a half-seeded demo environment looked like
an empty one. Import guard narrowed to ImportError.
- restore_and_verify campaign script: silently skipped paths with no resolvable
archived instance, so a short campaign run looked complete.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013S1qbQs31YSox1azQChqs2
…asons) These are boundaries whose job is to catch everything. Each noqa names where the error is logged and what curated form escapes, so the next reader does not have to re-derive the judgement: - health_service (x10): readiness, library status, four catalog counts, the rebuild lookup, the per-probe DB loop, library and LTFS component checks. All already log with exc_info; the escape is a degraded HealthStatus, never an exception to the caller. A health endpoint that 500s is a broken health endpoint. - cli/main restore_file / restore_tree / archive_sharded (x3): CLI top level, safe_job_error() into the job record and stderr, then exit 1. - routes_iblade CSV and XML negotiation (x2): the escape is the original JSON body plus X-OpenBlade-Content-Negotiation: json-fallback; reason=... A parity-gated endpoint must not 500 because a converter tripped. - routes_test_runner background task: nothing can catch this task's exception; the run record (status=failed, exit_code=-1) is the escape. - check_emulators probe script (x2): an unreachable emulator is the result. - tests/hardware teardown (x7): a failed unmount in `finally` must not mask the assertion that failed first, and the unload still has to run. No behaviour change in this commit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013S1qbQs31YSox1azQChqs2
BLE is now in `select`, so every `# noqa: BLE001` in the tree is live enforcement instead of documentation about a follow-up. The rule-set comment records the policy the markers encode: broad is legitimate only at a boundary whose job is to catch everything, and the reason must name where the error is logged and what curated form escapes. S stays deliberately unselected (3,748 findings, overwhelmingly S101 `assert` in tests, which is the point of tests), so the five `# noqa: S404/S603/S310` markers were enforcing nothing while looking like they were. They are now plain comments stating the safety property they stood for — argv-list/shell=False discipline in scripts/deploy.py and the two subprocess-based tests, and that the deploy postcheck URL is operator-supplied at deploy time, never per-request. The information survives without pretending a linter checks it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013S1qbQs31YSox1azQChqs2
Bucket 1 of the mypy-to-zero effort: pure annotations, no behaviour change. - catalog/models.py: PathMapping was the one table declared with bare `Column(...)` instead of `Mapped[...] = mapped_column(...)`, so every field read/write typed as `Column[str]` rather than `str`. Same columns, same nullability, same defaults. - domain/scalar_coordinate.py: `_coerce_int` replaces six `# type: ignore` comments that mypy now reports as unused; it raises TypeError, which `from_dict` already converts to ValueError, so bad payloads behave as before. - bootstrap.py: TypedDicts for the library/demo-dataset seeder specs (they were `dict[str, object]`), enum members instead of the bare strings pydantic was coercing anyway, and explicit LibraryBackend/LTFSBackend annotations on the two-branch backend construction. 304 -> 234 mypy errors.
…arams Bucket 2: pure annotations (no-untyped-def, type-arg). No behaviour change. - Bare `dict`/`list` annotations spelled out as `dict[str, Any]` / `list[Any]` in the protocol gateway, path-mapping stats and the LTFS/storage-compat/ gateway routes. - `repo=Depends(...)` / `context=Depends(...)` parameters given the explicit types the rest of the API package already uses. - `SidecarValidationError.field` was an implicit-Optional `str = None`; now `str | None`. `SidecarResolver.__init__` takes `NasService | None`. - repository.py: `_SupportsModelDump`/`_FileRecordLike` protocols replace `hasattr`-then-attribute-access and an `object` parameter; `create_role` takes a `Mapping` (read-only, and `dict` is invariant so a caller holding a narrower value type could not pass one). - routes_ltfs: `_active_mounts` is `dict[str, MountHandle]` as its comment said, and `GET /ltfs/status` probes a `_SupportsToJson` protocol instead of relying on AttributeError from the real backend falling into the generic handler. 234 -> 171 mypy errors.
Bucket 3: annotations plus narrowings the runtime already guaranteed. No behaviour change; every narrowing is either a `cast` (no runtime effect) or an isinstance guard on a branch that previously reached the same outcome. - domain/backends.py: LTFSBackend gains `write_bytes`, which both backends already implement and callers already reached through this protocol. Positional-only with the wider `FileInstance | None` return so the real and simulator signatures both conform. - nas/ingest.py, nas/hydration.py: parameters typed `MockLibraryBackend` / `MockLTFSBackend` now take the LibraryBackend/LTFSBackend protocols. They only ever used protocol methods, and nas_config already passed `context.ltfs` -- which is the REAL backend in a real-hardware deployment, so the Mock annotations were simply wrong. - Mount registries typed `dict[str, MountHandle]` instead of `object` (routes_ltfs, nas_config verify, sharded_archive) so `unmount`/`stat`/ `write_file` type-check without a cast. - nas_config: `_as_int` for decoded-JSON drive fields (raises TypeError exactly where `int()` did); loop variables renamed where one name held two types. - nas/catalog_rebuild.py: `_text_column`/`_int_column` are cast-only helpers for columns read back out of serialized `dict[str, object]` rows -- validation stays in the pydantic model one line down, so no new failure mode. - api/main.py: middleware `call_next` typed `RequestResponseEndpoint` from starlette, removing eleven `# type: ignore[operator]` comments. - api/aml_state.py: bound `.get()` results before their isinstance checks so they narrow, widened `_next_media_slot_address` to accept the `None` partition that imported media legitimately carry, and renamed a `partition` variable that shadowed the partition *dict*. 171 -> 24 mypy errors (remaining ones are in files handled separately). One `# type: ignore[method-assign]` added, on `app.openapi = custom_openapi` -- the documented FastAPI "Extending OpenAPI" idiom, with no subclass-based alternative that keeps `app` a module-level instance.
templates/nas_placeholder.html came across with the parked package but no route, template or static file references it (grepped across .py/.html/.js/.md). Its body is scaffolding copy about where NAS actions live, so it would only mislead anyone who found it. Removed rather than landed: the previous commit is the first time this file would enter the tree, and shipping dead code in the same change that introduces it is not a tradeoff worth making. Verified: the app still builds (36 routes) and all 37 tests pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013S1qbQs31YSox1azQChqs2
Bucket 4: annotations and runtime-guaranteed narrowings across the AML routes,
the hardware adapters and the NAS/jobs leaves. No behaviour change.
Notable, beyond filling in annotations:
- domain/wire.py: one `coerce_int` replaces four independently-written private
copies of the same "narrow an `object` before `int()`" helper (aml_state
coordinate parsing, nas_config drive fields, routes_upload catalog fields,
the /aml move fast path). It accepts exactly what `int()` accepts and raises
the same exceptions, so the 422 / ValueError paths that depended on `int()`
failing still fail identically.
- domain/backends.py: `TapeListingBackend` is a separate optional protocol,
deliberately NOT folded into LibraryBackend -- the mock and real backends
implement `list_tapes`, but ScalarHttpLibraryBackend does not, so putting it
on the base protocol would be a lie about a shipped backend. See the latent
bug recorded at the one call site.
- routes_aml_library: an `_` unpack target collided with the `_: AmlUser`
auth-dependency parameter, giving one name two types. Renamed the unpack.
- routes_aml_physical: `Slot(...)` -> `Slot.model_validate({...})` because
`libraryCoordinate` is an extra field (`extra="allow"`), so it cannot be a
typed keyword argument. Verified byte-identical `model_dump_json()` output.
- pydantic `Field("x", alias=...)` spelled `Field(default="x", alias=...)`; the
first positional argument already was the default.
- rbac_service: a non-list `permissions` column now yields an empty permission
set instead of raising out of `list()`. Fail-closed either way (403 rather
than 500) and the column is always a JSON list.
- simulator MockLTFSBackend.write_file asserts the FileInstance it must have:
`write_bytes` only returns None on its barcode-string metadata branch, and
this caller always passes a MountHandle.
Four `# type: ignore` comments the first pass added were replaced with real
types; two remain in the tree and are justified at their sites.
Latent bug mypy found (call-arg), with its regression test.
`run_archive_job` ended with structlog-style keyword arguments on a *stdlib*
logger:
logger.info("archive job completed", job_id=job_id, files_archived=...)
`logging.Logger.info` forwards unknown keywords to `Logger._log()`, which raises
`TypeError: Logger._log() got an unexpected keyword argument 'job_id'`. It hid
because `Logger.info` checks `isEnabledFor(INFO)` first: with the root logger at
its default WARNING -- which is what the whole test suite runs at -- the call is
a no-op. Any deployment that logs at INFO crashed the SUCCESS path of every
archive job, *after* `catalog.update_job_state(job_id, "completed")`, so the
catalog said completed while the caller got a 500.
`logger` here is `logging.getLogger(__name__)`, not structlog (this is the only
stdlib logger in the tree being called that way -- the other nine structured
call sites all use `structlog.get_logger`), so the fields move into `extra`,
which attaches them to the record instead of passing them to `_log()`.
The test raises the level explicitly, because running the archive job without
doing so proves nothing. Mutation-checked: reverting the one-line fix makes it
fail with the TypeError.
The scalar-i3 profile has declared `ieSlotCount: 2` since it was written, but `scalar_i3_default()` never passed it to MockLibraryBackend, so the default simulator library reported an import/export station of ZERO elements while the /aml surface advertised two. Every mailslot operation therefore answered "this library has no import/export elements" — the opposite of what the config says. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013S1qbQs31YSox1azQChqs2
`openblade mailslot list|import|export` had no HTTP surface, so the web UI could
not drive the I/E station at all. This is the thin half: MailslotService keeps
every decision (slot picking, the catalog state write) and the tape orchestrator
keeps the export guard.
- GET /mailslot/slots every I/E element and what is in it
- GET /mailslot/export-preview/{barcode} what would leave, moving nothing
- POST /mailslot/import {ie_slot, to_slot?}
- POST /mailslot/export {barcode, ie_slot?, force?}
An export that would take archived data out of the library answers 409 with the
orchestrator's own refusal message — which names the cartridge, its volume group
and sample paths — and `force: true` is the only way past it, exactly as
`--force` is on the CLI. Tests cover the refusal and the forced override, and are
mutation-checked: disabling `_guard_export` fails them.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013S1qbQs31YSox1azQChqs2
`openblade hardware drive-health` was CLI-only. GET /hardware/drive-health serves the same report so the console can show a failing drive. Read-only and guarded: it goes through require_real_hardware(), so on the simulator backend it answers 503 with the curated reason naming both hardware variables rather than going looking for a device. A drive that does not implement the TapeAlert log page is a 200 with `tapeAlertSupported: false` and a reason — that is a property of the drive, not a failure. The blocking sg_inq / sg_logs subprocesses run off the event loop so a 30s device timeout cannot stall the AML parity surface. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013S1qbQs31YSox1azQChqs2
`openblade restore tree` was CLI-only. The route reuses run_tree_restore and answers with TreeRestoreResult.to_dict() verbatim, so the CLI and the API cannot drift: same request, same payload, same job row. `dry_run: true` plans only and is the preview a UI should call first — it is the only place the file and tape count shows up before media moves. Failures answer with safe_job_error() text, so mkltfs/mtx output never reaches the wire, and the job row is marked failed. It runs on the event loop thread deliberately, like POST /restore/: the catalog session is process-global and not thread-safe. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013S1qbQs31YSox1azQChqs2
Prefixes are folded out of the same /catalog/ listing the Catalog Records page browses, offered as chips, and still typeable when that listing fails. "Run restore" stays disabled until a dry run has been done for the prefix and destination currently in the form, so nobody moves media without seeing the file and tape count first. The result panel names skipped files (catalogued but never archived) and per-file failures — a tree that comes back short must not read as a clean run. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013S1qbQs31YSox1azQChqs2
Message thread with the whole conversation sent on every turn — the endpoint is stateless and request/response, so there is no stream to render. Tool calls are shown dim and by name only, matching what the API returns. The empty state carries domain suggestion chips, since the assistant's capabilities are otherwise invisible. A 503 renders the documented setup instructions rather than an error, pops the orphaned user turn and hands the question back to the composer: a thread must never show a question that nothing answered. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013S1qbQs31YSox1azQChqs2
Additive only (8 lines in App.tsx, 4 in Sidebar.tsx) so the in-flight NAS/FileStation branch, which also touches both files, stays a trivial merge. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013S1qbQs31YSox1azQChqs2
…ware fixture catch Adversarial review of this branch found two things the rule itself could not: - ingest.py `_write_assignment`: BLE001 exempts any except body containing a `raise`, and that `raise` is unreachable whenever `_abort_on_file_error()` is False — the unconditional default for CacheDriveIngest and the allow_partial_dataset_success case for SourceStreamIngest. So the ordinary continue-on-error path was a silent swallow that never appeared in the inventory. Logged with exc_info, with a regression test asserting a traceback was captured. (The ruff exemption is a reminder that the rule finds candidates, not conclusions.) - tests/hardware/conftest.py: narrowing `real_library_backend` to (ImportError, AttributeError) changed real-hardware test setup — an OSError on device open or a bad OPENBLADE_CHANGER_DEVICE would stop falling back to mtx. The fixture's contract is "some real changer backend", and this branch cannot be tested against a rig, so the broad catch is restored with the reasoning written down instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013S1qbQs31YSox1azQChqs2
`mypy openblade` now exits 0 under the repo's `strict = true` config, so the advisory exemption is gone: `continue-on-error` removed, and the job added to ci-gate's `check` list so a red typecheck actually fails the one required check (the backend-lint flip removed its `continue-on-error` but never added the gate row, so it only blocks via its own job status -- left as-is, it is not this change's job). mypy is pinned exactly (2.3.0) alongside ruff, for the reason the ruff pin records: with a floating `>=`, a new upload that adds an error code or tightens inference turns a green tree red with no commit behind it. types-PyYAML is pinned with it so openblade/nas/sidecar.py type-checks without an import ignore. docs/wiki/reference/api.md is regenerated: the six /api/gateway routes that had no return annotation now declare one, so FastAPI emits a response schema and the generated table names `object` / `object[]` instead of `application/json`. Nothing else in the page moved.
The preview panel kept describing the previously-checked cartridge after the barcode was edited, which is worse than showing nothing — and the same stale object fed the force dialog. Regression test included. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013S1qbQs31YSox1azQChqs2
openblade.db is the only record of file->tape mapping and measured tape capacities; losing it loses what's on every cartridge with almost nothing recoverable from tape. Adds an operator-facing backup/restore pair on top of the existing openblade.dr online-backup helpers: - `catalog backup --dest DIR [--keep N]`: sqlite3.Connection.backup() online snapshot (safe with a live writer, unlike a file copy), gzipped, retention pruned (default keep 14). - `catalog restore-backup <file> --confirm-db-path <path>`: destructive, refuses unless --confirm-db-path is typed exactly equal to the live DB path and refuses while any job is pending/running; decompresses and verifies (integrity_check + schema + a real query) before ever touching the live file, then atomically replaces it. The decompression workdir is staged inside the live DB's own directory (not the system temp dir) so the final replace is guaranteed to be a same-filesystem, atomic rename. - `catalog` had to become a Typer/Click group to host the new subcommands while keeping the old bare `openblade catalog [path]` invocation working; a custom _CatalogGroup routes an unrecognised first token to a synthetic `ls` subcommand instead of letting Click's own positional-argument parsing swallow it. - --keep 0 no longer deletes the backup that was just taken (it prunes only OLDER backups, never the one from this run). - docker-compose.yml gets a commented-out (off by default) nightly backup sidecar example; docs/wiki/guides/the-catalog.md gains a Backup section. Both destructive-restore guards (path mismatch, active jobs) and both bug fixes above are mutation-checked: disabling each guard/fix independently makes the corresponding test fail, then passes again once restored. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013S1qbQs31YSox1azQChqs2
- `openblade --version` reads the single-sourced pyproject [project].version
via importlib.metadata (eager Typer/Click option, exits before any
subcommand resolves).
- .github/workflows/release.yml: on `v*` tag push, cross-checks the tag
against pyproject's version (fails fast on mismatch), builds+pushes the
controller image (root Dockerfile) to ghcr.io/amantux/openblade tagged
{version,latest}, and cuts a GitHub Release with generated notes.
Permissions are least-privilege and job-scoped (contents: read at the
workflow level, contents: write + packages: write only on the release job),
matching this repo's existing workflows. Actions are pinned to tags (@vn),
matching ci.yml's own style. Dockerfile.web is not tracked on master, so no
web image is built -- documented at the build step rather than invented.
- docs/wiki/guides/releasing.md: how to cut a release, what CI does, how
operators pull the image, and a first-run caveat (see report).
This workflow has not been exercised by an actual tag push yet -- YAML
parses, yamllint is clean, and every `run:` block passes `bash -n`, but the
GHCR login/push and release-notes generation are only proven by a real `v*`
tag.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013S1qbQs31YSox1azQChqs2
The T3 reviewer returned BLOCK with four MUST-FIX items, two carrying working repros against live sockets. All are fixed here with mutation-checked tests; the suite goes 37 -> 73. 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. Now resolves the host and judges every returned address. Deliberately does NOT block RFC1918: the reviewer proposed is_private, but a real Scalar i3 lives on the operator's LAN, so that would refuse every legitimate device. Blocks loopback, link-local (covers 169.254.169.254), unspecified, multicast and reserved. Fails closed on an unresolvable name. 2. Probe followed redirects. The vetted URL was only the first hop: a host answering "302 Location: http://127.0.0.1/..." turned a blocked target into a reachable one, and httpx replays the body on 307/308, so the submitted device password could be harvested by an arbitrary host. follow_redirects=False, and a 3xx no longer counts as reachable or as a successful auth. 3. Path confinement. _is_safe_storage_path blocked traversal but not WHICH absolute roots were reachable, and those values become real filesystem sinks in a backend whose /archive and /restore routes have no per-user authorization -- so /proc/self/environ, which holds OPENBLADE_API_TOKEN, was archivable to tape. Added _is_allowed_storage_root, applied to the four genuine filesystem paths only. catalog_path and folder_path deliberately keep the traversal-only check: the former is a catalog key resolved in the DB, the latter is share-relative and matching the backend's own validator. Over-applying it would have broken legitimate config. 4. Confused deputy. Because the client presents the instance-wide OPENBLADE_API_TOKEN as the native bearer, and the native config surface (/nas/policies, /nas/shares, /nas/pools, /nas/cache-drives, /nas/source-stream) has no require_auth, any session could rewrite storage policy regardless of AML role. Added a role gate on the six config-write routes, resolved from /aml/users/me at login and failing closed on an unknown role. Also from the same round: reject dot-only identifiers (_POOL_ID_ALLOWED permitted ".", so ".." matched and collapsed a URL segment); validate volume_group, the one field reaching the backend unchecked; stop echoing non-JSON upstream bodies into the browser; treat 3xx as an error rather than crashing on .json(); cap the pre-auth login bucket and add an IP-only counter so spraying one password across many usernames is throttled; revoke the AML session on logout; reject backslash in redirect targets instead of relying on Werkzeug's escaping; constrain the reflected Host used in copy-paste mount commands; and stop bare int() on backend values from 500ing a page. A native-token mismatch now aborts 502 naming the remedy instead of clearing the session -- the dual-credential scheme made that 401 indistinguishable from session expiry, which produced an unbreakable login loop. Every guard was mutation-checked: 13 of 13 produce a failing test when removed. Two of the first drafts were vacuous and were rewritten until they failed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013S1qbQs31YSox1azQChqs2
Adds the four MUST-FIX findings and their fixes, the ten lesser items, the areas the reviewer attacked and found clean, and the mutation-check result (13/13). Also records the one recommendation deliberately NOT taken: blocking is_private would have refused every real LAN tape library, so the address check blocks loopback/link-local/unspecified/multicast/reserved and allows RFC1918. The defect was that the old guard compared host spellings, not that its scope was wrong. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013S1qbQs31YSox1azQChqs2
Review finding. `POST /restore/tree` ran a minutes-long restore directly on the event loop, which stalls every other request in the process: `/health` (the container health check allows 5s × 5 retries, so the container goes unhealthy), the AML emulator parity surface, and the caller's own request — nginx gives up at `proxy_read_timeout 120s` and returns 504 while the restore keeps going, so the operator sees a failed restore that actually succeeded and a retry runs it twice. The thread-safety argument in the old comment was not a reason to stay on the loop: routes_assist.py already establishes the pattern of handing the worker its own `CatalogRepository(get_session())` instead of the process-global Session. Also from review: log the full exception server-side (the other half of safe_job_error's contract), and let a typed OpenBladeError keep the status the app's handler gives it instead of flattening an unsafe-catalog-path 400 into a 500. The new test asserts the restore does not run on the loop thread by checking `asyncio.get_running_loop()` from inside it, and is mutation-checked. NOTE: the first version of that test used `threading.main_thread()` and passed with the fix reverted — the TestClient drives the app from its own portal thread. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013S1qbQs31YSox1azQChqs2
Review finding, two leaks on one route: - `read_tape_alerts` appends the failing tool's own stderr to its reason; that was returned verbatim in a 200. The exit code stays (it is the actionable half), the tool text is logged server-side instead. - `sg_inq`'s `raise_on_error()` raises `CommandError`, which is not an OpenBladeError and so escaped as a bare 500 whose message is the full argv plus stderr. Now a curated 502, with the exception logged. Both are mutation-checked. The "read-only" test no longer relies on FastAPI's free 405s either: it captures every argv at the SafeRunner boundary with dry-run off and asserts exactly `sg_inq <dev>` and `sg_logs -p 0x2e <dev>`. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013S1qbQs31YSox1azQChqs2
Review finding. `assess_export` answers "carries nothing" for a barcode that does not exist — true, and useless: rendered next to an Export button, a typo reads as "safe to export". The preview now 404s unless the barcode is in the library's inventory or the catalog, and the path parameter is length-bounded like the POST body's already was. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013S1qbQs31YSox1azQChqs2
Review finding, and the worst one on the frontend: the "Force export…" button
lived inside the refusal block but read the barcode field at click time, and the
refusal panel outlived an edit of that field. So typing ARC001L8, being refused,
then changing the field to SCR002L8 left a force button that would arm the dialog
for a cartridge the server had never assessed or refused — and the dialog enabled
"Export anyway" on the typed barcode alone, which also meant it could confirm with
no data-loss panel on screen at all whenever the preview was slow or had failed.
Now: the refused barcode is tracked separately and is the only thing force can
target, editing the field resets the mutation, and the dialog requires a loaded
assessment *for that barcode* before confirm is enabled — refusing with "an
unknown payload is not an empty one" and a Check-again button otherwise.
Both gates are mutation-checked. Also fixed: selecting the I/E placeholder option
produced `Number('') === 0`, a real element id, so Import fired for slot 0.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013S1qbQs31YSox1azQChqs2
Review finding (same class as the export one). Editing the prefix or destination left the previous plan/run panel on screen, and a completed run left "Run restore" enabled — one stray click from restoring the same tree twice, or reading a result for a destination that is no longer in the form. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013S1qbQs31YSox1azQChqs2
Review finding. On 429 or 502 the typed question was lost and an unanswered user turn stayed in the thread with no way to retry it. Popping the orphaned turn and restoring the draft is right for every failure; only the setup panel is 503-specific. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013S1qbQs31YSox1azQChqs2
… hunks The BLE branch's final commit accidentally swept another agent's stash-pop residue (committed conflict markers) into ci.yml, both emulator workflows and README. Its legitimate content (ingest logging, hardware-fixture catch, tests) is kept; the four contaminated files resolve to the already-merged clean versions (flask branch's web-flask wiring + origin/master workflows).
…t it Self-review of the previous commit. The `[^)]*\)` pattern stopped at the first closing parenthesis, so a tool message that contained one — `log_sense: field in cdb illegal (rc=5), /dev/sg1 dsn=secret` — failed to match and the whole stderr went on the wire after all. The suffix is appended last by `read_tape_alerts`, so the pattern is now anchored to the end of the string with DOTALL. The test fixture carries the nested parenthesis, and reverting to the old pattern fails it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013S1qbQs31YSox1azQChqs2
…eAlert reason at the source Two parallel-agent reconciliations: the parity route/UI still spoke boolean force while the service demands confirm_barcode (guaranteed TypeError) — the typed-barcode dialog now IS the wire confirmation, mistyped barcodes 409. The drive-health reason regex (reviewer-repro'd leaking device paths/DSNs on parenthesized sg3_utils stderr) is deleted; tool text lives in reason_detail (log/CLI only) and reason is curated at construction, end-to-end tested with hostile stderr through the real read_tape_alerts. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013S1qbQs31YSox1azQChqs2
…rives+I/E; backend-lint gates through ci-gate Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013S1qbQs31YSox1azQChqs2
…Y_003, self-inflicted) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013S1qbQs31YSox1azQChqs2
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the six gaps from the post-#44 audit, eight parallel workstreams integrated:
Integration additions: TapeAlert reason curated at the source (reviewer-repro'd paren leak), tree-restore serialized under the archive media lock, mailslot known-check covers drives+I/E.
Local gates: unit+safety+integration 2,347+ passed / 0 failed · i3 142/142 both auth modes · live mhvtl suite 54/0 · frontend 68 tests + build · ruff+format clean · mypy 0/169 files.
🤖 Generated with Claude Code
https://claude.ai/code/session_013S1qbQs31YSox1azQChqs2