Enforce remote quarantine for fresh immutable artifacts #170

Merged

benvin merged 2 commits from benvin/remote-quarantine into master 2026-10-25 15:16:23 +11:00

Conversation 2 Commits 2 Files Changed 13

+451 -10

unkin-agent commented 2026-10-25 13:26:39 +11:00

Member

Copy Link

Copy Source

Remotes store quarantine_enabled/quarantine_days but the proxy never enforces them, so freshly published (possibly malicious) artifacts are cached immediately.

• refuse uncached immutable paths published under quarantine_days ago with a JSON 403
• take publish time from Last-Modified capped at now, else a Redis first-seen key; 503 + Retry-After when that lookup fails
• reject enabled quarantine with quarantine_days<1; backfill stored <=0 to 3
• default quarantine_days to 3 on create and keep it on update
• let goproxy virtual probes skip a quarantining member

Remotes store `quarantine_enabled`/`quarantine_days` but the proxy never enforces them, so freshly published (possibly malicious) artifacts are cached immediately. - refuse uncached immutable paths published under `quarantine_days` ago with a JSON 403 - take publish time from `Last-Modified` capped at now, else a Redis first-seen key; 503 + `Retry-After` when that lookup fails - reject enabled quarantine with `quarantine_days<1`; backfill stored `<=0` to 3 - default `quarantine_days` to 3 on create and keep it on update - let goproxy virtual probes skip a quarantining member

unkin-agent added 1 commit 2026-10-25 13:26:39 +11:00

enforce remote quarantine for fresh immutable artifacts ... 62c009b837

- refuse uncached immutable paths published under quarantine_days ago with a JSON 403
- take publish time from Last-Modified, else a Redis first-seen key
- let goproxy virtual probes skip a quarantining member
- default quarantine_days to 3 on create and keep it on update
- document quarantine in caching.md

unkin-agent commented 2026-10-25 13:30:11 +11:00

Author

Member

Copy Link

Copy Source

• internal/proxy/engine.go:680 — QuarantineDays <= 0 silently disables quarantine while quarantine_enabled=true; the old create path stored 0 (Go zero, not the column default 3), so existing enabled remotes created without quarantine_days stay unprotected, and PUT {"quarantine_days":0} persists 0 → add a migration setting quarantine_days=3 where it is 0 and reject days < 1 when enabled in remotes.go create/update.
• internal/proxy/engine.go:687 — Last-Modified is trusted unbounded: a future date (clock skew) extends the block past quarantine_days, and a mirror re-sync that bumps mtime on old files re-quarantines them → clamp published to min(published, now).
• internal/proxy/engine.go:689-691 — Redis error becomes published = now, so the client gets a 403 claiming a fresh publish with a full-window available time for what is an infra fault → return 503 (+Retry-After) on FirstSeen failure, keep it fail-closed.
• internal/api/v2/remotes.go:78,132 — default/preserve change for quarantine_days plus the goproxy virtual probe change ride with the engine enforcement → split into 3 PRs: (1) engine+proxy.go enforcement, (2) goproxy_merger probe, (3) remotes.go default/preserve.
• nit: internal/proxy/quarantine_test.go — no test for the Redis-error fail-closed branch, a future Last-Modified, or stale_on_error with a quarantined path → add them.

- internal/proxy/engine.go:680 — `QuarantineDays <= 0` silently disables quarantine while `quarantine_enabled=true`; the old create path stored 0 (Go zero, not the column default 3), so existing enabled remotes created without `quarantine_days` stay unprotected, and `PUT {"quarantine_days":0}` persists 0 → add a migration setting `quarantine_days=3` where it is 0 and reject days < 1 when enabled in remotes.go create/update. - internal/proxy/engine.go:687 — Last-Modified is trusted unbounded: a future date (clock skew) extends the block past `quarantine_days`, and a mirror re-sync that bumps mtime on old files re-quarantines them → clamp `published` to `min(published, now)`. - internal/proxy/engine.go:689-691 — Redis error becomes `published = now`, so the client gets a 403 claiming a fresh publish with a full-window `available` time for what is an infra fault → return 503 (+Retry-After) on FirstSeen failure, keep it fail-closed. - internal/api/v2/remotes.go:78,132 — default/preserve change for `quarantine_days` plus the goproxy virtual probe change ride with the engine enforcement → split into 3 PRs: (1) engine+proxy.go enforcement, (2) goproxy_merger probe, (3) remotes.go default/preserve. - nit: internal/proxy/quarantine_test.go — no test for the Redis-error fail-closed branch, a future Last-Modified, or `stale_on_error` with a quarantined path → add them.

unkin-agent added 1 commit 2026-10-25 13:36:13 +11:00

fail closed on quarantine misconfig and first-seen errors ... 7cd0edf7ff

- backfill quarantine_days<=0 to 3 with migration 0006
- reject quarantine_enabled with quarantine_days<1 on create and update
- cap upstream Last-Modified at now
- answer 503 with Retry-After when the first-seen lookup fails

unkin-agent commented 2026-10-25 13:40:41 +11:00

Author

Member

Copy Link

Copy Source

• internal/proxy/engine.go:694 — first-seen key TTL is window+24h, so a path not requested for >4 days (3d window) loses its key and the next request records a fresh first-seen, quarantining an already-eligible artifact for another full window → drop the TTL to a long retention (e.g. 30d) or persist first-seen in Postgres; add a test that a key past the window still resolves as eligible.
• internal/virtual/goproxy_merger.go:210 — a quarantine-check 503 (Redis down) from a member is flattened by %v into a generic error, losing status and Retry-After → return/wrap with %w and map ProxyError status in the caller.
• nit: internal/api/v1/proxyerror_test.go:17 — no test for the HEAD path returning 403/503 on a miss; add one for Head with a fresh undated artifact and Redis down.

- internal/proxy/engine.go:694 — first-seen key TTL is window+24h, so a path not requested for >4 days (3d window) loses its key and the next request records a fresh first-seen, quarantining an already-eligible artifact for another full window → drop the TTL to a long retention (e.g. 30d) or persist first-seen in Postgres; add a test that a key past the window still resolves as eligible. - internal/virtual/goproxy_merger.go:210 — a quarantine-check 503 (Redis down) from a member is flattened by `%v` into a generic error, losing status and Retry-After → return/wrap with %w and map ProxyError status in the caller. - nit: internal/api/v1/proxyerror_test.go:17 — no test for the HEAD path returning 403/503 on a miss; add one for Head with a fresh undated artifact and Redis down.

benvin merged commit 6cf84c4f6c into master 2026-10-25 15:16:23 +11:00

benvin deleted branch benvin/remote-quarantine 2026-10-25 15:16:24 +11:00

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

No items

No Label

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

benvin (BenVincent) unkin-agent (Unkin Agent) unkinben (Ben Vincent)

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: unkin/artifactapi#170