containerName in packages/core/src/sandbox/docker/container.ts appends a 32-bit FNV-1a digest to a sanitized sandbox id, and the two backends added in #12 copied that construction.
Both copies were changed to SHA-256 truncated to 64 bits during the review of #12 (local/root.ts, microsandbox/sandbox.ts). The Docker original was left alone because it is pre-existing on main and outside that PR's diff, and #12 is held to surgical changes.
Why it matters. The sanitizer is lossy by design — .. and a/b are things a caller may legitimately name a sandbox and neither may survive into a name — so the digest is the only thing separating two ids that sanitize alike. A collision means two sandbox ids naming one container, and one id's destroy() is then docker rm --force on the other's machine. At 32 bits that reaches even odds after roughly 77,000 ids.
The reason given for FNV — that a real hash would need an async web-crypto call — does not hold: node:crypto's createHash is synchronous. 64 bits of SHA-256 puts the same even-odds point past five billion ids.
The change, matching what local/root.ts now does:
import { createHash } from 'node:crypto'
function shortDigest(value: string): string {
return createHash('sha256').update(value).digest('hex').slice(0, 16)
}
Note this renames every container an existing deployment already has, so containers created before the change are no longer found by their sandbox id. That is acceptable for a backend with no released consumers, and is the reason to do it now rather than later.
Raised by cubic-dev-ai on #12 (#12 (comment)) and promised as a follow-up in the reply there.
containerNameinpackages/core/src/sandbox/docker/container.tsappends a 32-bit FNV-1a digest to a sanitized sandbox id, and the two backends added in #12 copied that construction.Both copies were changed to SHA-256 truncated to 64 bits during the review of #12 (
local/root.ts,microsandbox/sandbox.ts). The Docker original was left alone because it is pre-existing onmainand outside that PR's diff, and #12 is held to surgical changes.Why it matters. The sanitizer is lossy by design —
..anda/bare things a caller may legitimately name a sandbox and neither may survive into a name — so the digest is the only thing separating two ids that sanitize alike. A collision means two sandbox ids naming one container, and one id'sdestroy()is thendocker rm --forceon the other's machine. At 32 bits that reaches even odds after roughly 77,000 ids.The reason given for FNV — that a real hash would need an async web-crypto call — does not hold:
node:crypto'screateHashis synchronous. 64 bits of SHA-256 puts the same even-odds point past five billion ids.The change, matching what
local/root.tsnow does:Note this renames every container an existing deployment already has, so containers created before the change are no longer found by their sandbox id. That is acceptable for a backend with no released consumers, and is the reason to do it now rather than later.
Raised by cubic-dev-ai on #12 (#12 (comment)) and promised as a follow-up in the reply there.