Files
PaperClipAI/server/src/storage/s3-credential-refresh.test.ts
T
DottaandClaude Opus 5.5 693de27c90 fix(storage): wait out expired or rotated S3 credentials and re-resolve the key (#20456)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work
> - In cloud mode, run logs, provider traces, attachments and remote
workspace snapshots go to S3
> - The server uses the default AWS credential chain, and real keys
expire or rotate while runs are in flight
> - #20299 made runs wait out a storage outage, but a 403
`ExpiredToken`, `InvalidAccessKeyId` or `SignatureDoesNotMatch` was a
permanent `adapter_failed`
> - The SDK also caches a key until its stated expiration (forever for a
key with none), so a rotated key was never picked up
> - This pull request treats credential rejections as a storage wait and
re-resolves the key from the credential chain after a rejection
> - The benefit is that runs survive key expiry and rotation without
failed attempts or a manual Retry

## Linked Issues or Issue Description

Refs #20299 (storage outage wait). Harness that found this: #20455.

**What existing behavior does this improve?**
Runs wait out S3 outages (5xx, timeouts, resets) but fail at once when
the store rejects the server's credentials.

**Subsystem affected**
server storage (S3 provider) and heartbeat run completion.

**Current behavior**
Restart-chaos wave w-20261110T041556-s501 (`--storage s3
--storage-faults ST5`): one 41 s window of 403 `ExpiredToken` failed 4
of 6 tasks. Each run ended `adapter_failed` with "The provided token has
expired.", and the automatic retries failed within one second, so the
tasks needed a board Retry (R3 `retry_did_not_complete`).

**Proposed behavior**
A credential rejection is a transient infrastructure condition.
Critical-path storage calls wait it out like an outage, and a run that
stops on it gets the `storage_unavailable` wait retry. After each
rejection the S3 client re-resolves its key with `forceRefresh`, so the
next attempt signs with a rotated key. The run error names the
credentials. If they never come back, the exhausted-wait message tells
the operator to check the storage service and its credentials.

**Reason and benefit**
Expiring or rotated keys are normal in cloud deployments. They must not
fail work.

## What Changed

- `storage-unavailable.ts`: `ExpiredToken`, `ExpiredTokenException`,
`TokenRefreshRequired`, `InvalidToken`, `InvalidAccessKeyId`,
`InvalidClientTokenId` and `SignatureDoesNotMatch` answered by the store
with 400/403 count as storage unavailable. `AccessDenied` and a
credential chain that cannot load any key (`CredentialsProviderError`)
stay permanent. New credential-specific run message; the exhausted
message also names credentials.
- `s3-credential-refresh.ts` (new): S3 client middleware that calls the
client's credential provider with `forceRefresh` after a rejection, once
per credential generation (requests that signed with an already-replaced
key do not refresh again), then rethrows so the existing storage wait
owns the retry.
- `s3-provider.ts`: installs the middleware on both S3 clients.
- `run-completion.ts`: uses the credential-aware message.

## Verification

- `cd server && npx vitest run
src/__tests__/storage-s3-credential-rotation.test.ts src/storage/
src/__tests__/storage-outage-wait.test.ts
src/__tests__/storage-s3-transient-retry.test.ts
src/__tests__/storage-s3-failure-diagnostics.test.ts
src/services/heartbeat/run-completion.test.ts` (186 pass).
- The new `storage-s3-credential-rotation.test.ts` fails 5 of 6 cases on
master (red) and passes with this change (green). It covers a real key
rotation through the default credential chain, 403 windows for each
code, the message, and `AccessDenied` staying permanent.
- `cd server && npx tsc --noEmit` passes.
- `s3-credential-refresh.test.ts`: two parallel requests signed with the
old key, rejected 50 ms and 400 ms apart, cause one forced lookup (3
before the generation check, 2 after).
- Restart-chaos rerun on a build with this fix and #20455:
w-20261110T050044-s503 (`--env local --workloads A1,C1 --faults K1,K2
--tasks 8 --storage s3 --storage-faults ST5,ST5R --storage-fault-seconds
30-90`) passed 8/8 tasks (R1–R6 and T) with a 39 s `ExpiredToken` window
and an 85 s real key rotation (76 credential 403s). Before the fix,
w-20261110T041556-s501 failed 4/6 tasks.

## Risks

- Scope: calls with a Readable body (streamed `putObject`, buffered
multipart) cannot be resent, so they still fail on the first rejection,
as they do for outages. The middleware has already re-resolved the key,
so the caller's next attempt (the run-level wait retry for run work)
uses the new key.

- A key that the store rejects for good (for example a wrong secret) now
waits up to the storage wait budget (5 min in the call, then up to 10
run retries) before the board shows the exhausted message, instead of
failing at once.
- `forceRefresh` re-runs the credential chain after a rejection. Calls
are coalesced, so a burst of rejections causes one refresh.

## Model Used

- Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, with tool use and code
execution in Claude Code.

## Checklist

- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-11-10 00:33:39 -06:00

55 lines
2.3 KiB
TypeScript

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 = `<?xml version="1.0" encoding="UTF-8"?>
<Error><Code>InvalidAccessKeyId</Code><Message>Injected rejection</Message><RequestId>req-403</RequestId></Error>`;
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<void>(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<void>(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();
}
});
});