Skip to content

Commit 8f324b3

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 78fa47b commit 8f324b3

File tree

2 files changed

+176
-122
lines changed

2 files changed

+176
-122
lines changed

src/gateway/r2.test.ts

Lines changed: 88 additions & 55 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,128 +83,154 @@ 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 });
129+
it('does not call mountBucket — uses direct s3fs instead', async () => {
130+
const { sandbox, mountBucketMock, startProcessMock } = createMockSandbox({ mounted: false });
131+
startProcessMock
132+
.mockResolvedValueOnce(createMockProcess(''))
133+
.mockResolvedValueOnce(createMockProcess(''))
134+
.mockResolvedValueOnce(createMockProcess(''))
135+
.mockResolvedValueOnce(createMockProcess('s3fs on /data/moltbot type fuse.s3fs\n'));
136+
120137
const env = createMockEnvWithR2();
121138

122139
await mountR2Storage(sandbox, env);
123140

124-
expect(console.log).toHaveBeenCalledWith(
125-
'R2 bucket mounted successfully - moltbot data will persist across sessions',
126-
);
141+
expect(mountBucketMock).not.toHaveBeenCalled();
127142
});
128143
});
129144

130145
describe('error handling', () => {
131-
it('returns false when mountBucket throws and mount check fails', async () => {
132-
const { sandbox, mountBucketMock, startProcessMock } = createMockSandbox({ mounted: false });
133-
mountBucketMock.mockRejectedValue(new Error('Mount failed'));
146+
it('returns false when s3fs mount fails and post-mount check fails', async () => {
147+
const { sandbox, startProcessMock } = createMockSandbox({ mounted: false });
134148
startProcessMock
135-
.mockResolvedValueOnce(createMockProcess(''))
136-
.mockResolvedValueOnce(createMockProcess(''));
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)
137154

138155
const env = createMockEnvWithR2();
139156

140157
const result = await mountR2Storage(sandbox, env);
141158

142159
expect(result).toBe(false);
143-
expect(console.error).toHaveBeenCalledWith('Failed to mount R2 bucket:', expect.any(Error));
160+
expect(console.error).toHaveBeenCalledWith(
161+
'Failed to mount R2 bucket: s3fs mount did not succeed',
162+
);
144163
});
145164

146-
it('returns true if mount fails but check shows it is actually mounted', async () => {
147-
const { sandbox, mountBucketMock, startProcessMock } = createMockSandbox();
165+
it('returns true if mount check passes despite errors during setup', async () => {
166+
const { sandbox, startProcessMock } = createMockSandbox();
148167
startProcessMock
149-
.mockResolvedValueOnce(createMockProcess('')) // isR2Mounted before mount
150-
.mockResolvedValueOnce(createMockProcess('')) // clearS3fsPasswdFiles
151-
.mockResolvedValueOnce(createMockProcess('s3fs on /data/moltbot type fuse.s3fs\n')); // isR2Mounted after error
152-
153-
mountBucketMock.mockRejectedValue(new Error('Transient error'));
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
154171

155172
const env = createMockEnvWithR2();
156173

157174
const result = await mountR2Storage(sandbox, env);
158175

159176
expect(result).toBe(true);
160-
expect(console.log).toHaveBeenCalledWith('R2 bucket is mounted despite error');
177+
expect(console.log).toHaveBeenCalledWith(
178+
'R2 bucket is mounted despite errors during setup',
179+
);
161180
});
162181
});
163182

164183
describe('concurrent mount protection', () => {
165-
it('only calls mountBucket once when invoked concurrently', async () => {
166-
const { sandbox, mountBucketMock } = createMockSandbox({ mounted: false });
184+
it('only runs mount once when invoked concurrently', async () => {
185+
const { sandbox, startProcessMock } = createMockSandbox({ mounted: false });
186+
// Default mock returns empty (not mounted), override specific calls
187+
startProcessMock
188+
.mockResolvedValueOnce(createMockProcess('')) // isR2Mounted
189+
.mockResolvedValueOnce(createMockProcess('')) // passwd write
190+
.mockResolvedValueOnce(createMockProcess('')) // s3fs mount
191+
.mockResolvedValueOnce(createMockProcess('s3fs on /data/moltbot type fuse.s3fs\n')); // verify
192+
167193
const env = createMockEnvWithR2();
168194

169-
// Fire two mount calls concurrently (simulates waitUntil + catch-all race)
170195
const [result1, result2] = await Promise.all([
171196
mountR2Storage(sandbox, env),
172197
mountR2Storage(sandbox, env),
173198
]);
174199

175200
expect(result1).toBe(true);
176201
expect(result2).toBe(true);
177-
// mountBucket should only have been called once despite two concurrent callers
178-
expect(mountBucketMock).toHaveBeenCalledTimes(1);
202+
// passwd write + s3fs mount should only run once (plus isR2Mounted checks)
203+
// No duplicate mount attempts
204+
const mountCalls = startProcessMock.mock.calls.filter((call: unknown[]) =>
205+
(call[0] as string).startsWith('mkdir -p'),
206+
);
207+
expect(mountCalls).toHaveLength(1);
179208
});
180209

181210
it('resets lock after failure so next attempt can retry', async () => {
182-
const { sandbox, mountBucketMock, startProcessMock } = createMockSandbox({ mounted: false });
183-
// First attempt: mount fails and post-error check also says not mounted
184-
mountBucketMock.mockRejectedValueOnce(new Error('Mount failed'));
211+
const { sandbox, startProcessMock } = createMockSandbox({ mounted: false });
212+
// First attempt: all checks fail
185213
startProcessMock
186-
.mockResolvedValueOnce(createMockProcess('')) // isR2Mounted before mount
187-
.mockResolvedValueOnce(createMockProcess('')); // isR2Mounted after error
214+
.mockResolvedValueOnce(createMockProcess('')) // isR2Mounted
215+
.mockResolvedValueOnce(createMockProcess('')) // passwd write
216+
.mockResolvedValueOnce(createMockProcess('', { exitCode: 1 })) // s3fs fails
217+
.mockResolvedValueOnce(createMockProcess('')) // verify (not mounted)
218+
.mockResolvedValueOnce(createMockProcess('')); // final check (not mounted)
188219

189220
const env = createMockEnvWithR2();
190221

191222
const result1 = await mountR2Storage(sandbox, env);
192223
expect(result1).toBe(false);
193224

194-
// Second attempt should be allowed (lock was released)
195-
mountBucketMock.mockResolvedValueOnce(undefined);
196-
startProcessMock.mockResolvedValueOnce(createMockProcess('')); // isR2Mounted before mount
225+
// Second attempt should work (lock was released)
226+
startProcessMock
227+
.mockResolvedValueOnce(createMockProcess('')) // isR2Mounted
228+
.mockResolvedValueOnce(createMockProcess('')) // passwd write
229+
.mockResolvedValueOnce(createMockProcess('')) // s3fs mount
230+
.mockResolvedValueOnce(createMockProcess('s3fs on /data/moltbot type fuse.s3fs\n')); // verify
197231

198232
const result2 = await mountR2Storage(sandbox, env);
199233
expect(result2).toBe(true);
200-
expect(mountBucketMock).toHaveBeenCalledTimes(2);
201234
});
202235
});
203236
});

0 commit comments

Comments
 (0)