rest: treat BoxID as opaque server-issued identifier#498
Merged
Conversation
Replaces the format allow-list (12-char Base62 + 26-char ULID) in
BoxID::is_valid with a single permissive rule:
non-empty
length <= MAX_LENGTH (128)
bytes restricted to [A-Za-z0-9_-]
Rationale: comparable sandbox / container / cloud SDKs treat resource
identifiers as opaque server-issued strings. E2B's
Sandbox.connect(sandbox_id) stores the value verbatim with no
client-side validation; Modal Sandbox.from_id, Docker container
ids/names, Kubernetes UIDs, Stripe object ids and AWS resource ids
all follow the same pattern — the server is the authority. The prior
allow-list was the root cause of the silent-mint bug
(`feedback_silent_id_fallback`) where the SDK would silently call
BoxIDMint::mint() when a server-issued id failed BoxID::parse(),
producing a fresh local id that subsequent box-scoped REST calls
404'd against. dev.boxlite.ai returns UUIDs; the SDK rejected them
and silently invented IDs.
Also propagates BoxResponse::to_box_info() parse errors instead of
the silent mint, as belt-and-suspenders against the few corrupting
shapes the new validator still rejects (empty, oversize, path-
traversal, whitespace, URL-unsafe punctuation).
Side-effects:
- Drop FULL_LENGTH / LEGACY_LENGTH / UUID_LENGTH constants and the
is_uuid_format helper.
- BoxIDMint::mint() now uses BoxIDMint::MINT_LENGTH (= 12); local
minting policy stays unchanged but is decoupled from validation.
- BoxID::short() is length-aware (truncates by min(len, 8)) so a
short server id cannot panic the Display/Debug paths.
- Loosen openapi/rest-sandbox-open-api.yaml `box_id` to length
1..=128, with documentation that clients must treat the value as
opaque. Concrete formats (ULID, Base62, UUID) are kept as
non-normative examples.
- Update src/boxlite/tests/lifecycle.rs assertion to use
BoxIDMint::MINT_LENGTH.
- Update REST call sites that consume to_box_info() to propagate via
?: rest/litebox.rs (start, stop, clone), rest/runtime.rs (5 sites
including the list_info iterator collect).
Tests: id.rs gains positive cases (server-prefix style like
`sb_abc123`, mixed underscore/hyphen, boundary lengths) and negative
cases (empty, oversize, path-traversal, whitespace, URL-unsafe
punctuation); the proptest fixtures are rewritten to exercise the
permissive rule across the full URL-safe character class. The
rest::types unparseable test asserts against actually-corrupting
inputs (empty, slash, whitespace, dot) rather than `not-an-id`,
which is now legitimately valid.
Note on scope: the SSE reader / status poller coordination work
that previously coexisted on this branch has been moved to a
separate branch (`rest-sdk-execution-wait-coordination`) for a
follow-up PR, so this PR is reviewable as a focused identifier
change.
3301a32 to
eb726b8
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replaces the format allow-list (12-char Base62 / 26-char ULID / 36-char UUID) in
BoxID::is_validwith a permissive opaque-identifier rule: non-empty, length ≤ 128, bytes restricted to[A-Za-z0-9_-]. Server is the authority on identifier format; the client just stores and replays whatever it gets.Motivated by the
dev.boxlite.aiintegration failure where the server returned UUIDs the SDK couldn't parse, and the SDK silently calledBoxIDMint::mint()to invent a fresh local id (feedback_silent_id_fallback). Every box-scoped REST call afterwards 404'd. Adding a UUID arm fixes that one server; the next server format would break the SDK again. The right fix is to stop allow-listing.This matches how comparable sandbox / container / cloud SDKs treat resource IDs:
Sandbox.connect(sandbox_id)stores the value verbatim, no validation.Sandbox.from_id, Docker container ids/names, Kubernetes UIDs, Stripe object ids (pi_...,cus_...), AWS resource ids (i-...,vpc-...) — same pattern.The remaining client-side checks are anti-corruption only (non-empty, length cap, URL-/path-safe charset) so the id can be embedded in REST paths and on-disk dirs without surprises.
Also propagates
BoxResponse::to_box_info()parse errors instead of the silent mint, as belt-and-suspenders against the few corrupting shapes the new validator still rejects (empty, oversize, path-traversal, whitespace, URL-unsafe punctuation).Changes
src/boxlite/src/runtime/id.rs— replaceFULL_LENGTH/LEGACY_LENGTH/UUID_LENGTHconstants with a singleMAX_LENGTH = 128. Dropis_uuid_format. Permissiveis_valid. Length-awareshort(). AddBoxIDMint::MINT_LENGTH.src/boxlite/src/rest/types.rs—to_box_info()returnsBoxliteResult<BoxInfo>, propagating parse failures.src/boxlite/src/rest/{litebox,runtime}.rs— call sites updated with?.src/boxlite/tests/lifecycle.rs— assertion usesBoxIDMint::MINT_LENGTH.openapi/rest-sandbox-open-api.yaml—box_iddocumented as opaque,minLength: 1, maxLength: 128. Concrete formats kept as non-normative examples.Tests: 42 unit tests in
id::tests::*(10 new positive/negative cases + rewritten proptests), 31 inrest::tests::*. Clippy-D warningsclean.cargo fmt --checkclean.Out of scope
The SSE reader / status-poller coordination work that was previously bundled here has been moved to a separate branch (
rest-sdk-execution-wait-coordination) for a follow-up PR. That work touches different code, has different review concerns, and benefits from being reviewable in isolation.Test plan
cargo test -p boxlite --lib --features rest id— 42 passingcargo test -p boxlite --lib --features rest rest— 31 passingcargo clippy -p boxlite --features rest --tests -- -D warningscargo fmt --checkdev.boxlite.ai(UUID server) and in-treeboxlite serve(Base62 server) once disk space allows