Summary
POST /upload and POST /upload/batch write the file's sessionKey to Redis (session:<session_id>) without awaiting the write before forwarding the file to file-server. file-server reads that key synchronously on receipt to build its own authorization key (upload:<sessionKey><session_id><fileId>). If the network round-trip to file-server completes before the SET has propagated, file-server reads back null and registers the upload under a sessionKey-less key that no legitimate request can ever match.
The failure is silent at upload time (/upload returns 200) and only surfaces later, as a 403 Unauthorized file reference / upload_missing when the file is referenced from a sandboxed execution — by which point the original request context is gone and the error is hard to correlate back to the upload step.
Root cause
service/src/service/router.ts:
/upload (~line 407, pre-fix):
connection.set(`session:${session_id}`, sessionKey, 'EX', env.SESSION_CACHE_TTL);
// not awaited — axios.put to file-server can win the race
axios.put(`${env.FILE_SERVER_URL}/sessions/${session_id}/objects/${fileId}`, file, ...)- /upload/batch (~line 612, pre-fix): same issue, plus a sessionKeySet boolean that only guards against re-issuing the SET — it doesn't make concurrent files in the same batch wait for it to actually land.
file-server.ts (uploadFile):
const sessionKey = await redisClient.get(`session:${session_id}`); // may read null
await redisClient.set(`upload:${sessionKey}${session_id}${fileId}`, 'true', 'EX', TTL);No validation that sessionKey is non-null before using it to build the registration key — a null sessionKey is silently accepted and baked into the key.
Reproduction
Deterministic repro (see attached patch/description): delay the SET in router.ts by ~300ms so the PUT to file-server always wins the race, then upload a single file via /upload. Inspecting Redis afterwards shows:
upload:null<session_id><fileId> <- registered (wrong)
upload:<sessionKey><session_id><fileId> <- missing
In production, we observed this without any artificial delay, on a conversation that uploaded 11 files in a single request: 3-4 of the 11
files ended up with a null-prefixed key, while the rest were fine — consistent with each file being an independent race and the failure probability compounding with file count. A scan of our production Redis turned up 300+ live upload:null* keys at a single point in time, so this is not a rare occurrence at any real upload volume.
Suggested fix
Await the Redis write before issuing the PUT to file-server, e.g.:
connection.set(`session:${session_id}`, sessionKey, 'EX', env.SESSION_CACHE_TTL)
.then(() => axios.put(`${env.FILE_SERVER_URL}/sessions/${session_id}/objects/${fileId}`, file, ...))
.then(response => resolve(response.data))
.catch(reject);For /upload/batch, replace the sessionKeySet boolean with a shared promise so every file in the batch (not just the one that issues the SET) waits on the completed write:
if (!sessionKeySetPromise) {
sessionKeySetPromise = connection.set(`session:${session_id}`, sessionKey, 'EX', env.SESSION_CACHE_TTL);
}
sessionKeySetPromise.then(() => axios.put(...)).then(...).catch(...);As defense in depth, file-server.uploadFile could also refuse to register an upload key when the resolved sessionKey is null/falsy, rather than silently writing upload:null... — that would turn this class of bug into a loud, immediate error instead of a delayed, hard to trace 403 much later in a different request.
Impact
Any upload — especially multi-file uploads via /upload/batch or several files attached to one /upload call — can silently produce files that are permanently unusable in sandboxed code execution for the session's TTL (default 24h), with no error surfaced to the end user until they try to actually use the file in code, at which point the failure looks unrelated to the upload.
Summary
POST /uploadandPOST /upload/batchwrite the file'ssessionKeyto Redis (session:<session_id>) without awaiting the write before forwarding the file tofile-server.file-serverreads that key synchronously on receipt to build its own authorization key (upload:<sessionKey><session_id><fileId>). If the network round-trip tofile-servercompletes before theSEThas propagated,file-serverreads backnulland registers the upload under asessionKey-less key that no legitimate request can ever match.The failure is silent at upload time (
/uploadreturns 200) and only surfaces later, as a403 Unauthorized file reference/upload_missingwhen the file is referenced from a sandboxed execution — by which point the original request context is gone and the error is hard to correlate back to the upload step.Root cause
service/src/service/router.ts:/upload(~line 407, pre-fix):file-server.ts (uploadFile):
No validation that sessionKey is non-null before using it to build the registration key — a null sessionKey is silently accepted and baked into the key.
Reproduction
Deterministic repro (see attached patch/description): delay the SET in router.ts by ~300ms so the PUT to file-server always wins the race, then upload a single file via /upload. Inspecting Redis afterwards shows:
In production, we observed this without any artificial delay, on a conversation that uploaded 11 files in a single request: 3-4 of the 11
files ended up with a null-prefixed key, while the rest were fine — consistent with each file being an independent race and the failure probability compounding with file count. A scan of our production Redis turned up 300+ live upload:null* keys at a single point in time, so this is not a rare occurrence at any real upload volume.
Suggested fix
Await the Redis write before issuing the PUT to file-server, e.g.:
For /upload/batch, replace the sessionKeySet boolean with a shared promise so every file in the batch (not just the one that issues the SET) waits on the completed write:
As defense in depth, file-server.uploadFile could also refuse to register an upload key when the resolved sessionKey is null/falsy, rather than silently writing upload:null... — that would turn this class of bug into a loud, immediate error instead of a delayed, hard to trace 403 much later in a different request.
Impact
Any upload — especially multi-file uploads via /upload/batch or several files attached to one /upload call — can silently produce files that are permanently unusable in sandboxed code execution for the session's TTL (default 24h), with no error surfaced to the end user until they try to actually use the file in code, at which point the failure looks unrelated to the upload.