Skip to content

id(world): PR #927's oversized-layout fallback never checks its own result against presentation_bytes (#811) #942

Closed

Description

@dezren39

Finding

Open PR #927 (Jules, for #811) fixes the oversized-layout() poison path by falling back to program.view() cropped to the requested size — but the fallback result is never checked against presentation_bytes, the very limit the branch exists to enforce. A world whose default view is also large (or not a screen) gets an over-limit presentation delivered as success.

Found by the pr-diff-review job reviewing PR #927, 2026-10-25. PR #927 is still open, so this can be folded into it or fixed immediately after it lands; the code quoted is from the PR head (4d33369c), pkgs/id/src/world/actor.rs, WorldCommand::ViewAt handler.

Evidence

  • The layout branch triggers only when text.len() > self.core.limits.presentation_bytes (default 8192, pkgs/id/src/world/mod.rs:91), increments view_failures, and falls back.
  • Fallback, non-screen case: Ok(Some(fallback_text)) is returned for any program.view() text that parse_surface doesn't recognize as a screen — with no length check at all. A guest whose view() returns a >8192-byte non-screen presentation sails through.
  • Fallback, screen case: the surface is cropped with surface.fit(width, height) and re-serialized, but the serialized result is never compared to presentation_bytes. Screens may be up to MAX_SCREEN_WIDTH×MAX_SCREEN_HEIGHT = 120×50 = 6000 cells (pkgs/id/src/world_presentation.rs:15-19), each cell a JSON object — a dense cropped screen is far over 8192 bytes. On serialization failure the code does unwrap_or(fallback_text), returning the original unchecked text.
  • The PR's own test (an_oversized_layout_degrades_to_a_view_fallback_and_recovers, pkgs/id/src/world/tests.rs) covers only a tiny 10×10 fallback view, so none of these cases is exercised.
  • Related bound disagreement in the same PR: the ClientFrame::Resize handler in pkgs/id/src/world_session/mod.rs first validates width/height against 500×200 (error text: "viewport dimensions must be width 1..=500 and height 1..=200"), then silently clamps the stored viewport to MAX_SCREEN_WIDTH/MAX_SCREEN_HEIGHT = 120/50. A client that resizes to, say, 300×100 is told that size is legal but is actually served a 120×50 view. Which bound is meant to govern — and whether silently shrinking a legal resize is the intended id phase-9: a joiner's resize (up to 500x200) can permanently poison a world via the guest layout() call exceeding presentation_bytes #811 mitigation — deserves an explicit verdict in the same triage.

Why it matters

#811 is about a guest presentation exceeding the byte limit breaking the world. A fallback that can itself exceed the limit reintroduces the over-limit delivery on the exact path meant to prevent it — consumers sized for presentation_bytes (SSH/web surfaces) receive unbounded presentations, and the failure mode moves somewhere harder to diagnose.

Acceptance criteria

  • The ViewAt fallback result (cropped screen serialization, non-screen text, and the serialization-failure path) is checked against presentation_bytes before being returned; an over-limit fallback degrades to an error (counted like other view failures) instead of an over-limit success.
  • A regression test with a fallback view that is itself over the limit (dense screen and/or non-screen text) proves the world still does not deliver it and does not poison.
  • The Resize bound disagreement (validation allows 500×200, the stored viewport is silently clamped to 120×50) is resolved — one governing bound, and the error text matches it — or documented as intentional.

— filed by the pr-diff-review job (PR #927, relating to #811); triage via the repo-health queue pilot.

Activity

  1. added
    kind/bugSomething isn't working (prefixed form of the classic bug label)
    task/triageQueue packet kind: verify, reproduce, and route this item
    priority/normalOrdinary priority (the default)
    queue/queuedHas a queued task in the Supabase agent queue
    app/idPackage/app: id (pkgs/ or apps/ in this repo)
    on Oct 26, 2026
  2. dezren39 commented on Oct 26, 2026

    @dezren39
    MemberAuthor

    🤖 Agent note · filed

  3. added
    work/assignedQueue worker lifecycle state
    queue/claimedQueue worker lifecycle state
    and removed
    queue/queuedHas a queued task in the Supabase agent queue
    on Oct 31, 2026
  4. dezren39 commented on Oct 31, 2026

    @dezren39
    MemberAuthor

    🤖 Agent note · picked up

    • Agent: openchamber-scheduled-lane
    • Session / ID: queue task 1089
    • Task: Revalidate triage against current GitHub state
    • Next: Check source PR and issue state before proceeding.
  5. 21 remaining items

  6. added
    queue/claimedQueue worker lifecycle state
    queue/parkedQueue task is parked (waiting on CI, time, or an event)
    status/parkedDeliberately paused with a recorded resume condition
    and removed
    queue/claimedQueue worker lifecycle state
    on Nov 3, 2026
  7. dezren39 commented on Nov 3, 2026

    @dezren39
    MemberAuthor

    🤖 Agent note · parked

  8. dezren39 commented on Nov 3, 2026

    @dezren39
    MemberAuthor

    🤖 Agent note · parked

  9. dezren39 commented on Nov 3, 2026

    @dezren39
    MemberAuthor

    Done (task 8051, lane code-e): PR #5061 squash-merged as 75f5f1472060cfc777b7fb2e72b1e0344d5e0875 (2026-11-03T18:56:26Z); this issue auto-closed. The ViewAt fallback is now bounded by presentation_bytes on all three paths (cropped serialization, non-screen text, serialization failure), with regression tests in pkgs/id/src/world/tests.rs and the Resize clamp verdict documented in world_session/mod.rs. Evidence: line-limit check pass on the PR; laptop cargo test -p id --lib oversized 14 passed and world::tests 164 passed; Codex disposition recorded on PR #5061 (usage-limit failure, zero inline findings).

  10. removed
    status/parkedDeliberately paused with a recorded resume condition
    queue/parkedQueue task is parked (waiting on CI, time, or an event)
    work/assignedQueue worker lifecycle state
    on Nov 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

app/idPackage/app: id (pkgs/ or apps/ in this repo)effort/sSmall: expected under 2 hours of workkind/bugSomething isn't working (prefixed form of the classic bug label)mode/onlineFully doable remotely: design, config, code, cloudpriority/normalOrdinary priority (the default)source/pr-diff-reviewFiled by the pr-diff-review jobtask/triageQueue packet kind: verify, reproduce, and route this item

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions