diff --git a/server/src/__tests__/storage-s3-credential-rotation.test.ts b/server/src/__tests__/storage-s3-credential-rotation.test.ts new file mode 100644 index 00000000000..515c2167ec6 --- /dev/null +++ b/server/src/__tests__/storage-s3-credential-rotation.test.ts @@ -0,0 +1,88 @@ +import { createServer } from "node:http"; +import type { AddressInfo } from "node:net"; +import { afterAll, beforeAll, beforeEach, describe, expect, it } from "vitest"; +import { createS3StorageProvider } from "../storage/s3-provider.js"; +import { isStorageUnavailableError, storageUnavailableMessage, STORAGE_CREDENTIALS_REJECTED_MESSAGE, + waitOutStorageOutages } from "../storage/storage-unavailable.js"; + +const rejection = (code: string) => ` +${code}Injected credential rejectionreq-403`; + +let acceptedKey = "first-access-key"; +let rejectAllUntil = 0; +let rejectionCode = "InvalidAccessKeyId"; +const signedWith: string[] = []; +const server = createServer((req, res) => { + req.resume(); + req.on("end", () => { + const key = /Credential=([^/]+)\//.exec(String(req.headers.authorization ?? ""))?.[1] ?? ""; + signedWith.push(key); + if (key !== acceptedKey || Date.now() < rejectAllUntil) { + res.writeHead(403, { "content-type": "application/xml" }); + res.end(rejection(rejectionCode)); + return; + } + res.writeHead(200, { etag: '"v1"' }); + res.end(); + }); +}); +const savedEnv = { ...process.env }; +let endpoint = ""; + +beforeAll(async () => { + await new Promise(resolve => server.listen(0, "127.0.0.1", resolve)); + endpoint = `http://127.0.0.1:${(server.address() as AddressInfo).port}`; + delete process.env.AWS_SESSION_TOKEN; + delete process.env.AWS_PROFILE; +}); +afterAll(async () => { + process.env = savedEnv; + await new Promise(resolve => server.close(() => resolve())); +}); +beforeEach(() => { + signedWith.length = 0; + acceptedKey = "first-access-key"; + rejectAllUntil = 0; + rejectionCode = "InvalidAccessKeyId"; + process.env.AWS_ACCESS_KEY_ID = "first-access-key"; + process.env.AWS_SECRET_ACCESS_KEY = "first-secret-key"; +}); + +const provider = () => createS3StorageProvider({ bucket: "bucket", region: "us-east-1", endpoint, forcePathStyle: true }); +const put = (storage: ReturnType, objectKey: string) => + storage.putObject({ objectKey, body: Buffer.from("{}"), contentType: "application/json", contentLength: 2 }); + +describe("S3 credentials that expire or rotate during a run (storage chaos ST5)", () => { + it("re-resolves a rotated key and finishes the write instead of failing the run", async () => { + const storage = provider(); + await put(storage, "company/runs/before.json"); + acceptedKey = "second-access-key"; + process.env.AWS_ACCESS_KEY_ID = "second-access-key"; + process.env.AWS_SECRET_ACCESS_KEY = "second-secret-key"; + await waitOutStorageOutages(() => put(storage, "company/runs/after.json")); + expect(signedWith).toEqual(["first-access-key", "first-access-key", "second-access-key"]); + }, 30_000); + + it.each(["ExpiredToken", "InvalidAccessKeyId", "SignatureDoesNotMatch"])( + "waits out a window of 403 %s answers with a valid key", async (code) => { + rejectionCode = code; + rejectAllUntil = Date.now() + 3_000; + await expect(waitOutStorageOutages(() => put(provider(), "company/runs/window.json"))).resolves.toBeUndefined(); + expect(signedWith.length).toBeGreaterThan(1); + }, 30_000); + + it("reports a rejected key as a storage wait with a credentials message outside the wait", async () => { + acceptedKey = "never-issued"; + const error = await put(provider(), "company/runs/rejected.json").catch(thrown => thrown); + expect(isStorageUnavailableError(error)).toBe(true); + expect(storageUnavailableMessage(error)).toBe(STORAGE_CREDENTIALS_REJECTED_MESSAGE); + expect(signedWith).toHaveLength(1); + }); + + it("keeps a 403 AccessDenied a permanent failure", async () => { + rejectionCode = "AccessDenied"; + acceptedKey = "never-issued"; + const error = await put(provider(), "company/runs/denied.json").catch(thrown => thrown); + expect(isStorageUnavailableError(error)).toBe(false); + }); +}); diff --git a/server/src/services/heartbeat/run-completion.ts b/server/src/services/heartbeat/run-completion.ts index 6ab4f3630f8..072df99604f 100644 --- a/server/src/services/heartbeat/run-completion.ts +++ b/server/src/services/heartbeat/run-completion.ts @@ -19,7 +19,7 @@ import { ENVIRONMENT_PROVIDER_UNAVAILABLE_CODE, findEnvironmentProviderUnavailab import { REPOSITORY_CATALOG_UNAVAILABLE_CODE, findRepositoryCatalogUnavailableError } from "../repository-catalog-unavailable-error.js"; import { findWorkspaceProviderFailure } from "../workspace-provider-failure.js"; import { isRunnerRevisionMismatch, RUNNER_REVISION_MISMATCH_CODE, RUNNER_REVISION_MISMATCH_RETRY } from "../runner-revision-mismatch.js"; -import { isStorageUnavailableError, STORAGE_UNAVAILABLE_CODE, STORAGE_UNAVAILABLE_EXHAUSTED_MESSAGE, STORAGE_UNAVAILABLE_MESSAGE, STORAGE_UNAVAILABLE_RETRY } from "../../storage/storage-unavailable.js"; +import { isStorageUnavailableError, STORAGE_UNAVAILABLE_CODE, STORAGE_UNAVAILABLE_EXHAUSTED_MESSAGE, STORAGE_UNAVAILABLE_RETRY, storageUnavailableMessage } from "../../storage/storage-unavailable.js"; import { type RunSessionOutcome, resolveNextSessionState, normalizeUsageTotals, resolveCacheAdjustedCostUsd, resolveLedgerBiller, resolveLedgerCostStatus, normalizeLedgerBillingType } from "./run-state.js"; import { readGitConnectionFailure } from "../git-connection-failure.js"; import { CONFIGURATION_INCOMPLETE_FAILURE_CODE, ConfigurationIncompleteFailure } from "./run-preparation.js"; @@ -1146,7 +1146,7 @@ export function createHeartbeatRunCompletion(db: Db, dependencies: HeartbeatRunC : null; const message = interruptedExactResume ? describeRefusedExactResume(interruptedExactResume, thrownMessage) - : isStorageUnavailableError(err) ? STORAGE_UNAVAILABLE_MESSAGE : thrownMessage; + : isStorageUnavailableError(err) ? storageUnavailableMessage(err) : thrownMessage; const failureErrorCode = workspaceValidationFailure?.code ?? configurationIncompleteFailure?.code ?? @@ -1381,7 +1381,7 @@ export function createHeartbeatRunCompletion(db: Db, dependencies: HeartbeatRunC const providerFailure = providerFailureFromError(outerErr); const currentUserRedactionOptions = await getCurrentUserRedactionOptions(); const storageUnavailable = isStorageUnavailableError(outerErr); - const message = storageUnavailable ? STORAGE_UNAVAILABLE_MESSAGE : redactCurrentUserText( + const message = storageUnavailable ? storageUnavailableMessage(outerErr) : redactCurrentUserText( identityRedactor.redact(outerErr instanceof Error ? outerErr.message : "Unknown setup failure"), diff --git a/server/src/storage/s3-credential-refresh.test.ts b/server/src/storage/s3-credential-refresh.test.ts new file mode 100644 index 00000000000..396e20b8927 --- /dev/null +++ b/server/src/storage/s3-credential-refresh.test.ts @@ -0,0 +1,54 @@ +import { createServer } from "node:http"; +import type { AddressInfo } from "node:net"; +import { PutObjectCommand, S3Client } from "@aws-sdk/client-s3"; +import { afterAll, beforeAll, describe, expect, it } from "vitest"; +import { refreshS3CredentialsOnRejection } from "./s3-credential-refresh.js"; + +const REJECTION = ` +InvalidAccessKeyIdInjected rejectionreq-403`; + +let acceptedKey = "first"; +const delays = [50, 400]; +let rejected = 0; +const server = createServer((req, res) => { + req.resume(); + req.on("end", () => { + const key = /Credential=([^/]+)\//.exec(String(req.headers.authorization ?? ""))?.[1] ?? ""; + if (key === acceptedKey) { res.writeHead(200, { etag: '"v1"' }); res.end(); return; } + setTimeout(() => { + res.writeHead(403, { "content-type": "application/xml" }); + res.end(REJECTION); + }, delays[rejected++] ?? 0); + }); +}); +let endpoint = ""; + +beforeAll(async () => { + await new Promise(resolve => server.listen(0, "127.0.0.1", resolve)); + endpoint = `http://127.0.0.1:${(server.address() as AddressInfo).port}`; +}); +afterAll(async () => { await new Promise(resolve => server.close(() => resolve())); }); + +describe("S3 credential refresh after a rejection", () => { + it("re-resolves the key once when parallel requests signed with the old key are rejected at different times", async () => { + let issued = "first"; + let lookups = 0; + const client = new S3Client({ endpoint, region: "us-east-1", forcePathStyle: true, maxAttempts: 1, + credentials: async () => { lookups += 1; return { accessKeyId: issued, secretAccessKey: `${issued}-secret` }; } }); + refreshS3CredentialsOnRejection(client); + const put = (key: string) => client.send(new PutObjectCommand({ Bucket: "bucket", Key: key, Body: "{}" })); + try { + await put("warm"); + expect(lookups).toBe(1); + acceptedKey = "second"; + issued = "second"; + const results = await Promise.allSettled([put("a"), put("b")]); + expect(results.map(result => result.status)).toEqual(["rejected", "rejected"]); + expect(lookups).toBe(2); + await put("after"); + expect(lookups).toBe(2); + } finally { + client.destroy(); + } + }); +}); diff --git a/server/src/storage/s3-credential-refresh.ts b/server/src/storage/s3-credential-refresh.ts new file mode 100644 index 00000000000..3e2a0ac557d --- /dev/null +++ b/server/src/storage/s3-credential-refresh.ts @@ -0,0 +1,41 @@ +import type { S3Client } from "@aws-sdk/client-s3"; +import { isCredentialRejectionCode } from "./storage-unavailable.js"; + +type CredentialProvider = (options?: { forceRefresh?: boolean }) => Promise; + +function rejectsCredentials(error: unknown): boolean { + if (!error || typeof error !== "object") return false; + const { name, Code, $metadata } = error as { name?: unknown; Code?: unknown; $metadata?: { httpStatusCode?: unknown } }; + const status = typeof $metadata?.httpStatusCode === "number" ? $metadata.httpStatusCode : null; + return isCredentialRejectionCode(name, status) || isCredentialRejectionCode(Code, status); +} + +export function refreshS3CredentialsOnRejection(client: S3Client): void { + let refreshing: Promise | null = null; + const provider = () => { + const credentials = client.config.credentials as unknown as CredentialProvider | undefined; + return typeof credentials === "function" ? credentials : null; + }; + const current = () => provider()?.().catch(() => undefined) ?? Promise.resolve(undefined); + const refresh = async (used: unknown) => { + const credentials = provider(); + if (!credentials) return; + if (!refreshing) { + if (used !== undefined && await current() !== used) return; + refreshing ??= credentials({ forceRefresh: true }).catch(() => undefined).finally(() => { refreshing = null; }); + } + await refreshing; + }; + client.middlewareStack.add( + (next) => async (args) => { + const used = await current(); + try { + return await next(args); + } catch (error) { + if (rejectsCredentials(error)) await refresh(used); + throw error; + } + }, + { step: "initialize", priority: "low", name: "paperclipS3CredentialRefresh" }, + ); +} diff --git a/server/src/storage/s3-provider.ts b/server/src/storage/s3-provider.ts index 395c4b5e2dc..7ff5012f4b4 100644 --- a/server/src/storage/s3-provider.ts +++ b/server/src/storage/s3-provider.ts @@ -14,6 +14,7 @@ import { recordS3BodyFailure, withS3FailureDiagnostics } from "./s3-failure.js"; import { sendThroughStorageOutage } from "./storage-unavailable.js"; import { addAbortSignal, Readable } from "node:stream"; import { retryS3RequestTimeouts } from "./s3-request-timeout-retry.js"; +import { refreshS3CredentialsOnRejection } from "./s3-credential-refresh.js"; import type { StorageProvider, GetObjectResult, HeadObjectResult } from "./types.js"; import { notFound, unprocessable } from "../errors.js"; @@ -137,6 +138,7 @@ export function createS3StorageProvider(config: S3ProviderConfig): StorageProvid forcePathStyle: Boolean(config.forcePathStyle), }); retryS3RequestTimeouts(client); + refreshS3CredentialsOnRejection(client); // File sources carry Content-MD5 as their integrity check. Send plain bodies // so the backend verifies that digest directly; the default client wraps // bodies in aws-chunked framing with its own trailing checksum. @@ -147,6 +149,7 @@ export function createS3StorageProvider(config: S3ProviderConfig): StorageProvid requestChecksumCalculation: "WHEN_REQUIRED", }); retryS3RequestTimeouts(fileClient); + refreshS3CredentialsOnRejection(fileClient); async function listExactVersions(key: string, signal?: AbortSignal) { const versions = new Set(), cursors = new Set(); diff --git a/server/src/storage/storage-unavailable.test.ts b/server/src/storage/storage-unavailable.test.ts index 8fa8deb5a42..6cf259be41c 100644 --- a/server/src/storage/storage-unavailable.test.ts +++ b/server/src/storage/storage-unavailable.test.ts @@ -26,6 +26,10 @@ describe("storage unavailability", () => { ["408 RequestTimeout", s3Error("RequestTimeout", 408)], ["a reset recorded under its error name", Object.assign(new Error("socket hang up"), { code: "ECONNRESET" })], ["a refused connection without a status", s3Error("Error", undefined, "ECONNREFUSED")], + ["400 ExpiredToken", s3Error("ExpiredToken", 400)], + ["403 ExpiredToken", s3Error("ExpiredToken", 403)], + ["403 InvalidAccessKeyId", s3Error("InvalidAccessKeyId", 403)], + ["403 SignatureDoesNotMatch", s3Error("SignatureDoesNotMatch", 403)], ])("treats %s from the store as unavailable", async (_label, error) => { expect(isStorageUnavailableError(await recorded(error))).toBe(true); }); @@ -33,6 +37,8 @@ describe("storage unavailability", () => { it.each([ ["404 NoSuchKey", s3Error("NoSuchKey", 404)], ["403 AccessDenied", s3Error("AccessDenied", 403)], + ["a credential source that could not load a key", s3Error("CredentialsProviderError")], + ["a credential code without a store status", s3Error("ExpiredToken")], ["412 PreconditionFailed", s3Error("PreconditionFailed", 412)], ["an ordinary error", new Error("adapter crashed")], ])("does not treat %s as an outage", async (_label, error) => { diff --git a/server/src/storage/storage-unavailable.ts b/server/src/storage/storage-unavailable.ts index 395357b9575..865f341643e 100644 --- a/server/src/storage/storage-unavailable.ts +++ b/server/src/storage/storage-unavailable.ts @@ -9,7 +9,9 @@ export const STORAGE_UNAVAILABLE_WAIT_RETRY_REASON = "storage_unavailable_wait"; export const STORAGE_UNAVAILABLE_MESSAGE = "File storage was unavailable, so this attempt stopped before it could save its work. Paperclip will retry automatically when storage is back."; -export const STORAGE_UNAVAILABLE_EXHAUSTED_MESSAGE = "File storage stayed unavailable through every automatic retry, so Paperclip stopped retrying this task. Retry once storage is back."; +export const STORAGE_CREDENTIALS_REJECTED_MESSAGE = "File storage rejected Paperclip's storage credentials (they may have expired or been rotated), so this attempt stopped before it could save its work. Paperclip will retry automatically when valid credentials are available."; + +export const STORAGE_UNAVAILABLE_EXHAUSTED_MESSAGE = "File storage stayed unavailable or kept rejecting Paperclip's storage credentials through every automatic retry, so Paperclip stopped retrying this task. Check the storage service and its credentials, then Retry."; export const STORAGE_UNAVAILABLE_RETRY = { retryReason: STORAGE_UNAVAILABLE_WAIT_RETRY_REASON, wakeReason: "storage_unavailable_retry", maxAttempts: 10, delayMs: 60_000, @@ -20,6 +22,15 @@ const UNAVAILABLE_NAMES = new Set(["ServiceUnavailable", "SlowDown", "InternalEr const UNAVAILABLE_CODES = new Set(["ECONNRESET", "ECONNREFUSED", "ECONNABORTED", "ETIMEDOUT", "EPIPE", "EAI_AGAIN", "ENOTFOUND", "ERR_STREAM_PREMATURE_CLOSE", "UND_ERR_SOCKET"]); +const CREDENTIAL_REJECTION_CODES = new Set(["ExpiredToken", "ExpiredTokenException", "TokenRefreshRequired", "InvalidToken", + "InvalidAccessKeyId", "InvalidClientTokenId", "SignatureDoesNotMatch"]); +const CREDENTIAL_REJECTION_STATUS = new Set([400, 403]); + +export function isCredentialRejectionCode(code: unknown, status: number | null | undefined): boolean { + return typeof code === "string" && CREDENTIAL_REJECTION_CODES.has(code) + && typeof status === "number" && CREDENTIAL_REJECTION_STATUS.has(status); +} + function ownValue(value: unknown, key: string): unknown { if (!value || typeof value !== "object") return undefined; try { @@ -46,10 +57,20 @@ function recordedStorageFailure(error: unknown) { return null; } +export function isStorageCredentialRejection(error: unknown): boolean { + const recorded = recordedStorageFailure(error); + return recorded !== null && isCredentialRejectionCode(recorded.diagnostics.errorCode, recorded.diagnostics.httpStatus); +} + +export function storageUnavailableMessage(error: unknown): string { + return isStorageCredentialRejection(error) ? STORAGE_CREDENTIALS_REJECTED_MESSAGE : STORAGE_UNAVAILABLE_MESSAGE; +} + export function isStorageUnavailableError(error: unknown): boolean { const recorded = recordedStorageFailure(error); if (!recorded) return false; const { diagnostics } = recorded; + if (isCredentialRejectionCode(diagnostics.errorCode, diagnostics.httpStatus)) return true; const status = diagnostics.httpStatus; if (status !== null && (status < 200 || status >= 300)) return UNAVAILABLE_STATUS.has(status); return UNAVAILABLE_NAMES.has(diagnostics.errorCode) || UNAVAILABLE_CODES.has(diagnostics.errorCode) || networkFailure(recorded.error);