Skip to content

feat(download): civitai download + --base-model filter; surface on-site-gen trap; fix stale update-check - #131

Merged
ZacxDev merged 3 commits into
mainfrom
zach/cli-download-and-base-model
Jul 17, 2026
Merged

feat(download): civitai download + --base-model filter; surface on-site-gen trap; fix stale update-check#131
ZacxDev merged 3 commits into
mainfrom
zach/cli-download-and-base-model

Conversation

@ZacxDev

@ZacxDev ZacxDev commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

What & why

A blind-dogfood found the CLI is great at discovery but can't assemble: no download verb, no base-model filter, the clean-named "training-zip" trap, and a version-check that reported Latest release: v0.1.61 while running v0.1.62. This PR closes all four.

Feature 1 — civitai download (the missing verb)

  • Deterministic identifier: positional download <version-id> = a model-VERSION id; --model <model-id> (mutually exclusive) resolves the model's default (primary/latest) published version. Exactly one required — no numeric-ambiguity guessing.
  • Multi-file: default = the version's primary file; --file <name> selects one (exact, else unique case-insensitive substring; ambiguous/none errors and lists the files); --all downloads every file.
  • Output: --out <path> (single file only), --out-dir <dir> (server-named files into a dir, works with --all), default = server name in cwd; parent dirs created.
  • Mechanics: streams to <target>.part → renames on success (never a truncated final file); follows the /api/download → signed-storage 302 (Go strips Authorization cross-host); TTY-aware progress to stderr; large files streamed, never buffered.
  • Auth: uses the configured token source automatically (login token or CIVITAI_API_KEY); 401/403 → actionable "run civitai login" / "early-access or gated" message; --anon forces no token.
  • SHA256 verify (default on): mismatch deletes the .part and exits non-zero with both hashes; --no-verify skips; no published SHA256 → warn + skip verify (not a hard fail).
  • Idempotency: present target that verifies (or --no-verify) is skipped; --force re-downloads.
  • On-site-gen guard (papercut CLI DX: page-money template, init --dir/--name, validate warnings, harness next-steps #3): a selected file whose type != "Model" (e.g. a training-data ZIP masquerading as the ~18 MB "version") is refused by default; --allow-nonmodel overrides. --all skips bundled non-model files with a warning rather than aborting the whole run.

Feature 2 — models search --base-model (papercut #2)

Repeatable StringSliceVar → REST baseModels param. Verified against the live API and the civitai source (model.schema.ts: baseModels: z.union([z.string(), z.string().array()]); service does mv."baseModel" IN (…)) — repeated baseModels= keys are OR semantics. This is the key discovery filter for video checkpoints, which all share --type Checkpoint and differ only by base model ("Wan Video 2.2 T2V-A14B").

Papercut #3b — surface the trap in list/detail output

models get / model-versions get human output tags a version whose primary file is not weights with [training data — no downloadable weights]. --json is an unchanged raw passthrough.

Papercut #5 — stale update-check, root-caused

The symptom was the GitHub /releases/latest draft-exclusion artifact: while v0.1.62's release was a draft, that endpoint returned v0.1.61, so a v0.1.62 machine compared as "ahead" and the old code printed a misleading Latest release: v0.1.61 line. Confirmed resolved at the source — the endpoint now returns v0.1.62. Fixed the compare-guard to be robust regardless: current >= latest now shows NO notice (was the misleading older-"Latest release" line), and decideNotice force-refreshes the cache when it pins a "latest" older than the running version, so a stale pre-publish record can't linger through its TTL.

What I verified live

  • baseModels param exists, accepts repeated keys, and applies OR semantics (23 Illustrious + 8 Pony versions for baseModels=Pony&baseModels=Illustrious).
  • Version-detail file fields: id, name, type, sizeKB, primary, downloadUrl, hashes.SHA256 (+ AutoV1/AutoV2/CRC32/BLAKE3).
  • A real 24 KB download end-to-end: streamed with progress, SHA256 matched the API's published hash, .part renamed away, idempotent skip on re-run, and --model+--out created nested dirs.
  • The update-check endpoint now returns v0.1.62 (draft published), so the original symptom no longer reproduces; the compare-guard is hardened anyway.

Test coverage

httptest-server, table-driven, matching the repo's style:

  • download: version-id path, --model resolution, primary default, --file exact/substring/ambiguous/no-match, --all, --all skips non-model, SHA256 verify pass + mismatch (deletes .part + errors), missing-hash warn+skip, non-Model refusal + --allow-nonmodel, --out/--out-dir (dir creation), --out+--all conflict, redirect follow, Authorization sent when a token is configured, anonymous sends none, 401 refresh + non-refreshable 401 preserved, skip-if-present + --force, actionable 401.
  • --base-model: repeated-key encoding + single spaced value.
  • marker: rendered in models get + model-versions get, absent for weights, --json unchanged.
  • update-check: current >= latest → no notice; stale-cache force-refresh; behind still notifies.

go build ./... && go test ./... && go vet ./... all green (also -race). The branch-protection required checks pins-vs-published + scaffold-currency are unrelated to this diff.

Follow-up

The developer.civitai.com CLI guide (site/guide/cli.md, civitai-developer-docs) needs a follow-up documenting download + --base-modelnot touched here (separate repo).

🤖 Generated with Claude Code

ZacxDev and others added 2 commits July 17, 2026 17:34
…; surface on-site-gen trap; fix stale update-check

Closes the dogfood gap where the CLI could discover models but not ASSEMBLE
(no download verb; no base-model filter; the training-zip trap; a stale
version-check).

Feature 1 — `civitai download`:
- Deterministic identifier: positional <version-id>, or `--model <id>` resolves
  the model's default (primary/latest) published version. Exactly one required.
- Multi-file: default = primary file; `--file` (exact | unique substring,
  ambiguous errors + lists); `--all`. `--out` (single) / `--out-dir` (dir,
  parent-dir creation).
- Streams body to `<target>.part`, renames on success (never a truncated final
  file); follows the /api/download → signed-storage 302; TTY-aware progress to
  stderr; large files never buffered in memory.
- Uses the configured token source automatically (login token or
  CIVITAI_API_KEY); actionable 401/403 message for gated/early-access.
- SHA256 verify on by default (mismatch deletes .part + errors; missing hash →
  warn+skip, not a hard fail); idempotent skip-if-present + `--force`.
- On-site-gen guard: refuses a selected file whose type != "Model" (training-data
  registration) unless `--allow-nonmodel`.

Feature 2 — `models search --base-model` (repeatable) → REST `baseModels` filter
(verified live: repeated keys = OR across values). Distinguishes video
checkpoints that all share `--type Checkpoint`.

Papercut 3b — `models get` / `model-versions get` human output tags a version
whose primary file is not weights `[training data — no downloadable weights]`
(--json unchanged).

Papercut 5 — update-check: root-caused the "Latest release: v0.1.61 while on
v0.1.62" symptom as the GitHub /releases/latest draft-exclusion artifact (now
resolved: the endpoint returns v0.1.62). Made the compare-guard robust
regardless: current >= latest now shows NO notice (was a misleading older
"Latest release" line), and the cache force-refreshes when it pins a "latest"
older than the running version.

Verified live: baseModels param OR-semantics; model-version file fields
(id/name/type/sizeKB/primary/downloadUrl/hashes.SHA256); a real 24 KB download
end-to-end with SHA256 matching the API's published hash, idempotent skip, and
--model/--out dir creation.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
…, stall timeout

Address four 🟡 audit findings on the `download` command:

1. Path traversal / arbitrary-write: sanitize the SERVER-supplied f.Name via
   filepath.Base in the two server-named modes (default + --out-dir) before
   joining, so a hostile name ("../../etc/foo", "/abs/x") can't escape cwd /
   --out-dir. --out (the user's own explicit path) stays verbatim. An unusable
   basename (".", "/", "..") errors clearly instead of writing junk.

2. Token exfiltration: attach the Bearer only when the download URL is https
   civitai.com/*.civitai.com (exact dotted-suffix, not substring) OR the exact
   configured API origin. An off-domain/non-https signed-storage target is
   fetched WITHOUT the token. 401-refresh path preserved.

3. Real SIGINT cancellation: signal.NotifyContext threads a Ctrl-C-bound context
   into the request + io.Copy so an interrupt cancels the transfer and the
   cleanup defer removes the .part. Also self-heal a stale .part from an aborted
   run (no resume). Makes the PR's "Ctrl-C cleanup" claim actually true.

4. Stall timeout: set ResponseHeaderTimeout (30s) on a CLONE of the transport
   (no overall Timeout — large files stream long) so a silent server can't hang
   forever; shared transport state is untouched.

Tests: table-driven targetPath sanitization + end-to-end hostile-name download;
isTrustedDownloadHost table + off-domain no-auth integration; cancelled-context
mid-download .part cleanup; transport header-timeout unit + behavioral.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
@ZacxDev

ZacxDev commented Jul 17, 2026

Copy link
Copy Markdown
Contributor Author

Audit fixes — 4 🟡 findings addressed (c8eb448)

An adversarial audit of the download command surfaced four medium-severity issues; all are now fixed with focused tests. Full suite green (go build/vet/test ./... + go test -race ./internal/cmd/... ./internal/api/...).

1. Path traversal / arbitrary write from the server-supplied filename (internal/cmd/download.go, targetPath). The API-supplied files[].name was joined into the output path unsanitized, so a hostile name like /home/user/.bashrc or ../../etc/foo could write outside cwd / outside --out-dir. Now the two server-named modes (default and --out-dir) reduce the name to filepath.Base(...) before joining; a degenerate basename (., /, .. — empty/all-slashes name) errors clearly instead of writing junk. --out (the user's own explicit target) is intentionally left verbatim.

2. Bearer token attached to any download host (internal/api/download.go, doDownload). f.DownloadURL is server-supplied, so a hostile value could exfiltrate the token. The token is now attached only when the URL is https civitai.com / *.civitai.com (exact dotted-suffix match — evilcivitai.com and civitai.com.evil.com are rejected) or the exact configured API origin. Off-domain / non-https targets (e.g. the signed object-storage redirect) are fetched with no Authorization. The 401-refresh path is preserved.

3. Real SIGINT / context cancellation + .part cleanup (internal/cmd/download.go). The command previously used context.Background(), so Ctrl-C could not cancel an in-flight transfer. It now uses signal.NotifyContext(..., os.Interrupt) threaded into the request + io.Copy; an interrupt cancels the copy and the existing cleanup defer removes the .part. A stale .part from an aborted run is also self-healed on the next run (no resume).

Correction to the PR description: the earlier "Ctrl-C cleanup" claim was not actually implemented against context.Background() — this change makes it true, with a test that cancels the context mid-stream and asserts no .part/final file is left.

4. No stall timeout on the download client (internal/api/download.go, downloadHTTPClient). There is deliberately no overall Timeout (large files stream for a long time), but a server that accepted the connection then never sent headers could hang forever. A ResponseHeaderTimeout (30s) is now set on a clone of the transport (shared transport state untouched).

Tests added: table-driven targetPath sanitization (hostile names through default + --out-dir, --out verbatim) + an end-to-end hostile-name download asserting the file lands inside --out-dir; isTrustedDownloadHost table (look-alike/substring/scheme cases) + an off-domain integration test asserting no Authorization reaches an untrusted host; a cancelled-context mid-download test asserting .part cleanup; a transport header-timeout unit + behavioral test.

Out of scope (left for fast-follow, as noted in the audit): the 🟢 nits — friendlier --out-is-a-dir error, duplicate compareVersions, >100% progress clamp, --all duplicate-name overwrite.

@ZacxDev
ZacxDev merged commit b839277 into main Jul 17, 2026
9 checks passed
@ZacxDev
ZacxDev deleted the zach/cli-download-and-base-model branch July 17, 2026 23:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant