Skip to content

Commit 20c60fb

Browse files
committed
fix: replace mountBucket() with direct s3fs mount to prevent passwd duplication
sandbox.mountBucket() manages the s3fs passwd file from the orchestration layer outside the container and appends a new credential entry on every call. Because the container persists across Worker invocations the entries accumulate and s3fs refuses to mount with "multiple entries for the same bucket(default) in the passwd file." In-container cleanup (rm, sort -u) cannot reach the orchestration-layer file. Replace mountBucket() with direct s3fs mounting inside the container, following the pattern from the Cloudflare Containers FUSE-mount docs: 1. Write credentials to /etc/passwd-s3fs via startProcess (overwrite, not append — always exactly one entry) 2. Run s3fs directly inside the container 3. Verify the mount succeeded Credentials are passed via process env vars (R2_KEY, R2_SECRET) to avoid embedding secrets in the command string. The in-flight promise lock is retained to coalesce concurrent callers within the same Worker isolate. https://claude.ai/code/session_01E5t9gPHDGGrTUWeagkDjVo
1 parent e5bac4b commit 20c60fb

File tree

2 files changed

+160
-172
lines changed

2 files changed

+160
-172
lines changed

src/gateway/r2.test.ts

Lines changed: 82 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,15 @@ describe('mountR2Storage', () => {
6565
});
6666

6767
describe('mounting behavior', () => {
68-
it('mounts R2 bucket when credentials provided and not already mounted', async () => {
69-
const { sandbox, mountBucketMock } = createMockSandbox({ mounted: false });
68+
it('mounts R2 via s3fs when credentials provided and not already mounted', async () => {
69+
const { sandbox, startProcessMock } = createMockSandbox({ mounted: false });
70+
// isR2Mounted (not mounted) → passwd setup → s3fs mount → isR2Mounted (mounted)
71+
startProcessMock
72+
.mockResolvedValueOnce(createMockProcess('')) // isR2Mounted check
73+
.mockResolvedValueOnce(createMockProcess('')) // passwd file write
74+
.mockResolvedValueOnce(createMockProcess('')) // s3fs mount
75+
.mockResolvedValueOnce(createMockProcess('s3fs on /data/moltbot type fuse.s3fs\n')); // verify
76+
7077
const env = createMockEnvWithR2({
7178
R2_ACCESS_KEY_ID: 'key123',
7279
R2_SECRET_ACCESS_KEY: 'secret',
@@ -76,159 +83,152 @@ describe('mountR2Storage', () => {
7683
const result = await mountR2Storage(sandbox, env);
7784

7885
expect(result).toBe(true);
79-
expect(mountBucketMock).toHaveBeenCalledWith('moltbot-data', '/data/moltbot', {
80-
endpoint: 'https://account123.r2.cloudflarestorage.com',
81-
credentials: {
82-
accessKeyId: 'key123',
83-
secretAccessKey: 'secret',
84-
},
85-
});
86+
// Verify passwd file is written with env vars (not embedded in command)
87+
expect(startProcessMock).toHaveBeenCalledWith(
88+
expect.stringContaining('passwd-s3fs'),
89+
expect.objectContaining({
90+
env: { R2_KEY: 'key123', R2_SECRET: 'secret' },
91+
}),
92+
);
93+
// Verify s3fs mount command
94+
expect(startProcessMock).toHaveBeenCalledWith(
95+
expect.stringContaining('s3fs moltbot-data /data/moltbot'),
96+
);
8697
});
8798

8899
it('uses custom bucket name from R2_BUCKET_NAME env var', async () => {
89-
const { sandbox, mountBucketMock } = createMockSandbox({ mounted: false });
90-
const env = createMockEnvWithR2({
91-
R2_ACCESS_KEY_ID: 'key123',
92-
R2_SECRET_ACCESS_KEY: 'secret',
93-
CF_ACCOUNT_ID: 'account123',
94-
R2_BUCKET_NAME: 'moltbot-e2e-test123',
95-
});
100+
const { sandbox, startProcessMock } = createMockSandbox({ mounted: false });
101+
startProcessMock
102+
.mockResolvedValueOnce(createMockProcess(''))
103+
.mockResolvedValueOnce(createMockProcess(''))
104+
.mockResolvedValueOnce(createMockProcess(''))
105+
.mockResolvedValueOnce(createMockProcess('s3fs on /data/moltbot type fuse.s3fs\n'));
106+
107+
const env = createMockEnvWithR2({ R2_BUCKET_NAME: 'custom-bucket' });
96108

97109
const result = await mountR2Storage(sandbox, env);
98110

99111
expect(result).toBe(true);
100-
expect(mountBucketMock).toHaveBeenCalledWith(
101-
'moltbot-e2e-test123',
102-
'/data/moltbot',
103-
expect.any(Object),
112+
expect(startProcessMock).toHaveBeenCalledWith(
113+
expect.stringContaining('s3fs custom-bucket /data/moltbot'),
104114
);
105115
});
106116

107117
it('returns true immediately when bucket is already mounted', async () => {
108-
const { sandbox, mountBucketMock } = createMockSandbox({ mounted: true });
118+
const { sandbox, startProcessMock } = createMockSandbox({ mounted: true });
109119
const env = createMockEnvWithR2();
110120

111121
const result = await mountR2Storage(sandbox, env);
112122

113123
expect(result).toBe(true);
114-
expect(mountBucketMock).not.toHaveBeenCalled();
124+
// Only one startProcess call (the isR2Mounted check) — no mount attempted
125+
expect(startProcessMock).toHaveBeenCalledTimes(1);
115126
expect(console.log).toHaveBeenCalledWith('R2 bucket already mounted at', '/data/moltbot');
116127
});
117128

118-
it('logs success message when mounted successfully', async () => {
119-
const { sandbox } = createMockSandbox({ mounted: false });
120-
const env = createMockEnvWithR2();
121-
122-
await mountR2Storage(sandbox, env);
123-
124-
expect(console.log).toHaveBeenCalledWith(
125-
'R2 bucket mounted successfully - moltbot data will persist across sessions',
126-
);
127-
});
128-
});
129-
130-
describe('error handling', () => {
131-
it('returns false when mountBucket throws and mount check fails', async () => {
129+
it('does not call mountBucket — uses direct s3fs instead', async () => {
132130
const { sandbox, mountBucketMock, startProcessMock } = createMockSandbox({ mounted: false });
133-
mountBucketMock.mockRejectedValue(new Error('Mount failed'));
134-
// isR2Mounted (not mounted) → deduplicateS3fsPasswd → isR2Mounted after error (not mounted)
135131
startProcessMock
136132
.mockResolvedValueOnce(createMockProcess(''))
137133
.mockResolvedValueOnce(createMockProcess(''))
138-
.mockResolvedValueOnce(createMockProcess(''));
134+
.mockResolvedValueOnce(createMockProcess(''))
135+
.mockResolvedValueOnce(createMockProcess('s3fs on /data/moltbot type fuse.s3fs\n'));
139136

140137
const env = createMockEnvWithR2();
141138

142-
const result = await mountR2Storage(sandbox, env);
139+
await mountR2Storage(sandbox, env);
143140

144-
expect(result).toBe(false);
145-
expect(console.error).toHaveBeenCalledWith('Failed to mount R2 bucket:', expect.any(Error));
141+
expect(mountBucketMock).not.toHaveBeenCalled();
146142
});
143+
});
147144

148-
it('returns true if mount fails but check shows it is actually mounted', async () => {
149-
const { sandbox, mountBucketMock, startProcessMock } = createMockSandbox();
145+
describe('error handling', () => {
146+
it('returns false when s3fs mount fails and post-mount check fails', async () => {
147+
const { sandbox, startProcessMock } = createMockSandbox({ mounted: false });
150148
startProcessMock
151-
.mockResolvedValueOnce(createMockProcess('')) // isR2Mounted before mount
152-
.mockResolvedValueOnce(createMockProcess('')) // deduplicateS3fsPasswd
153-
.mockResolvedValueOnce(createMockProcess('s3fs on /data/moltbot type fuse.s3fs\n')); // isR2Mounted after error
154-
155-
mountBucketMock.mockRejectedValue(new Error('Transient error'));
149+
.mockResolvedValueOnce(createMockProcess('')) // isR2Mounted (not mounted)
150+
.mockResolvedValueOnce(createMockProcess('')) // passwd write
151+
.mockResolvedValueOnce(createMockProcess('', { exitCode: 1, stderr: 'mount error' })) // s3fs fails
152+
.mockResolvedValueOnce(createMockProcess('')) // verify (not mounted)
153+
.mockResolvedValueOnce(createMockProcess('')); // final check (not mounted)
156154

157155
const env = createMockEnvWithR2();
158156

159157
const result = await mountR2Storage(sandbox, env);
160158

161-
expect(result).toBe(true);
162-
expect(console.log).toHaveBeenCalledWith('R2 bucket is mounted despite error');
159+
expect(result).toBe(false);
160+
expect(console.error).toHaveBeenCalledWith(
161+
'Failed to mount R2 bucket: s3fs mount did not succeed',
162+
);
163163
});
164164

165-
it('deduplicates passwd and retries s3fs on "multiple entries" error', async () => {
166-
const { sandbox, mountBucketMock, startProcessMock } = createMockSandbox({ mounted: false });
167-
mountBucketMock.mockRejectedValue(
168-
new Error('S3FSMountError: s3fs: there are multiple entries for the same bucket(default)'),
169-
);
165+
it('returns true if mount check passes despite errors during setup', async () => {
166+
const { sandbox, startProcessMock } = createMockSandbox();
170167
startProcessMock
171-
.mockResolvedValueOnce(createMockProcess('')) // isR2Mounted before mount
172-
.mockResolvedValueOnce(createMockProcess('')) // deduplicateS3fsPasswd (pre-mount)
173-
// After "multiple entries" error:
174-
.mockResolvedValueOnce(createMockProcess('')) // deduplicateS3fsPasswd (retry)
175-
.mockResolvedValueOnce(createMockProcess('')) // s3fs direct retry
176-
.mockResolvedValueOnce( // isR2Mounted after retry
177-
createMockProcess('s3fs on /data/moltbot type fuse.s3fs\n'),
178-
);
168+
.mockResolvedValueOnce(createMockProcess('')) // isR2Mounted (not mounted)
169+
.mockRejectedValueOnce(new Error('startProcess failed')) // passwd write throws
170+
.mockResolvedValueOnce(createMockProcess('s3fs on /data/moltbot type fuse.s3fs\n')); // final check
179171

180172
const env = createMockEnvWithR2();
181173

182174
const result = await mountR2Storage(sandbox, env);
183175

184176
expect(result).toBe(true);
185177
expect(console.log).toHaveBeenCalledWith(
186-
'Deduplicating s3fs passwd files and retrying mount...',
178+
'R2 bucket is mounted despite errors during setup',
187179
);
188-
expect(console.log).toHaveBeenCalledWith('R2 bucket is mounted despite error');
189180
});
190181
});
191182

192183
describe('concurrent mount protection', () => {
193-
it('only calls mountBucket once when invoked concurrently', async () => {
194-
const { sandbox, mountBucketMock } = createMockSandbox({ mounted: false });
184+
it('only runs mount once when invoked concurrently', async () => {
185+
const { sandbox, startProcessMock } = createMockSandbox({ mounted: false });
186+
startProcessMock
187+
.mockResolvedValueOnce(createMockProcess('')) // isR2Mounted
188+
.mockResolvedValueOnce(createMockProcess('')) // passwd write
189+
.mockResolvedValueOnce(createMockProcess('')) // s3fs mount
190+
.mockResolvedValueOnce(createMockProcess('s3fs on /data/moltbot type fuse.s3fs\n')); // verify
191+
195192
const env = createMockEnvWithR2();
196193

197-
// Fire two mount calls concurrently (simulates waitUntil + catch-all race)
198194
const [result1, result2] = await Promise.all([
199195
mountR2Storage(sandbox, env),
200196
mountR2Storage(sandbox, env),
201197
]);
202198

203199
expect(result1).toBe(true);
204200
expect(result2).toBe(true);
205-
// mountBucket should only have been called once despite two concurrent callers
206-
expect(mountBucketMock).toHaveBeenCalledTimes(1);
201+
// s3fs mount command should only run once
202+
const mountCalls = startProcessMock.mock.calls.filter((call: unknown[]) =>
203+
(call[0] as string).startsWith('mkdir -p'),
204+
);
205+
expect(mountCalls).toHaveLength(1);
207206
});
208207

209208
it('resets lock after failure so next attempt can retry', async () => {
210-
const { sandbox, mountBucketMock, startProcessMock } = createMockSandbox({ mounted: false });
211-
// First attempt: mount fails and post-error check also says not mounted
212-
mountBucketMock.mockRejectedValueOnce(new Error('Mount failed'));
209+
const { sandbox, startProcessMock } = createMockSandbox({ mounted: false });
210+
// First attempt: all checks fail
213211
startProcessMock
214-
.mockResolvedValueOnce(createMockProcess('')) // isR2Mounted before mount
215-
.mockResolvedValueOnce(createMockProcess('')) // deduplicateS3fsPasswd
216-
.mockResolvedValueOnce(createMockProcess('')); // isR2Mounted after error
212+
.mockResolvedValueOnce(createMockProcess('')) // isR2Mounted
213+
.mockResolvedValueOnce(createMockProcess('')) // passwd write
214+
.mockResolvedValueOnce(createMockProcess('', { exitCode: 1 })) // s3fs fails
215+
.mockResolvedValueOnce(createMockProcess('')) // verify (not mounted)
216+
.mockResolvedValueOnce(createMockProcess('')); // final check (not mounted)
217217

218218
const env = createMockEnvWithR2();
219219

220220
const result1 = await mountR2Storage(sandbox, env);
221221
expect(result1).toBe(false);
222222

223-
// Second attempt should be allowed (lock was released)
224-
mountBucketMock.mockResolvedValueOnce(undefined);
223+
// Second attempt should work (lock was released)
225224
startProcessMock
226-
.mockResolvedValueOnce(createMockProcess('')) // isR2Mounted before mount
227-
.mockResolvedValueOnce(createMockProcess('')); // deduplicateS3fsPasswd
225+
.mockResolvedValueOnce(createMockProcess('')) // isR2Mounted
226+
.mockResolvedValueOnce(createMockProcess('')) // passwd write
227+
.mockResolvedValueOnce(createMockProcess('')) // s3fs mount
228+
.mockResolvedValueOnce(createMockProcess('s3fs on /data/moltbot type fuse.s3fs\n')); // verify
228229

229230
const result2 = await mountR2Storage(sandbox, env);
230231
expect(result2).toBe(true);
231-
expect(mountBucketMock).toHaveBeenCalledTimes(2);
232232
});
233233
});
234234
});

0 commit comments

Comments
 (0)