Skip to content

rest: treat BoxID as opaque server-issued identifier#498

Merged
DorianZheng merged 1 commit into
mainfrom
rest-sdk-wait-coordination
May 10, 2026
Merged

rest: treat BoxID as opaque server-issued identifier#498
DorianZheng merged 1 commit into
mainfrom
rest-sdk-wait-coordination

Conversation

@DorianZheng

@DorianZheng DorianZheng commented May 10, 2026

Copy link
Copy Markdown
Member

Summary

Replaces the format allow-list (12-char Base62 / 26-char ULID / 36-char UUID) in BoxID::is_valid with 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.ai integration failure where the server returned UUIDs the SDK couldn't parse, and the SDK silently called BoxIDMint::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:

  • E2B Sandbox SDK (packages/python-sdk/e2b/sandbox/main.py) — Sandbox.connect(sandbox_id) stores the value verbatim, no validation.
  • Modal 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 — replace FULL_LENGTH / LEGACY_LENGTH / UUID_LENGTH constants with a single MAX_LENGTH = 128. Drop is_uuid_format. Permissive is_valid. Length-aware short(). Add BoxIDMint::MINT_LENGTH.
  • src/boxlite/src/rest/types.rsto_box_info() returns BoxliteResult<BoxInfo>, propagating parse failures.
  • src/boxlite/src/rest/{litebox,runtime}.rs — call sites updated with ?.
  • src/boxlite/tests/lifecycle.rs — assertion uses BoxIDMint::MINT_LENGTH.
  • openapi/rest-sandbox-open-api.yamlbox_id documented 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 in rest::tests::*. Clippy -D warnings clean. cargo fmt --check clean.

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 passing
  • cargo test -p boxlite --lib --features rest rest — 31 passing
  • cargo clippy -p boxlite --features rest --tests -- -D warnings
  • cargo fmt --check
  • Smoke against dev.boxlite.ai (UUID server) and in-tree boxlite serve (Base62 server) once disk space allows

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.
@DorianZheng DorianZheng force-pushed the rest-sdk-wait-coordination branch from 3301a32 to eb726b8 Compare May 10, 2026 03:20
@DorianZheng DorianZheng changed the title rest: harden Execution::wait coordination + opt-in sse_silence_max rest: treat BoxID as opaque server-issued identifier May 10, 2026
@DorianZheng DorianZheng merged commit e2195ba into main May 10, 2026
34 checks passed
@DorianZheng DorianZheng deleted the rest-sdk-wait-coordination branch May 10, 2026 03:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant