fail closed on quarantine misconfig and first-seen errors
- 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
This commit is contained in:
@@ -674,24 +674,32 @@ func (e *Engine) checkUpstreamOnce(ctx context.Context, remote models.Remote, pa
|
||||
}
|
||||
|
||||
// checkQuarantine refuses an uncached immutable artifact published less than
|
||||
// quarantine_days ago. The publish time is the upstream Last-Modified, else
|
||||
// the first time this path was seen.
|
||||
// quarantine_days ago. The publish time is the upstream Last-Modified, capped
|
||||
// at now, else the first time this path was seen.
|
||||
func (e *Engine) checkQuarantine(ctx context.Context, remote models.Remote, path string, upstream http.Header) error {
|
||||
if !remote.QuarantineEnabled || remote.QuarantineDays <= 0 {
|
||||
if !remote.QuarantineEnabled {
|
||||
return nil
|
||||
}
|
||||
if a, err := e.db.GetArtifact(ctx, remote.Name, path); err == nil && a != nil {
|
||||
return nil
|
||||
}
|
||||
window := time.Duration(remote.QuarantineDays) * 24 * time.Hour
|
||||
days := remote.QuarantineDays
|
||||
if days < 1 {
|
||||
days = models.DefaultQuarantineDays
|
||||
}
|
||||
window := time.Duration(days) * 24 * time.Hour
|
||||
now := time.Now()
|
||||
published, err := http.ParseTime(upstream.Get("Last-Modified"))
|
||||
if err != nil {
|
||||
if published, err = e.cache.FirstSeen(ctx, remote.Name, path, window+24*time.Hour); err != nil {
|
||||
slog.Warn("first-seen lookup failed, quarantining", "remote", remote.Name, "path", path, "error", err)
|
||||
published = time.Now()
|
||||
slog.Warn("first-seen lookup failed", "remote", remote.Name, "path", path, "error", err)
|
||||
return &ProxyError{Status: http.StatusServiceUnavailable, Message: "quarantine check unavailable", RetryAfter: 30}
|
||||
}
|
||||
}
|
||||
if until := published.Add(window); time.Now().Before(until) {
|
||||
if published.After(now) {
|
||||
published = now
|
||||
}
|
||||
if until := published.Add(window); now.Before(until) {
|
||||
return &ProxyError{
|
||||
Status: http.StatusForbidden,
|
||||
Message: fmt.Sprintf("quarantined: published %s, available from %s", published.UTC().Format(time.RFC3339), until.UTC().Format(time.RFC3339)),
|
||||
@@ -863,6 +871,8 @@ type ProxyError struct {
|
||||
// Published and Available are set only on a quarantine refusal.
|
||||
Published time.Time
|
||||
Available time.Time
|
||||
// RetryAfter, in seconds, is sent as Retry-After when set.
|
||||
RetryAfter int
|
||||
}
|
||||
|
||||
func (e *ProxyError) Error() string { return e.Message }
|
||||
|
||||
@@ -8,6 +8,7 @@ import (
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"git.unkin.net/unkin/artifactapi/internal/cache"
|
||||
"git.unkin.net/unkin/artifactapi/pkg/models"
|
||||
goredis "github.com/redis/go-redis/v9"
|
||||
)
|
||||
@@ -27,6 +28,8 @@ func quarantineUpstream(t *testing.T) *httptest.Server {
|
||||
w.Header().Set("Last-Modified", time.Now().Add(-time.Hour).UTC().Format(http.TimeFormat))
|
||||
case "/old.tgz":
|
||||
w.Header().Set("Last-Modified", time.Now().Add(-30*24*time.Hour).UTC().Format(http.TimeFormat))
|
||||
case "/future.tgz":
|
||||
w.Header().Set("Last-Modified", time.Now().Add(30*24*time.Hour).UTC().Format(http.TimeFormat))
|
||||
case "/undated.tgz", "/pkg":
|
||||
default:
|
||||
http.NotFound(w, r)
|
||||
@@ -136,3 +139,48 @@ func TestQuarantineKeepsServingCached(t *testing.T) {
|
||||
t.Errorf("artifact cached before quarantine refused on refetch: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestQuarantineCapsFutureLastModified(t *testing.T) {
|
||||
requireStack(t)
|
||||
r := quarantineRemote(t, "q-future", true)
|
||||
_, err := testEngine.Fetch(context.Background(), r, "future.tgz", prov(t, models.PackageNPM))
|
||||
pe := wantQuarantined(t, err)
|
||||
if d := time.Until(pe.Available); d < 71*time.Hour || d > 72*time.Hour {
|
||||
t.Errorf("available in %s, want ~72h", d)
|
||||
}
|
||||
}
|
||||
|
||||
func TestQuarantineRedisErrorUnavailable(t *testing.T) {
|
||||
requireStack(t)
|
||||
ctx := context.Background()
|
||||
r := quarantineRemote(t, "q-redis-down", true)
|
||||
r.StaleOnError = true
|
||||
down := must(cache.NewRedis(testRedis))
|
||||
_ = down.Close()
|
||||
eng := NewEngine(testDB, down, testEngine.store)
|
||||
_, err := eng.Fetch(ctx, r, "undated.tgz", prov(t, models.PackageNPM))
|
||||
var pe *ProxyError
|
||||
if !errors.As(err, &pe) || pe.Status != http.StatusServiceUnavailable || pe.RetryAfter == 0 || Quarantined(err) {
|
||||
t.Fatalf("want 503 with Retry-After, got %#v", err)
|
||||
}
|
||||
if a, _ := testDB.GetArtifact(ctx, r.Name, "undated.tgz"); a != nil {
|
||||
t.Error("artifact cached while quarantine check unavailable")
|
||||
}
|
||||
}
|
||||
|
||||
func TestQuarantineStaleOnErrorServesNothingUncached(t *testing.T) {
|
||||
requireStack(t)
|
||||
ctx := context.Background()
|
||||
r := quarantineRemote(t, "q-stale", true)
|
||||
r.StaleOnError = true
|
||||
p := prov(t, models.PackageNPM)
|
||||
_, err := testEngine.Fetch(ctx, r, "fresh.tgz", p)
|
||||
_ = wantQuarantined(t, err)
|
||||
r.BaseURL = "http://127.0.0.1:1"
|
||||
if _, err := testEngine.Fetch(ctx, r, "fresh.tgz", p); err == nil {
|
||||
t.Error("stale_on_error served an uncached quarantined path")
|
||||
}
|
||||
if a, _ := testDB.GetArtifact(ctx, r.Name, "fresh.tgz"); a != nil {
|
||||
t.Error("quarantined artifact was cached")
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user