Security audit + CI hardening

[xuyanwen2012]

Stability and security release: bug fixes, hardening and dead-code removal,
with no new features and no breaking API changes. One commit
(Move speckit scaffolding to openspec) is tooling that rode along and is
not part of the release itself.

Before upgrading a running deployment

The minimum bearer token length went from 16 to 32 characters (security
audit SEC-002). Config::load bails on a shorter token, so a host whose
config.toml predates this will not start — systemd's
Restart=on-failure will just loop. This is not an API change and so is
absent from the compatibility list below, but it is the one change that can
take a deployment down.

Check on the host before upgrading, without printing the token:

awk -F'"' '/^token/ {print "token length:", length($2)}' config.toml

>= 32 is fine. Anything shorter needs a new token
(openssl rand -hex 32), and the Android client must be updated in the
same window — it is the same bearer token, so a rotation without the
client returns 401 for everything. Hosts set up via just setup are
unaffected: it has always generated 64 hex characters.

Nothing else about the upgrade needs manual work. config.toml is otherwise
backward compatible (the three fields added since v2.0.0 all have defaults,
and unknown keys are ignored), and the one new migration
(0004_add_progress) is an additive ALTER TABLE ... DEFAULT 0.0 applied
automatically at startup. v2.0.0 upgrades straight to this release with no
intermediate step.

For API consumers (zun-android-app)

No endpoint was added, removed, renamed, or changed signature. Verified
against the client: nothing here requires a coordinated client change.

Response shape (2):

• GET /jobs/{id} no longer returns metadata. It read an <output>.json
  sidecar that nothing in this codebase has ever written, so it was
  always null despite the contract promising otherwise.
• GET /jobs/{id}'s error is now redacted like a 5xx body — filesystem
  paths become <path>, URLs <url>. Previously it handed the phone the
  server's comfy_url and local paths verbatim.

Status codes (1):

• Uploads over 20 MiB return 413 payload_too_large instead of 400.

New validation, previously accepted (4):

• timeout_seconds must be 1..=1800 — see the queue fix below.
• label ≤ 1000 B, description ≤ 4000 B, input_name ≤ 255 B.
• A multipart submit whose bytes don't match input_sha256 is now rejected
  even when that hash is already cached. Previously the cached file was
  used and the uploaded bytes silently ignored, so a client sending
  (hash of A, bytes of B) got a 202 and a job rendered from A — despite the
  contract already stating "Server verifies bytes match".

Behaviour (3):

• Default timeout_seconds 60 → 120.
• Jobs no longer fail when ComfyUI is unreachable. They return to
  queued and resume when it comes back. Clients that render queued as
  in-progress will show a job as running for the duration of an outage.
• ?wait= long-polling returns on the change itself and always within the
  requested window.

The queue-wedging bug

timeout_seconds was accepted from the client unvalidated. A prompt stored
with a negative value reached the worker as -1i64 as u64 == u64::MAX —
a timeout that never fires, on a worker that runs exactly one job at a
time. That wedged the entire queue until the process was restarted. Now
range-checked on write and clamped on read, so rows written before the
check still produce a timeout that fires.

Surviving a ComfyUI restart

Any error from process_job used to mark the job failed, so a backend
that was merely absent had every queued job claimed, burned through its
retries and buried ~1.5 s apart, with no endpoint to bring them back.
"The backend isn't there" is not the job's fault: connect failures and ws
handshake timeouts are now typed as unreachable, and the worker requeues
and backs off. Verified live — with ComfyUI killed, three jobs sat
queued for 25 s with zero failures, then drained on their own.

Storage that was never reclaimed

Every job uploads its input into ComfyUI's input/ and leaves a second
copy of its result in ComfyUI's output/. ComfyUI prunes neither and has
no delete endpoint, so purge_after_days only ever bounded this server's
copies. On the dev box: 23 MB under data/outputs beside 445 MB of the
same images under ComfyUI/output, plus 116 MB of spent inputs.

New optional comfy_data_dir points the purge task at ComfyUI. The sweep
is deliberately narrow because it deletes from a directory this server does
not own: opt-in, never recursive, and limited to files that are both
zun_-prefixed and past the retention window. Enabling it removed 686
files and 538 MB while leaving every non-zun_ file untouched.

