From 3bac08db893124900ffb7e2882aa1bf4dcca96b0 Mon Sep 17 00:00:00 2001 From: yoyoliuuu Date: Mon, 7 Sep 2026 00:53:13 -0400 Subject: [PATCH 1/2] feat: accept an edge-verified identity, and make the page prefix-aware Makes a submission attributable to a signed-in person instead of a name somebody typed, by letting this service sit behind the lab's single Caddy edge. The companion route and the framed panel are in ac-organic-lab. Why the edge and not a login here: a session cookie cannot be shared with this gateway on its own address -- raw 100.x addresses cannot carry a `Domain` cookie and *.ts.net is on the Public Suffix List, so browsers drop tailnet-wide cookies (AUTH_DESIGN, "Why sessions can't be shared per-host"). One origin behind the edge is the only arrangement that yields one login, so a page served on this port is architecturally excluded from SSO. ## identity.py Trusts the edge's injected `X-Auth-User` only when the request also carries `X-Edge-Auth` matching `BAMBU_EDGE_SHARED_SECRET` -- something a caller coming straight off the tailnet cannot produce, which matters because this port stays directly reachable. Same mechanism as the xArm's arrangement. Three properties worth reviewing closely, since this is the security-relevant part: - **Fails closed.** No configured secret means no trusted identity, ever -- never "believe the header because we have nothing to check it against". A deployment with no secret behaves exactly as before this commit, which is what makes shipping ahead of deployment safe. - **Constant-time comparison** (`hmac.compare_digest`): `==` on a secret leaks it a byte at a time. - **A verified identity overrides any client-supplied name.** Otherwise a signed-in person could file work under someone else's name. An edge that proves itself but names nobody is anonymous, not authenticated. ## What it records `requested_by_verified` / `approved_by_verified` on every job, and a "(verified identity)" marker in the history note. The distinction is stored per job rather than inferred from how the service happened to be deployed when the job arrived. `GET /whoami` reports what the current request carries, so the page can word itself honestly -- `identity_available` separates "not signed in" from "this deployment cannot tell who you are". It echoes nothing the caller did not already present, and never the secret. ## Prefix-aware page The page derives its API base by stripping the trailing `/ui` from its own URL, so one file serves both the direct deployment (base "") and any edge prefix (`/bambu`), with no server-side rewrite and no build-time config -- the same arrangement as the OT-2 operator SPA. Hardcoded `/printers` would have reached the dashboard instead of this gateway. It also answers at both `/ui` and `/ui/`: serving only one would make Starlette redirect between them with a `Location` that drops the edge prefix, landing the visitor on the dashboard. That is the trap the /xarm5 edge block documents. When an identity is verified the page shows the signed-in account and stops offering a name field; the "no sign-in" wording is only shown when it is true. ## Not deployed Needs root: install the Caddy route, set the same secret on both sides, restart both. The bind stays 0.0.0.0 until then -- reverting it to loopback before the edge route exists would break the page that currently works. Sequence recorded in docs/TODO.md. `uv run ruff check .` and `uv run pytest -q` (152 tests, 22 new) pass. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01UQvsfEeDitEyNzbCwrcEdD --- .env.example | 6 + README.md | 53 ++++++++- docs/TODO.md | 38 ++++++- src/bambu_server/config.py | 16 +++ src/bambu_server/identity.py | 105 +++++++++++++++++ src/bambu_server/main.py | 104 +++++++++++++++-- src/bambu_server/static/index.html | 62 ++++++++-- src/bambu_server/submissions.py | 30 ++++- tests/test_identity.py | 103 +++++++++++++++++ tests/test_submission_api.py | 175 +++++++++++++++++++++++++++++ 10 files changed, 665 insertions(+), 27 deletions(-) create mode 100644 src/bambu_server/identity.py create mode 100644 tests/test_identity.py diff --git a/.env.example b/.env.example index 875db19..f2d1f36 100644 --- a/.env.example +++ b/.env.example @@ -8,3 +8,9 @@ BAMBU_SERVER_PORT=8012 BAMBU_X1C_01_HOST=192.0.2.10 BAMBU_X1C_01_ACCESS_CODE=replace-me BAMBU_X1C_01_SERIAL=replace-me + +# Shared secret proving a request came through the lab's Caddy edge rather than +# straight off the tailnet. Set the SAME value in the edge's EnvironmentFile. +# When it is set, the edge's injected X-Auth-User becomes the recorded submitter +# instead of a typed-in label. Leave it unset and no identity is ever trusted. +#BAMBU_EDGE_SHARED_SECRET= diff --git a/README.md b/README.md index 5b99883..fb6f22d 100644 --- a/README.md +++ b/README.md @@ -83,6 +83,7 @@ Gateway routes: | GET | `/printers` | Safe printer inventory (no addresses or credentials) | | GET | `/status` | Aggregate gateway envelope (one component per printer) | | GET | `/ui` | Submission page for people (see below) | +| GET | `/whoami` | Whether this request carries an edge-verified identity | Per-printer STATUS_SPEC routes: @@ -288,15 +289,57 @@ mirrored to one JSON file each, so a restart does not empty a machine's queue. ### Identity and approval -`requested_by` and `approved_by` are **opaque identifiers, not authenticated -identities**. This service has no login; access is gated at the network layer by -Tailscale ACLs, exactly as for the status surface. They are recorded in the job's -history so decisions become attributable the moment a real identity provider -(`ac_auth`) is wired in. +This service has no login of its own, so who did what depends on how it is +reached, and every job records which of the two it got: + +- **Through the lab's Caddy edge** (`/bambu/*` — see *Behind the dashboard's + login* below), the edge authenticates the person against `ac_auth` and injects + `X-Auth-User`. The gateway believes that header **only** when the request also + carries `X-Edge-Auth` matching `BAMBU_EDGE_SHARED_SECRET`, which a caller + coming straight off the tailnet cannot produce. The signed-in account becomes + the recorded actor, overriding anything the client supplied — a signed-in + person must not be able to file work under someone else's name — and + `requested_by_verified` / `approved_by_verified` are `true`. +- **Reached directly**, `requested_by` / `approved_by` / `cancelled_by` are + **opaque labels, not identities**, exactly as the status surface is + unauthenticated. The `*_verified` fields are `false`. + +Two properties are deliberate. The gateway **fails closed**: with no +`BAMBU_EDGE_SHARED_SECRET` configured it trusts no injected identity at all, +rather than believing a header it cannot check. And the secret is compared in +constant time, because `==` on a secret leaks it a byte at a time. + +`GET /whoami` reports what the current request carries, which is how the page +words itself honestly — it distinguishes "not signed in" from "this deployment +cannot tell who you are" (`identity_available`). Approval is human-in-the-loop by design: nothing auto-approves, and a submission that did not pass validation can never be approved. +### Behind the dashboard's login + +The page is served at `/ui` relative to wherever the service is reached, and it +derives its API base by stripping that trailing `/ui` from its own URL. One file +therefore serves both the direct deployment and a path prefix behind the lab's +single Caddy edge, with no server-side rewrite and no build-time config — the +same arrangement as the OT-2 operator SPA. + +Fronting it that way is the **only** way it participates in SSO: a session +cookie cannot be shared with this gateway on its own address, because raw +`100.x` addresses cannot carry a `Domain` cookie and `*.ts.net` is on the Public +Suffix List, so browsers drop tailnet-wide cookies. One origin behind the edge +means one login (see `ac-organic-lab/docs/AUTH_DESIGN.md`). + +The route lives in `ac-organic-lab/deploy/Caddyfile.single-edge` as `/bambu/*`, +gated by `forward_auth` and injecting the identity described above; the dashboard +frames `/bambu/ui/` under Utils → 3D Printers. Once that route is live the +service's bind can go back to loopback, closing the unauthenticated +`/submissions` path on the tailnet. + +Note the page answers at both `/ui` and `/ui/`. Serving only one would make +Starlette redirect between them with a `Location` that drops the edge prefix, +landing the visitor on the dashboard. + ### Cancelling `POST /submissions/{id}/cancel` withdraws a waiting job. It is a **queue diff --git a/docs/TODO.md b/docs/TODO.md index 4b4f862..2137317 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -104,10 +104,46 @@ before pointing users at it. The durable home is probably the lab dashboard (`ac-organic-lab/web`) once `ac_auth` makes `requested_by` a real identity; this page is the interim surface. +## Edge identity / SSO (code done, deployment pending) + +The submission page and its API now work behind the lab's single Caddy edge, so +a submission can be attributed to a signed-in person instead of a typed-in +label. What shipped: + +- `identity.py` — trusts `X-Auth-User` only when `X-Edge-Auth` matches + `BAMBU_EDGE_SHARED_SECRET` (constant-time), **fails closed** with no secret + configured, and a verified identity overrides any client-supplied name. +- `requested_by_verified` / `approved_by_verified` on every job; the history + note marks a verified actor. +- `GET /whoami` so the page can word itself honestly. +- The page derives its API base from its own URL (strips a trailing `/ui`), so + one file serves the direct deployment and any edge prefix. Answers at both + `/ui` and `/ui/` — a slash redirect would drop the edge prefix. +- `ac-organic-lab`: the `/bambu/*` route in `deploy/Caddyfile.single-edge`, and + Utils → 3D Printers frames `/bambu/ui/`. + +**Not deployed.** Three root steps, none of which I can do: + +1. Install the updated `deploy/Caddyfile.single-edge` into `/etc/caddy` and + reload Caddy. +2. Set the *same* `BAMBU_EDGE_SHARED_SECRET` in Caddy's systemd + `EnvironmentFile` and in bambu-server's unit environment, then restart both. + Until then the embed shows a blank frame and the gateway trusts nothing — + both fail closed, which is why shipping this ahead of deployment is safe. +3. **Then** revert the bind to `127.0.0.1` (step 5 of the plan). It was widened + to `0.0.0.0` so the page was reachable at all; once the edge fronts it, the + loopback bind closes the unauthenticated `/submissions` path on the tailnet. + Doing it before the route exists would break the working page. + +Known gap, inherited from the OT-2 embed: a write inside the framed panel +bypasses the dashboard's `control_action` audit row. Submissions are recorded in +the job store's history, so there is a trail; it is not in `equipment_events` +until the gateway pushes to `/api/ingest/events`. + ## Test suite - `uv run ruff check .` passes. -- `uv run pytest -q` passes all 130 tests, including the FastAPI API tests and +- `uv run pytest -q` passes all 152 tests, including the FastAPI API tests and the submission pipeline (artifact inspection, validation, store/state machine, queue ETA, HTTP surface). Tests build their own `.3mf` and `.gcode` fixtures and use fake backends; nothing touches hardware. diff --git a/src/bambu_server/config.py b/src/bambu_server/config.py index bd33400..4205f0d 100644 --- a/src/bambu_server/config.py +++ b/src/bambu_server/config.py @@ -144,6 +144,22 @@ class PrinterCredentials(BaseModel): serial: SecretStr +#: Env var holding the secret the lab's Caddy edge presents on every proxied +#: request. Set the *same* value here and in the edge's EnvironmentFile. +EDGE_SECRET_ENV = "BAMBU_EDGE_SHARED_SECRET" + + +def resolve_edge_secret() -> str | None: + """The shared secret that lets this service trust an injected identity. + + Read from the environment, never from the YAML: it is a credential, and + `printers.local.yaml` is a config file people paste into issues. Absent + means no identity is ever trusted (see :mod:`bambu_server.identity`). + """ + + return (os.getenv(EDGE_SECRET_ENV) or "").strip() or None + + def resolve_credentials(printer: PrinterDefinition) -> PrinterCredentials: names = { "host": f"{printer.env_prefix}_HOST", diff --git a/src/bambu_server/identity.py b/src/bambu_server/identity.py new file mode 100644 index 0000000..fe877cf --- /dev/null +++ b/src/bambu_server/identity.py @@ -0,0 +1,105 @@ +"""Operator identity injected by a trusted edge. + +This service has no login of its own. When it is fronted by the lab's single +Caddy edge, that edge authenticates the person (``forward_auth`` against +``ac_auth``) and injects who they are as request headers. This module decides +whether to believe those headers. + +The rule, copied from the xArm's arrangement because the reasoning is the same: +the injected identity is trusted **only** when the request also carries a shared +secret that the edge holds and a direct caller cannot produce. The gateway's +port stays reachable on the tailnet, so without that check anyone could claim to +be anyone by setting a header. + +Two properties matter more than convenience here: + +* **Fail closed.** No configured secret means no trusted identity, ever -- + never "trust the header because we have nothing to check it against". A + deployment that has not been given a secret behaves exactly as it did before + this module existed. +* **Constant-time comparison.** Comparing secrets with ``==`` leaks their + contents through timing, one byte at a time. +""" + +from __future__ import annotations + +import hmac +import re + +from pydantic import BaseModel + +#: Injected by the edge after it has authenticated the person. +USER_HEADER = "X-Auth-User" +ROLE_HEADER = "X-Auth-Role" +#: Proof that the request came through the edge and not straight off the tailnet. +EDGE_AUTH_HEADER = "X-Edge-Auth" + +_CONTROL_CHARS = re.compile(r"[\x00-\x1f\x7f]") +_MAX_LEN = 120 + + +class Actor(BaseModel): + """Who the service believes is making a request. + + ``verified`` is the whole point: an unverified actor is a self-declared + label, and the difference is recorded on every job rather than blurred. + """ + + user: str | None = None + role: str | None = None + verified: bool = False + + +ANONYMOUS = Actor() + + +def _clean(value: str | None) -> str | None: + if not value: + return None + cleaned = _CONTROL_CHARS.sub("", value).strip()[:_MAX_LEN] + return cleaned or None + + +def resolve_actor(headers: object, *, edge_secret: str | None) -> Actor: + """Resolve the caller from request headers. + + ``headers`` is anything with a case-insensitive ``get`` (Starlette's + ``Headers``). Returns :data:`ANONYMOUS` unless an edge-verified identity is + present, so a caller can only ever *gain* attribution by coming through the + edge -- never lose a check by omitting a header. + """ + + if not edge_secret: + # Nothing to verify against. Deliberately not "trust the header". + return ANONYMOUS + + getter = getattr(headers, "get", None) + if getter is None: # pragma: no cover - defensive + return ANONYMOUS + + presented = getter(EDGE_AUTH_HEADER) + if not presented or not hmac.compare_digest(str(presented), edge_secret): + return ANONYMOUS + + user = _clean(getter(USER_HEADER)) + if user is None: + # The edge proved itself but named nobody: an authenticated request with + # no subject is not an identity. + return ANONYMOUS + return Actor(user=user, role=_clean(getter(ROLE_HEADER)), verified=True) + + +def actor_for(actor: Actor, supplied: str | None) -> tuple[str, bool]: + """Choose the name to record, and say whether it was verified. + + A verified identity always wins over whatever the client typed -- otherwise + a signed-in person could file work under someone else's name. Without one, + the supplied label is used and marked unverified. + """ + + if actor.verified and actor.user: + return actor.user, True + cleaned = _clean(supplied) + if cleaned is None: + raise ValueError("no actor supplied and no verified identity available") + return cleaned, False diff --git a/src/bambu_server/main.py b/src/bambu_server/main.py index 2334a60..26c6943 100644 --- a/src/bambu_server/main.py +++ b/src/bambu_server/main.py @@ -22,7 +22,17 @@ from pathlib import PurePosixPath from typing import Annotated -from fastapi import Depends, FastAPI, File, Form, HTTPException, Path, Query, UploadFile +from fastapi import ( + Depends, + FastAPI, + File, + Form, + HTTPException, + Path, + Query, + Request, + UploadFile, +) from fastapi.middleware.cors import CORSMiddleware from fastapi.responses import HTMLResponse from pydantic import BaseModel, Field @@ -36,7 +46,9 @@ Settings, load_settings, resolve_credentials, + resolve_edge_secret, ) +from .identity import Actor, actor_for, resolve_actor from .models import ( PROTOCOL_VERSION, ComponentStatus, @@ -78,13 +90,16 @@ class ApprovalRequest(BaseModel): """Sign-off recorded against a queued submission. - ``approved_by`` is an opaque identifier, **not** an authenticated identity: - this service has no login, and access is gated at the network layer. It is - recorded in the job's history so the decision is attributable once a real - identity provider is wired in. + When the request arrives through the lab's authenticating edge, the + signed-in identity is used and this field is ignored — a signed-in person + must not be able to file a decision under someone else's name. Without a + verified identity it is an opaque label, and the job records which of the + two it got. """ - approved_by: str = Field(min_length=1, max_length=120) + #: Optional: ignored when the edge supplies a verified identity, and + #: required only when it does not. + approved_by: str | None = Field(default=None, min_length=1, max_length=120) class CancellationRequest(BaseModel): @@ -94,10 +109,26 @@ class CancellationRequest(BaseModel): is free text kept in the job's history so a withdrawal is explicable later. """ - cancelled_by: str = Field(min_length=1, max_length=120) + cancelled_by: str | None = Field(default=None, min_length=1, max_length=120) reason: str | None = Field(default=None, max_length=500) +class Whoami(BaseModel): + """Who the service thinks you are, for the page to render honestly. + + The page shows a name field when nobody is verified and the signed-in user + when someone is; without this it would have to guess how it was deployed. + """ + + user: str | None = None + role: str | None = None + verified: bool = False + #: True when this deployment is capable of verifying an identity at all + #: (a secret is configured). Distinguishes "not signed in" from "this + #: service cannot tell who you are". + identity_available: bool = False + + def create_app( *, settings: Settings | None = None, @@ -107,11 +138,16 @@ def create_app( # One-slot holder rather than a module global: `create_app` may be called # more than once in a process (tests do), and each app owns its own store. stores: dict[str, SubmissionStore] = {} + # Resolved once at startup, not per request: it is a process-level + # credential, and re-reading the environment per call would let a later + # change silently alter who the service trusts mid-run. + secrets: dict[str, str | None] = {"edge": None} @asynccontextmanager async def lifespan(app: FastAPI) -> AsyncIterator[None]: active_settings = settings or load_settings() app.state.settings = active_settings + secrets["edge"] = resolve_edge_secret() store = SubmissionStore(active_settings.submissions) await asyncio.to_thread(store.load) stores["default"] = store @@ -162,6 +198,11 @@ def get_monitor( raise HTTPException(status_code=404, detail="printer not configured") return monitor + def get_actor(request: Request) -> Actor: + """The caller, as far as the trusted edge will vouch for them.""" + + return resolve_actor(request.headers, edge_secret=secrets["edge"]) + def get_store() -> SubmissionStore: store = stores.get("default") if store is None: # pragma: no cover - only outside the app lifespan @@ -176,7 +217,13 @@ async def gateway_info() -> GatewayInfo: printer_count=len(monitors), ) + # Registered at both spellings on purpose. Behind an edge path prefix the + # canonical URL is `/ui/`, and serving only `/ui` would make + # Starlette answer the trailing-slash form with a redirect to `/ui` -- + # whose Location drops the prefix, landing the visitor on the dashboard. + # (The same trap the xArm's /web/ route documents in the edge config.) @app.get("/ui", response_class=HTMLResponse, include_in_schema=False, tags=["gateway"]) + @app.get("/ui/", response_class=HTMLResponse, include_in_schema=False, tags=["gateway"]) async def submission_ui() -> HTMLResponse: """The operator/submitter page. @@ -229,6 +276,21 @@ async def gateway_status() -> EquipmentStatus: details={"monitoring_only": True, "printer_count": len(monitors)}, ) + @app.get("/whoami", response_model=Whoami, tags=["gateway"]) + async def whoami(actor: Annotated[Actor, Depends(get_actor)]) -> Whoami: + """Report the caller's verified identity, if the edge supplied one. + + Never echoes the shared secret, and says nothing a caller did not + already present. + """ + + return Whoami( + user=actor.user, + role=actor.role, + verified=actor.verified, + identity_available=secrets["edge"] is not None, + ) + @app.get("/printers", response_model=list[PrinterSummary], tags=["gateway"]) async def list_printers() -> list[PrinterSummary]: return [ @@ -316,8 +378,9 @@ async def printer_queue( async def create_submission( store: Annotated[SubmissionStore, Depends(get_store)], file: Annotated[UploadFile, File(description="A .3mf or .gcode artifact")], + actor: Annotated[Actor, Depends(get_actor)], target_machine: Annotated[str, Form(max_length=120)], - requested_by: Annotated[str, Form(max_length=120)], + requested_by: Annotated[str | None, Form(max_length=120)] = None, material: Annotated[str | None, Form(max_length=60)] = None, ) -> SubmissionJob: """Accept a print artifact, validate it, and queue it if it passes. @@ -327,6 +390,13 @@ async def create_submission( thread so a large artifact does not stall the status poll loop. """ + # A verified identity wins over the form field: a signed-in person must + # not be able to submit under someone else's name. + try: + owner, owner_verified = actor_for(actor, requested_by) + except ValueError as exc: + raise HTTPException(status_code=422, detail="requested_by is required") from exc + monitor = monitors.get(target_machine) if monitor is None: raise HTTPException(status_code=404, detail="unknown target machine") @@ -361,7 +431,8 @@ async def chunks() -> AsyncIterator[bytes]: chunks=chunks(), extension=extension, target_machine=target_machine, - requested_by=requested_by, + requested_by=owner, + requested_by_verified=owner_verified, material=material, original_filename=file.filename or "", ) @@ -402,6 +473,7 @@ async def read_submission( ) async def approve_submission( store: Annotated[SubmissionStore, Depends(get_store)], + actor: Annotated[Actor, Depends(get_actor)], submission_id: Annotated[str, Path(pattern=r"^[0-9a-f]{32}$")], approval: ApprovalRequest, ) -> SubmissionJob: @@ -416,7 +488,11 @@ async def approve_submission( if job is None: raise HTTPException(status_code=404, detail="unknown submission") try: - return await store.approve(job, approved_by=approval.approved_by) + who, verified = actor_for(actor, approval.approved_by) + except ValueError as exc: + raise HTTPException(status_code=422, detail="approved_by is required") from exc + try: + return await store.approve(job, approved_by=who, verified=verified) except InvalidTransition as exc: raise HTTPException(status_code=409, detail=str(exc)) from exc @@ -427,6 +503,7 @@ async def approve_submission( ) async def cancel_submission( store: Annotated[SubmissionStore, Depends(get_store)], + actor: Annotated[Actor, Depends(get_actor)], submission_id: Annotated[str, Path(pattern=r"^[0-9a-f]{32}$")], cancellation: CancellationRequest, ) -> SubmissionJob: @@ -440,11 +517,16 @@ async def cancel_submission( job = store.get(submission_id) if job is None: raise HTTPException(status_code=404, detail="unknown submission") + try: + who, verified = actor_for(actor, cancellation.cancelled_by) + except ValueError as exc: + raise HTTPException(status_code=422, detail="cancelled_by is required") from exc try: return await store.cancel( job, - cancelled_by=cancellation.cancelled_by, + cancelled_by=who, reason=cancellation.reason, + verified=verified, ) except InvalidTransition as exc: raise HTTPException(status_code=409, detail=str(exc)) from exc diff --git a/src/bambu_server/static/index.html b/src/bambu_server/static/index.html index eaa6dc6..b8c1d4a 100644 --- a/src/bambu_server/static/index.html +++ b/src/bambu_server/static/index.html @@ -124,8 +124,16 @@

Queue

return node; }; +// This page is served at /ui, and everything it calls lives under the +// same . Behind the lab's Caddy edge that base is a path prefix +// (/bambu), so a hardcoded "/printers" would reach the dashboard instead of +// this gateway. Deriving it from our own URL means one file serves both the +// direct deployment (base "") and any edge prefix, with no server-side rewrite +// and no build-time configuration. +const API_BASE = window.location.pathname.replace(/\/ui\/?$/, ""); + async function api(path, options) { - const response = await fetch(path, options); + const response = await fetch(API_BASE + path, options); let body = null; try { body = await response.json(); } catch (_) { /* no body */ } if (!response.ok) { @@ -135,7 +143,41 @@

Queue

return body; } -const state = { machine: null, profile: null }; +const state = { machine: null, profile: null, actor: null }; + +/** Show who the edge says you are, or admit that nobody checked. */ +async function loadIdentity() { + let who; + try { + who = await api("/whoami"); + } catch (_) { + return; + } + state.actor = who; + const field = $("requested_by"); + const label = document.querySelector('label[for="requested_by"]'); + const banner = $("banner"); + + if (who.verified && who.user) { + field.value = who.user; + field.readOnly = true; + field.disabled = true; + if (label) label.textContent = "Signed in as"; + banner.replaceChildren( + el("span", "Signed in as " + who.user + (who.role ? " (" + who.role + ")" : "") + + " — submissions are recorded against this account. "), + ); + } else if (who.identity_available) { + banner.replaceChildren( + el("span", "Not signed in: the name you enter is a label, not an identity. " + + "Open this page through the dashboard to have it recorded against your account. "), + ); + } else { + return; // Keep the page's default no-sign-in wording. + } + const note = el("strong", "Nothing is sent to a printer"); + banner.append(note, el("span", " — submitting queues a job; dispatch is not implemented.")); +} function renderSpecs(profile) { const specs = $("specs"); @@ -197,10 +239,14 @@

Queue

} async function act(id, path, field) { - const who = $("requested_by").value.trim() || window.prompt("Your name, for the record:"); - if (!who) return; const body = {}; - body[field] = who; + // With a verified identity the server ignores any name we send and uses the + // signed-in account, so there is nothing to ask for. + if (!(state.actor && state.actor.verified)) { + const who = $("requested_by").value.trim() || window.prompt("Your name, for the record:"); + if (!who) return; + body[field] = who; + } if (field === "cancelled_by") { const reason = window.prompt("Reason (optional):"); if (reason) body.reason = reason; @@ -302,15 +348,16 @@

Queue

const button = $("submit"); const message = $("submit-msg"); const file = $("file").files[0]; + const verified = Boolean(state.actor && state.actor.verified); const who = $("requested_by").value.trim(); message.className = "detail"; if (!file) { message.textContent = "Choose a file first."; return; } - if (!who) { message.textContent = "Enter your name first."; return; } + if (!verified && !who) { message.textContent = "Enter your name first."; return; } const form = new FormData(); form.append("file", file); form.append("target_machine", state.machine); - form.append("requested_by", who); + if (!verified) form.append("requested_by", who); const material = $("material").value.trim(); if (material) form.append("material", material); @@ -331,6 +378,7 @@

Queue

} (async function start() { + await loadIdentity(); try { const printers = await api("/printers"); const picker = $("machine"); diff --git a/src/bambu_server/submissions.py b/src/bambu_server/submissions.py index ccf5a69..974d33a 100644 --- a/src/bambu_server/submissions.py +++ b/src/bambu_server/submissions.py @@ -126,6 +126,16 @@ def clean_text(value: str | None, *, field: str, required: bool) -> str | None: return cleaned +def _verified_suffix(verified: bool) -> str: + """Mark a verified actor in free-text history. + + Structured fields carry a boolean; the history note is read by people, and + "who did this, and did we actually check" is the part worth spelling out. + """ + + return " (verified identity)" if verified else "" + + def safe_display_name(name: str) -> str: """Reduce a client-supplied filename to something safe to echo back. @@ -152,6 +162,11 @@ class SubmissionJob(BaseModel): submission_id: str target_machine: str requested_by: str = Field(min_length=1, max_length=120) + #: True when `requested_by` is an edge-verified identity rather than a name + #: the client typed. Recorded per job so a reader can tell an attributable + #: submission from a self-declared one, instead of having to know how the + #: service happened to be deployed when it arrived. + requested_by_verified: bool = False material: str | None = Field(default=None, max_length=60) original_filename: str artifact_kind: ArtifactKind @@ -161,6 +176,7 @@ class SubmissionJob(BaseModel): created_at: datetime updated_at: datetime approved_by: str | None = None + approved_by_verified: bool = False approved_at: datetime | None = None #: True once the stored artifact has been deleted (cancellation). The job #: record outlives its file so the audit trail survives, but nothing can be @@ -259,6 +275,7 @@ async def accept( extension: str, target_machine: str, requested_by: str, + requested_by_verified: bool = False, material: str | None, original_filename: str, ) -> SubmissionJob: @@ -304,6 +321,7 @@ async def accept( submission_id=submission_id, target_machine=target_machine, requested_by=owner, # type: ignore[arg-type] + requested_by_verified=requested_by_verified, material=filament, original_filename=safe_display_name(original_filename), artifact_kind=kind, @@ -347,7 +365,9 @@ async def record_validation( await self._persist(updated) return updated - async def approve(self, job: SubmissionJob, *, approved_by: str) -> SubmissionJob: + async def approve( + self, job: SubmissionJob, *, approved_by: str, verified: bool = False + ) -> SubmissionJob: """Record the human sign-off that gates dispatch. Approval is a *record*, not an action: it moves no hardware and starts @@ -372,12 +392,15 @@ async def approve(self, job: SubmissionJob, *, approved_by: str) -> SubmissionJo update={ "verdict": verdict, "approved_by": _CONTROL_CHARS.sub("", approved_by).strip()[:120], + "approved_by_verified": verified, "approved_at": now, } ) self._jobs[updated.submission_id] = updated return await self._transition_locked( - updated.submission_id, "approved", f"approved by {updated.approved_by}" + updated.submission_id, + "approved", + f"approved by {updated.approved_by}{_verified_suffix(verified)}", ) async def cancel( @@ -386,6 +409,7 @@ async def cancel( *, cancelled_by: str, reason: str | None = None, + verified: bool = False, ) -> SubmissionJob: """Withdraw a waiting job from its machine's queue. @@ -423,7 +447,7 @@ async def cancel( ) self._jobs[current.submission_id] = current.model_copy(update=update) - note = f"cancelled by {who}" + note = f"cancelled by {who}{_verified_suffix(verified)}" if why: note = f"{note}: {why}" return await self._transition_locked( diff --git a/tests/test_identity.py b/tests/test_identity.py new file mode 100644 index 0000000..ab445ff --- /dev/null +++ b/tests/test_identity.py @@ -0,0 +1,103 @@ +"""Edge-injected identity (bambu_server.identity).""" + +from __future__ import annotations + +import pytest +from starlette.datastructures import Headers + +from bambu_server.identity import ( + ANONYMOUS, + EDGE_AUTH_HEADER, + ROLE_HEADER, + USER_HEADER, + Actor, + actor_for, + resolve_actor, +) + +SECRET = "edge-secret-value" + + +def _headers(**values: str) -> Headers: + return Headers({k.replace("_", "-"): v for k, v in values.items()}) + + +def _edge(user: str = "alice", role: str | None = "operator", secret: str = SECRET) -> Headers: + raw = {EDGE_AUTH_HEADER: secret, USER_HEADER: user} + if role: + raw[ROLE_HEADER] = role + return Headers(raw) + + +def test_a_verified_edge_request_yields_an_identity() -> None: + actor = resolve_actor(_edge(), edge_secret=SECRET) + + assert actor == Actor(user="alice", role="operator", verified=True) + + +def test_no_configured_secret_trusts_nothing() -> None: + """Fail closed: nothing to verify against must never mean "believe it".""" + for secret in (None, "", " "): + assert resolve_actor(_edge(), edge_secret=secret) == ANONYMOUS + + +def test_a_wrong_or_missing_edge_secret_is_anonymous() -> None: + assert resolve_actor(_edge(secret="wrong"), edge_secret=SECRET) == ANONYMOUS + assert resolve_actor( + _headers(**{USER_HEADER: "alice"}), edge_secret=SECRET + ) == ANONYMOUS + + +def test_a_header_claim_without_the_edge_secret_is_ignored() -> None: + """The port is reachable on the tailnet, so a bare header proves nothing.""" + forged = Headers({USER_HEADER: "admin", ROLE_HEADER: "admin"}) + + assert resolve_actor(forged, edge_secret=SECRET) == ANONYMOUS + + +def test_a_verified_edge_naming_nobody_is_anonymous() -> None: + """An authenticated request with no subject is not an identity.""" + for user in ("", " ", "\x00\x01"): + headers = Headers({EDGE_AUTH_HEADER: SECRET, USER_HEADER: user}) + assert resolve_actor(headers, edge_secret=SECRET) == ANONYMOUS + + +def test_the_role_is_optional() -> None: + actor = resolve_actor(_edge(role=None), edge_secret=SECRET) + + assert actor.verified is True + assert actor.user == "alice" + assert actor.role is None + + +def test_control_characters_are_stripped_and_length_capped() -> None: + headers = Headers({EDGE_AUTH_HEADER: SECRET, USER_HEADER: "a\x00lice" + "x" * 400}) + actor = resolve_actor(headers, edge_secret=SECRET) + + assert actor.user is not None + assert "\x00" not in actor.user + assert actor.user.startswith("alice") + assert len(actor.user) <= 120 + + +def test_a_verified_identity_overrides_a_supplied_name() -> None: + """A signed-in person must not be able to act under someone else's name.""" + verified = Actor(user="alice", verified=True) + + assert actor_for(verified, "bob") == ("alice", True) + assert actor_for(verified, None) == ("alice", True) + + +def test_without_a_verified_identity_the_supplied_name_is_used_unverified() -> None: + assert actor_for(ANONYMOUS, "bob") == ("bob", False) + assert actor_for(ANONYMOUS, " bob ") == ("bob", False) + + +def test_no_identity_and_no_name_is_an_error() -> None: + for supplied in (None, "", " "): + with pytest.raises(ValueError): + actor_for(ANONYMOUS, supplied) + + +def test_headers_without_a_getter_are_anonymous() -> None: + assert resolve_actor(object(), edge_secret=SECRET) == ANONYMOUS diff --git a/tests/test_submission_api.py b/tests/test_submission_api.py index 0e0596e..6c81a8c 100644 --- a/tests/test_submission_api.py +++ b/tests/test_submission_api.py @@ -400,3 +400,178 @@ def test_the_ui_page_only_calls_public_endpoints(client: TestClient) -> None: def test_the_ui_page_is_not_in_the_api_schema(client: TestClient) -> None: """It is a page, not part of the contract a machine client reads.""" assert "/ui" not in client.get("/openapi.json").json()["paths"] + + +# --- edge-injected identity over HTTP --------------------------------------- + +EDGE_SECRET = "test-edge-secret" + + +def _edge_client(settings: Settings, backend: FakeBackend, monkeypatch) -> TestClient: + monkeypatch.setenv("BAMBU_EDGE_SHARED_SECRET", EDGE_SECRET) + app = create_app(settings=settings, backend_factory=lambda _d, _c: backend) + return TestClient(app) + + +def _edge_headers(user: str = "alice", role: str = "operator") -> dict[str, str]: + return {"X-Edge-Auth": EDGE_SECRET, "X-Auth-User": user, "X-Auth-Role": role} + + +def test_whoami_reports_no_identity_by_default(client: TestClient) -> None: + body = client.get("/whoami").json() + + assert body == { + "user": None, + "role": None, + "verified": False, + "identity_available": False, + } + + +def test_whoami_reports_the_edge_identity( + settings: Settings, backend: FakeBackend, monkeypatch +) -> None: + with _edge_client(settings, backend, monkeypatch) as client: + body = client.get("/whoami", headers=_edge_headers()).json() + + assert body == { + "user": "alice", + "role": "operator", + "verified": True, + "identity_available": True, + } + + +def test_whoami_distinguishes_not_signed_in_from_cannot_tell( + settings: Settings, backend: FakeBackend, monkeypatch +) -> None: + """`identity_available` is what lets the page word itself honestly.""" + with _edge_client(settings, backend, monkeypatch) as client: + body = client.get("/whoami").json() + + assert body["verified"] is False + assert body["identity_available"] is True + + +def test_a_forged_identity_header_is_ignored( + settings: Settings, backend: FakeBackend, monkeypatch +) -> None: + """The port stays reachable on the tailnet, so a bare header proves nothing.""" + with _edge_client(settings, backend, monkeypatch) as client: + body = client.get("/whoami", headers={"X-Auth-User": "admin"}).json() + assert body["verified"] is False + + response = _upload_with(client, headers={"X-Auth-User": "admin"}) + job = response.json() + + assert job["requested_by"] == "remote-user-1" + assert job["requested_by_verified"] is False + + +def _upload_with(client: TestClient, *, headers: dict[str, str], form: dict | None = None): + data = {"target_machine": "bambu_test_01", "requested_by": "remote-user-1"} + if form: + data.update(form) + return client.post( + "/submissions", + files={"file": ("part.gcode", SAMPLE_GCODE.encode(), "application/octet-stream")}, + data=data, + headers=headers, + ) + + +def test_a_verified_identity_becomes_the_submitter( + settings: Settings, backend: FakeBackend, monkeypatch +) -> None: + with _edge_client(settings, backend, monkeypatch) as client: + job = _upload_with(client, headers=_edge_headers()).json() + + # The form said remote-user-1; the signed-in account wins. + assert job["requested_by"] == "alice" + assert job["requested_by_verified"] is True + + +def test_approval_and_cancellation_record_the_verified_actor( + settings: Settings, backend: FakeBackend, monkeypatch +) -> None: + with _edge_client(settings, backend, monkeypatch) as client: + first = _upload_with(client, headers=_edge_headers()).json() + approved = client.post( + f"/submissions/{first['submission_id']}/approve", + json={"approved_by": "somebody-else"}, + headers=_edge_headers(user="bob"), + ).json() + + second = _upload_with(client, headers=_edge_headers()).json() + cancelled = client.post( + f"/submissions/{second['submission_id']}/cancel", + json={"reason": "not needed"}, + headers=_edge_headers(user="carol"), + ).json() + + assert approved["approved_by"] == "bob" + assert approved["approved_by_verified"] is True + assert approved["history"][-1]["note"] == "approved by bob (verified identity)" + + assert cancelled["state"] == "cancelled" + assert cancelled["history"][-1]["note"] == ( + "cancelled by carol (verified identity): not needed" + ) + + +def test_an_unverified_actor_is_marked_as_such(client: TestClient) -> None: + created = _upload(client).json() + approved = client.post( + f"/submissions/{created['submission_id']}/approve", + json={"approved_by": "lab-operator"}, + ).json() + + assert created["requested_by_verified"] is False + assert approved["approved_by_verified"] is False + assert approved["history"][-1]["note"] == "approved by lab-operator" + + +def test_a_name_is_required_when_no_identity_is_verified(client: TestClient) -> None: + response = client.post( + "/submissions", + files={"file": ("part.gcode", SAMPLE_GCODE.encode(), "application/octet-stream")}, + data={"target_machine": "bambu_test_01"}, + ) + assert response.status_code == 422 + + created = _upload(client).json() + for path, _ in (("approve", None), ("cancel", None)): + refused = client.post(f"/submissions/{created['submission_id']}/{path}", json={}) + assert refused.status_code == 422, path + + +def test_the_page_derives_its_api_base_from_its_own_url(client: TestClient) -> None: + """One file must serve both the direct deployment and an edge path prefix.""" + body = client.get("/ui").text + + assert "API_BASE" in body + assert 'window.location.pathname.replace' in body + # No bare-rooted fetch: that would reach the dashboard behind the edge. + assert 'fetch("/' not in body + assert "/whoami" in body + + +def test_no_secret_is_ever_returned( + settings: Settings, backend: FakeBackend, monkeypatch +) -> None: + with _edge_client(settings, backend, monkeypatch) as client: + for path in ("/whoami", "/", "/status", "/ui", "/submissions"): + assert EDGE_SECRET not in client.get(path, headers=_edge_headers()).text, path + + +def test_the_ui_is_served_at_both_slash_spellings(client: TestClient) -> None: + """Behind an edge prefix the canonical URL ends in a slash. + + Serving only `/ui` would make Starlette redirect `/ui/` to `/ui`, and that + Location drops the edge prefix — landing the visitor on the dashboard + instead of the page. Both spellings must answer directly. + """ + for path in ("/ui", "/ui/"): + response = client.get(path, follow_redirects=False) + assert response.status_code == 200, path + assert "Submit a print" in response.text From 51003e094cdc62c23c9a9bd9f929d1ac3dc05edf Mon Sep 17 00:00:00 2001 From: yoyoliuuu <yoyoyimeng.liu@mail.utoronto.ca> Date: Mon, 7 Sep 2026 00:58:28 -0400 Subject: [PATCH 2/2] feat: retention for finished jobs, and an edge deployment runbook Two follow-ups from bringing the pipeline up, both flagged during it. ## Retention Terminal records used to accumulate forever, and the only way to clear one was `rm` on the store directory -- which left the running process serving a record whose file was gone. Both halves are fixed: - Terminal records are swept at startup once older than `submissions.retain_terminal_days` (30 by default, null disables). - `DELETE /submissions/{id}` removes one finished job's record and artifact immediately. Three decisions worth recording: - **A job still in play is never swept, however old.** One stuck in `validating` is a signal, not litter, and quietly deleting it would destroy the evidence of whatever wedged it. - **Only a terminal job can be deleted.** Withdrawing a waiting job is `cancel`, which leaves a record of the decision; allowing delete there would erase the decision along with the job. - **Sweeping at startup, not on a timer.** The store loads from disk once, so a record removed underneath a running process lingers in memory until restart. Startup is the one moment the two views are guaranteed to agree -- and that divergence is precisely what made hand-cleanup necessary during bring-up. `TERMINAL_STATES` is derived from the transition table (states with no onward edge) rather than listed a second time, so a state added there cannot be forgotten here. A test pins that correspondence in both directions. ## docs/EDGE_DEPLOY.md An ordered runbook for putting the page behind the dashboard's login, since the remaining work is all root steps and easy to get in the wrong order. It leads with what the two access paths mean for attribution, validates the Caddyfile before installing it (a bad config takes the whole dashboard down), reloads rather than restarts Caddy, and ends with the step that must come last -- narrowing the gateway's bind back to loopback, which is only safe once the edge route works. The verification step is the one that matters: a forged `X-Auth-User` presented directly must come back `verified: false` while `identity_available: true` confirms the gateway actually picked the secret up. Both were checked against the real code. The secret itself is generated into the gitignored `.env` (which the unit already loads, so the gateway half needs no root) and the runbook reads it back with `sed` rather than having anyone retype it. It is not in git and not in any transcript. `uv run ruff check .` and `uv run pytest -q` (161 tests, 9 new) pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UQvsfEeDitEyNzbCwrcEdD --- README.md | 22 +++++ docs/EDGE_DEPLOY.md | 140 ++++++++++++++++++++++++++++++++ docs/TODO.md | 11 ++- src/bambu_server/config.py | 4 + src/bambu_server/main.py | 35 ++++++++ src/bambu_server/submissions.py | 66 ++++++++++++++- tests/test_submission_api.py | 30 ++++++- tests/test_submissions.py | 113 ++++++++++++++++++++++++++ 8 files changed, 413 insertions(+), 8 deletions(-) create mode 100644 docs/EDGE_DEPLOY.md diff --git a/README.md b/README.md index fb6f22d..d5c667f 100644 --- a/README.md +++ b/README.md @@ -105,6 +105,7 @@ Submission pipeline routes: | GET | `/submissions/{submission_id}` | One job with its verdict and history | | POST | `/submissions/{submission_id}/approve` | Record sign-off on a queued job | | POST | `/submissions/{submission_id}/cancel` | Withdraw a waiting job from the queue | +| DELETE | `/submissions/{submission_id}` | Delete a finished job's record (retention) | No `/control/*` routes exist, and no route dispatches a print. @@ -330,6 +331,8 @@ cookie cannot be shared with this gateway on its own address, because raw Suffix List, so browsers drop tailnet-wide cookies. One origin behind the edge means one login (see `ac-organic-lab/docs/AUTH_DESIGN.md`). +**Deploying it: [`docs/EDGE_DEPLOY.md`](docs/EDGE_DEPLOY.md).** + The route lives in `ac-organic-lab/deploy/Caddyfile.single-edge` as `/bambu/*`, gated by `forward_auth` and injecting the identity described above; the dashboard frames `/bambu/ui/` under Utils → 3D Printers. Once that route is live the @@ -340,6 +343,25 @@ Note the page answers at both `/ui` and `/ui/`. Serving only one would make Starlette redirect between them with a `Location` that drops the edge prefix, landing the visitor on the dashboard. +### Retention + +Terminal records (`rejected`, `finished`, `failed`, `cancelled`) are swept at +startup once older than `submissions.retain_terminal_days` (30 by default; set +it to null to keep everything). A job that is **still in play is never swept**, +however old — one stuck in `validating` is a signal, not litter. The set of +terminal states is derived from the transition table rather than listed twice, +so a state added there cannot be missed here. + +`DELETE /submissions/{id}` removes one finished job's record and artifact +immediately. Only a terminal job can be deleted: withdrawing one that is still +waiting is `cancel`, which leaves a record of the decision — deleting it would +erase that along with the job. + +Sweeping happens at startup rather than on a timer so the store's on-disk and +in-memory views stay identical. A record removed underneath a running process +lingers in memory until a restart, which is exactly the divergence that made +hand-cleanup necessary before this existed. + ### Cancelling `POST /submissions/{id}/cancel` withdraws a waiting job. It is a **queue diff --git a/docs/EDGE_DEPLOY.md b/docs/EDGE_DEPLOY.md new file mode 100644 index 0000000..dfcf364 --- /dev/null +++ b/docs/EDGE_DEPLOY.md @@ -0,0 +1,140 @@ +# Putting the submission page behind the dashboard's login + +The submission page works two ways, and the difference is who gets recorded: + +| reached | identity | recorded as | +|---|---|---| +| directly, `http://100.64.254.6:8012/ui` | none — no login on that port | a typed-in label, `*_verified: false` | +| through the edge, `/bambu/ui/` | the dashboard's `ac_auth` session | the signed-in account, `*_verified: true` | + +Only the second is attributable, and it is also the only one that can ever have +single sign-on: a session cookie cannot be shared with this gateway on its own +address, because raw `100.x` addresses cannot carry a `Domain` cookie and +`*.ts.net` is on the Public Suffix List. See +`ac-organic-lab/docs/AUTH_DESIGN.md`. + +This is the runbook for turning the second one on. Steps 1–3 are safe in any +order; **step 4 must come last**. + +Everything here fails closed. Until the secrets on both sides match, the +gateway trusts no injected identity and behaves exactly as it does today. + +--- + +## 0. The secret + +Already generated and stored in this repo's gitignored `.env` as +`BAMBU_EDGE_SHARED_SECRET`. Read it back rather than retyping it: + +```bash +grep '^BAMBU_EDGE_SHARED_SECRET=' /home/sdl2/caoyang/bambu-server/.env +``` + +To rotate it instead, generate a new one and update **both** sides together: + +```bash +openssl rand -hex 32 +``` + +## 1. Give Caddy the same secret + +The edge reads its secrets from an environment file supplied by a systemd +drop-in (`/etc/systemd/system/caddy.service.d/edge-secret.conf`, which points at +`/etc/caddy/edge-secrets.env`). Append the line — same value as `.env`: + +```bash +sudo sh -c 'printf "BAMBU_EDGE_SHARED_SECRET=%s\n" "$1" >> /etc/caddy/edge-secrets.env' _ \ + "$(sed -n 's/^BAMBU_EDGE_SHARED_SECRET=//p' /home/sdl2/caoyang/bambu-server/.env)" +``` + +Check it landed exactly once, without printing it: + +```bash +sudo grep -c '^BAMBU_EDGE_SHARED_SECRET=' /etc/caddy/edge-secrets.env # expect 1 +``` + +## 2. Install the edge route + +The `/bambu/*` block lives in `ac-organic-lab/deploy/Caddyfile.single-edge`. +Validate before installing — a bad config takes the whole dashboard down: + +```bash +cd /home/sdl2/caoyang/ac-organic-lab +caddy validate --config deploy/Caddyfile.single-edge --adapter caddyfile +sudo cp deploy/Caddyfile.single-edge /etc/caddy/Caddyfile +sudo systemctl reload caddy # reload, not restart: no dropped connections +systemctl is-active caddy +``` + +If the reload fails, Caddy keeps running the old config — fix and retry rather +than restarting. + +## 3. Restart the gateway so it reads the secret + +```bash +sudo systemctl restart bambu-server +sc_state=$(systemctl is-active bambu-server); echo "bambu-server: $sc_state" +``` + +### Verify + +The gateway must now *decline* to trust an unaccompanied header, and accept one +that comes through the edge: + +```bash +# Direct, forged header -> not verified. This is the important one. +curl -fsS -H 'X-Auth-User: admin' http://127.0.0.1:8012/whoami +# expect: {"user":null,"role":null,"verified":false,"identity_available":true} + +# `identity_available: true` confirms the gateway picked the secret up at all. +``` + +Then open the dashboard, go to **Utils → 3D Printers**, and confirm the framed +panel loads and shows *Signed in as <you>* instead of a name field. The +dashboard needs a rebuild only if its own code changed: + +```bash +cd /home/sdl2/caoyang/ac-organic-lab/web && npm run build +sudo systemctl restart ac-organic-lab-web +``` + +> Check `git status` there first — a build ships whatever is in the working +> tree, including anyone else's in-flight changes. + +## 4. Last: close the direct path + +Only once `/bambu/ui/` works. This narrows the gateway back to loopback, so +`POST /submissions` is no longer reachable unauthenticated from the tailnet. + +Edit `ExecStart` in `deploy/bambu-server.local.service` from `--host 0.0.0.0` +back to `--host 127.0.0.1`, then: + +```bash +cd /home/sdl2/caoyang/bambu-server +sudo cp deploy/bambu-server.local.service /etc/systemd/system/bambu-server.service +sudo systemctl daemon-reload && sudo systemctl restart bambu-server +``` + +The aggregator is unaffected either way — `equipment.yaml` polls +`127.0.0.1:8012`, and the edge proxies over loopback too. + +Afterwards the direct URL stops working, so remove or relabel the *Open +directly* fallback link in `ac-organic-lab`'s +`web/src/app/utils/printers/BambuPrinterPanel.tsx`. + +--- + +## If the framed panel is blank + +In order of likelihood: + +1. **Route not installed** — `curl -sI http://100.64.254.6/bambu/ui/` should + redirect or return 200, not the dashboard's 404. +2. **Not signed in** — the edge's `forward_auth` returns 401 and the frame shows + nothing. Log into the dashboard first. +3. **Prefix leak** — if the page loads but its data does not, check the browser + console for requests to `/printers` instead of `/bambu/printers`. The page + derives its base by stripping a trailing `/ui`, so it must be reached at + `/bambu/ui/` (the edge redirects `/bambu` and `/bambu/` there). +4. **Secret mismatch** — the page loads and works but still shows a name field. + `/bambu/whoami` will report `verified: false`. Compare the two values. diff --git a/docs/TODO.md b/docs/TODO.md index 2137317..f8f1fa5 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -82,9 +82,12 @@ Open, from the design's §10 data gaps and what the build surfaced: It is legal only from `queued` / `approved`, never for a dispatched job — aborting a print stays a control-plane action. Worth folding back into `SUBMISSION_PIPELINE_DESIGN.md` §5 when that doc is next revised. -- No retention policy: rejected and finished jobs, and their uploaded artifacts, - stay on disk and in `GET /submissions` indefinitely. Fine at current volume, - but it needs a sweep before this runs unattended for long. +- ~~No retention policy~~ — done. Terminal records are swept at startup past + `submissions.retain_terminal_days` (30 default, null disables), and + `DELETE /submissions/{id}` removes one finished job immediately. A job still + in play is never swept however old. Sweeping at startup rather than on a + timer keeps on-disk and in-memory views identical — the divergence that + forced hand-cleanup during bring-up. ## Submission page @@ -143,7 +146,7 @@ until the gateway pushes to `/api/ingest/events`. ## Test suite - `uv run ruff check .` passes. -- `uv run pytest -q` passes all 152 tests, including the FastAPI API tests and +- `uv run pytest -q` passes all 161 tests, including the FastAPI API tests and the submission pipeline (artifact inspection, validation, store/state machine, queue ETA, HTTP surface). Tests build their own `.3mf` and `.gcode` fixtures and use fake backends; nothing touches hardware. diff --git a/src/bambu_server/config.py b/src/bambu_server/config.py index 4205f0d..c7592b7 100644 --- a/src/bambu_server/config.py +++ b/src/bambu_server/config.py @@ -116,6 +116,10 @@ class SubmissionSettings(BaseModel): # enough to trust a motion-derived bounding box, so plate fit is reported # as not applicable rather than computed from a partial scan. scan_max_bytes: int = Field(default=64 * 1024 * 1024, ge=64 * 1024) + #: Age after which a *terminal* job's record is swept at startup. Jobs that + #: are still in play are never swept however old they are: a job stuck in + #: `validating` is a signal, not litter. Set to null to keep everything. + retain_terminal_days: float | None = Field(default=30.0, gt=0) class Settings(BaseModel): diff --git a/src/bambu_server/main.py b/src/bambu_server/main.py index 26c6943..f051768 100644 --- a/src/bambu_server/main.py +++ b/src/bambu_server/main.py @@ -15,6 +15,7 @@ """ import asyncio +import logging from collections.abc import AsyncIterator, Callable from contextlib import asynccontextmanager from datetime import UTC, datetime @@ -71,6 +72,8 @@ run_validation, ) +logger = logging.getLogger(__name__) + BackendFactory = Callable[[PrinterDefinition, PrinterCredentials], PrinterBackend] #: Read size for streaming an upload to disk. @@ -533,6 +536,38 @@ async def cancel_submission( except SubmissionError as exc: raise HTTPException(status_code=400, detail=str(exc)) from exc + @app.delete( + "/submissions/{submission_id}", + status_code=204, + tags=["submissions"], + ) + async def delete_submission( + store: Annotated[SubmissionStore, Depends(get_store)], + actor: Annotated[Actor, Depends(get_actor)], + submission_id: Annotated[str, Path(pattern=r"^[0-9a-f]{32}$")], + ) -> None: + """Delete a finished job's record and artifact. + + Retention housekeeping. Only a terminal job can be deleted: withdrawing + one that is still waiting is `cancel`, which leaves a record of the + decision, and deleting it instead would erase that along with the job. + """ + + job = store.get(submission_id) + if job is None: + raise HTTPException(status_code=404, detail="unknown submission") + try: + forgotten = await store.forget(job) + except InvalidTransition as exc: + raise HTTPException(status_code=409, detail=str(exc)) from exc + logger.info( + "Deleted submission %s (%s) on behalf of %s%s", + forgotten.submission_id, + forgotten.state, + actor.user or "an unidentified caller", + " (verified)" if actor.verified else "", + ) + return app diff --git a/src/bambu_server/submissions.py b/src/bambu_server/submissions.py index 974d33a..41aa691 100644 --- a/src/bambu_server/submissions.py +++ b/src/bambu_server/submissions.py @@ -27,7 +27,7 @@ import re import uuid from collections.abc import AsyncIterator -from datetime import UTC, datetime +from datetime import UTC, datetime, timedelta from pathlib import Path from typing import Literal, NoReturn @@ -87,6 +87,12 @@ "cancelled": frozenset(), } +#: States with nowhere left to go. Derived from the transition table rather +#: than listed again, so a state added there cannot be forgotten here. +TERMINAL_STATES: frozenset[str] = frozenset( + state for state, onward in ALLOWED_TRANSITIONS.items() if not onward +) + _CONTROL_CHARS = re.compile(r"[\x00-\x1f\x7f]") @@ -223,7 +229,13 @@ def settings(self) -> SubmissionSettings: return self._settings def load(self) -> None: - """Read persisted jobs from disk. Called once at startup.""" + """Read persisted jobs from disk, then sweep expired ones. + + Called once at startup. Sweeping here rather than on a timer keeps the + store's on-disk and in-memory views identical: a record removed while + the process runs would linger in memory until a restart, which is the + divergence that made manual cleanup necessary in the first place. + """ self._root.mkdir(parents=True, exist_ok=True) for path in sorted(self._root.glob("*.json")): @@ -235,7 +247,36 @@ def load(self) -> None: logger.warning("Ignoring unreadable submission record %s", path.name) continue self._jobs[job.submission_id] = job - logger.info("Loaded %d submission(s) from %s", len(self._jobs), self._root) + swept = self._sweep_expired() + logger.info( + "Loaded %d submission(s) from %s (%d expired record(s) swept)", + len(self._jobs), + self._root, + swept, + ) + + def _sweep_expired(self) -> int: + """Drop terminal records past the retention window.""" + + retain_days = self._settings.retain_terminal_days + if retain_days is None: + return 0 + cutoff = datetime.now(UTC) - timedelta(days=retain_days) + expired = [ + job + for job in self._jobs.values() + if job.state in TERMINAL_STATES and job.updated_at < cutoff + ] + for job in expired: + self._forget(job) + return len(expired) + + def _forget(self, job: SubmissionJob) -> None: + """Remove a job's record and any artifact still on disk.""" + + self.artifact_path(job).unlink(missing_ok=True) + (self._root / f"{job.submission_id}.json").unlink(missing_ok=True) + self._jobs.pop(job.submission_id, None) # -- reads ------------------------------------------------------------- @@ -454,6 +495,25 @@ async def cancel( current.submission_id, "cancelled", note ) + async def forget(self, job: SubmissionJob) -> SubmissionJob: + """Delete a terminal job's record and artifact. + + Retention housekeeping, not a workflow step: only a job that has + finished going anywhere can be forgotten. Withdrawing one that is still + waiting is :meth:`cancel`, which leaves an auditable record -- deleting + it instead would erase the decision along with the job. + """ + + async with self._lock: + current = self._require(job.submission_id) + if current.state not in TERMINAL_STATES: + raise InvalidTransition( + f"only a finished submission can be deleted; " + f"{current.submission_id} is {current.state}" + ) + await asyncio.to_thread(self._forget, current) + return current + # -- internals --------------------------------------------------------- def _require(self, submission_id: str) -> SubmissionJob: diff --git a/tests/test_submission_api.py b/tests/test_submission_api.py index 6c81a8c..f20a87d 100644 --- a/tests/test_submission_api.py +++ b/tests/test_submission_api.py @@ -284,7 +284,8 @@ def test_the_pipeline_exposes_no_control_routes_and_no_dispatch( assert not any("dispatch" in path for path in paths) for path, operations in paths.items(): for method in operations: - assert method.lower() in {"get", "post"}, (path, method) + # delete exists only for retention housekeeping on finished jobs. + assert method.lower() in {"get", "post", "delete"}, (path, method) def test_reads_never_cause_printer_io(client: TestClient, backend: FakeBackend) -> None: @@ -575,3 +576,30 @@ def test_the_ui_is_served_at_both_slash_spellings(client: TestClient) -> None: response = client.get(path, follow_redirects=False) assert response.status_code == 200, path assert "<title>Submit a print" in response.text + + +def test_a_finished_job_can_be_deleted_over_http(client: TestClient) -> None: + created = _upload(client).json() + sid = created["submission_id"] + client.post(f"/submissions/{sid}/cancel", json={"cancelled_by": "lab-operator"}) + + response = client.delete(f"/submissions/{sid}") + + assert response.status_code == 204 + assert client.get(f"/submissions/{sid}").status_code == 404 + assert client.get("/submissions").json() == [] + + +def test_a_waiting_job_cannot_be_deleted(client: TestClient) -> None: + """Withdrawing a queued job is `cancel`, which leaves a record.""" + created = _upload(client).json() + + response = client.delete(f"/submissions/{created['submission_id']}") + + assert response.status_code == 409 + assert "only a finished submission" in response.json()["detail"] + assert client.get(f"/submissions/{created['submission_id']}").status_code == 200 + + +def test_deleting_an_unknown_submission_is_404(client: TestClient) -> None: + assert client.delete("/submissions/" + "0" * 32).status_code == 404 diff --git a/tests/test_submissions.py b/tests/test_submissions.py index 39e62a3..73e3c85 100644 --- a/tests/test_submissions.py +++ b/tests/test_submissions.py @@ -3,6 +3,7 @@ from __future__ import annotations from collections.abc import AsyncIterator +from datetime import UTC, datetime, timedelta from pathlib import Path import pytest @@ -377,3 +378,115 @@ def test_cancellation_can_never_reach_a_dispatched_job() -> None: assert CANCELLABLE_STATES == {"queued", "approved"} for state in ("dispatching", "running", "finished", "failed", "rejected"): assert "cancelled" not in ALLOWED_TRANSITIONS[state] + + +# --- retention --------------------------------------------------------------- + + +def test_terminal_states_are_derived_from_the_transition_table() -> None: + """A state added to the table cannot be forgotten in the retention set.""" + from bambu_server.submissions import ALLOWED_TRANSITIONS, TERMINAL_STATES + + assert TERMINAL_STATES == {"rejected", "finished", "failed", "cancelled"} + for state in TERMINAL_STATES: + assert ALLOWED_TRANSITIONS[state] == frozenset() + for state in set(ALLOWED_TRANSITIONS) - TERMINAL_STATES: + assert ALLOWED_TRANSITIONS[state], state + + +async def test_only_a_finished_job_can_be_deleted( + store: SubmissionStore, profile: MachineProfile +) -> None: + job = await run_validation(store, await _accept(store), profile) + + with pytest.raises(InvalidTransition, match="only a finished submission"): + await store.forget(job) + + cancelled = await store.cancel(job, cancelled_by="operator") + forgotten = await store.forget(cancelled) + + assert forgotten.submission_id == job.submission_id + assert store.get(job.submission_id) is None + assert not (store.root / f"{job.submission_id}.json").exists() + + +async def test_deleting_removes_a_leftover_artifact_too( + store: SubmissionStore, profile: MachineProfile +) -> None: + """A rejected job keeps its artifact; deleting the record must not orphan it.""" + body = SAMPLE_GCODE.replace("; nozzle_diameter = 0.4", "; nozzle_diameter = 0.6") + job = await run_validation(store, await _accept(store, body.encode()), profile) + path = store.artifact_path(job) + assert job.state == "rejected" + assert path.exists() + + await store.forget(job) + + assert not path.exists() + assert list(store.root.iterdir()) == [] + + +async def test_expired_terminal_records_are_swept_at_startup( + store: SubmissionStore, profile: MachineProfile, tmp_path: Path +) -> None: + job = await run_validation(store, await _accept(store), profile) + cancelled = await store.cancel(job, cancelled_by="operator") + + # Backdate the record past the window, as an old job on disk would be. + stale = cancelled.model_copy( + update={"updated_at": datetime.now(UTC) - timedelta(days=40)} + ) + (store.root / f"{job.submission_id}.json").write_text( + stale.model_dump_json(), encoding="utf-8" + ) + + reopened = SubmissionStore( + SubmissionSettings(directory=tmp_path / "submissions", retain_terminal_days=30) + ) + reopened.load() + + assert reopened.get(job.submission_id) is None + assert list((tmp_path / "submissions").iterdir()) == [] + + +async def test_a_job_still_in_play_is_never_swept_however_old( + store: SubmissionStore, profile: MachineProfile, tmp_path: Path +) -> None: + """A job stuck mid-pipeline is a signal, not litter.""" + job = await run_validation(store, await _accept(store), profile) + stale = job.model_copy( + update={"updated_at": datetime.now(UTC) - timedelta(days=400)} + ) + (store.root / f"{job.submission_id}.json").write_text( + stale.model_dump_json(), encoding="utf-8" + ) + + reopened = SubmissionStore( + SubmissionSettings(directory=tmp_path / "submissions", retain_terminal_days=1) + ) + reopened.load() + + assert reopened.get(job.submission_id) is not None + assert reopened.get(job.submission_id).state == "queued" + + +async def test_retention_can_be_disabled( + store: SubmissionStore, profile: MachineProfile, tmp_path: Path +) -> None: + job = await run_validation(store, await _accept(store), profile) + cancelled = await store.cancel(job, cancelled_by="operator") + stale = cancelled.model_copy( + update={"updated_at": datetime.now(UTC) - timedelta(days=4000)} + ) + (store.root / f"{job.submission_id}.json").write_text( + stale.model_dump_json(), encoding="utf-8" + ) + + reopened = SubmissionStore( + SubmissionSettings( + directory=tmp_path / "submissions", retain_terminal_days=None + ) + ) + reopened.load() + + assert reopened.get(job.submission_id) is not None