This is opt-in and defaults to off — upgrading does not reclaim anything
by itself. Reclaiming the backlog is a deployment action: point
comfy_data_dir at ComfyUI's data directory (the one holding input/ and
output/), and the daily purge takes it from there.

Temp files left behind by failed writes

Raised in review against backup.rs and true there, but the same hole was
in paths::atomic_write and atomic_copy — the two that run on every job
output and every input upload, against once a day for the backup.

Nothing collects what leaks. prune_old skips anything whose extension
isn't db, and a jobs-2026-08-19.db.tmp.a1b2c3d4 name reports its
extension as the random suffix; purge only deletes paths recorded in the
DB, which a temp file never reaches. The likeliest reason to fail is a full
disk, so the leak arrives exactly when the space is needed.

prune_old's filter is left alone on purpose: the fix is for strays not to
be created, and widening that filter would weaken what it is for — not
touching files in backups/ this server did not write.

paths::Staged now owns the temp path and removes it on drop unless
committed, so the cleanup is not three call sites remembering. That is also
the half of SEC-007 that was missed: the implementations were unified
without checking that they behaved the same.

All three failure paths have regression tests, verified to fail against the
previous code. (An earlier note in this discussion said the failure path was
impractical to trigger — that was wrong: renaming onto a non-empty directory
fails while the staging write succeeds.)

Performance

Measured on an RTX 4070 Ti Super, FLUX2 klein, release build:

	before	after
three queued jobs	17.4 s	9.0 s
first job after a ComfyUI restart	19.5 s	3.7 s
long-poll latency	up to 750 ms	~0

• Thumb/preview encoding left the worker's critical path; it cost ~2.5 s
  against ~3.3 s of GPU time, so the card idled for nearly half of each
  job. Nothing depends on it finishing — the image handlers already
  generate a missing rendition lazily.
• A best-effort warm-up runs one throwaway generation at startup, moving
  the ~16 s of weight staging off the user's first tap.
• Inputs are uploaded under a content-addressed name, so N prompts against
  one photo leave one file in ComfyUI's input/ instead of N.

Also

• All direct dependencies upgraded to latest stable, including base64
  0.22→0.23 and tokio-tungstenite 0.29→0.30 across their major boundaries,
  with no source changes. Clears the yanked num-bigint from cargo audit.
• Documentation that described things the code did not do: a ZUN_LOG_FORMAT
  env var that does not exist, a worker job span that was never opened, a
  /health endpoint claimed to probe ComfyUI, and a README pointing at a
  plan/PLAN.md that isn't in the repo.
• ComfyClient::health() and its dedicated client were dead code; the
  reachability probe that replaced it has an actual caller (the warm-up).
• security-audit.md has no open findings: SEC-005 (unbounded response
  bodies, now capped at 8 MiB JSON / 128 MiB images) and SEC-007 (duplicated
  atomic-write implementation) both closed.

Verification

• cargo fmt --check, cargo clippy --all-targets -- -D warnings,
  cargo test --locked — 132 tests, 0 failures (up from 113).
• Every behavioural fix above has a regression test verified to fail
  against the pre-fix code.
• End-to-end against a live ComfyUI, including v0.33.2 (up from 0.19.3,
  14 minors): upload, ws progress frames, /history, /view, atomic write,
  derived images, AVIF negotiation, ETag/304, and cancel→/interrupt.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[gemini-code-assist]

Code Review

This pull request implements a comprehensive security hardening and CI/CD evaluation for the zun-rust-server. Key changes include increasing the minimum token length to 32 characters, introducing a WebSocket connection timeout to prevent worker stalls, ensuring atomic database backup writes via temporary files, mapping multipart payload size limit errors to a 413 status code, and adding extensive integration tests for backup, purge, and submit functionalities. Feedback on the changes suggests cleaning up the temporary database backup file if the VACUUM INTO or rename operations fail to prevent file leaks.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[gemini-code-assist]

If VACUUM INTO or tokio::fs::rename fails, the temporary file created by tmp_sibling will be left on disk. To prevent leaking temporary files on failure, we should clean up the temporary file if either operation fails.

    if let Err(e) = sqlx::query(sqlx::AssertSqlSafe(sql)).execute(pool).await {
        let _ = tokio::fs::remove_file(&tmp).await;
        return Err(e.into());
    }
    if let Err(e) = tokio::fs::rename(&tmp, &abs).await {
        let _ = tokio::fs::remove_file(&tmp).await;
        return Err(e.into());
    }