Skip to content

fix(cua-driver): carry full 32-bit snapshot generation in element tokens - #2608

Merged
f-trycua merged 3 commits into
trycua:mainfrom
hqhq1025:codex/element-token-generation-hardening
Jul 28, 2026
Merged

f-trycua merged 3 commits into
trycua:mainfrom
hqhq1025:codex/element-token-generation-hardening

Conversation

@hqhq1025

@hqhq1025 hqhq1025 commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Summary

element_token truncated a monotonic 32-bit snapshot generation to 16 bits. The truncated id repeats every 65,536 process-global snapshots. In a long-lived daemon, a repeated id can still exist in another runtime and pid lane, allowing a stale token to resolve to the wrong snapshot instead of failing.

Change

  • Keep the complete 32-bit snapshot generation.
  • Render the full value as eight lowercase hexadecimal digits.
  • Correct the module contract and collision rationale.

Example:

s00010001:3

Tokens remain opaque, process-local, and snapshot-scoped, so widening the prefix does not change a persisted client contract.

Validation

  • cargo test -p cua-driver-core element_token --locked: 20 passed
  • Full Rust and generated-contract CI are required before merge.

This is split from #2210 so the hardening change can be reviewed independently.

`register_snapshot` masked the minted id with `& 0xffff` and `format_token`
masked it again before rendering a 4-hex-char prefix, so the effective
generation space was 16 bits.

With `LRU_CAP_PER_PID = 8` still-resolvable entries per lane, each mint has
an 8/65536 chance of colliding with a live entry. A session issuing 10k
`get_window_state` calls therefore expects ~1.2 collisions. A collision does
not error: it resolves the token to a different snapshot, which reaches the
caller as a silent misclick — the exact failure this module's header calls
out ("if cua-driver ever changes its internal indexing the silent failure
mode is a misclick").

The prefix widens to 8 hex chars, so a token now reads `s0a1b2c3d:12` —
11-15 characters, still inside the 8-16 character budget the Surface 6 plan
called out, and still greppable in logs without a side table. The registry
continues to key on `(pid, snapshot_id)`, so a same-bits collision across
pids never aliases.

Tokens are documented as opaque and are per-snapshot handles that do not
survive a daemon restart, so widening the prefix does not break a
persisted format.

Verified:
- `cargo test -p cua-driver-core`: 415 passed, 0 failed
- `cargo test -p platform-macos`: 183 passed, 0 failed
- `cua-contract-gen all --check`: generated manifest already up to date
@hqhq1025
hqhq1025 requested a review from f-trycua as a code owner July 27, 2026 14:38
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@hqhq1025

Copy link
Copy Markdown
Contributor Author

@f-trycua — following up on #2210, which had grown to 19 files / +1274 −766. That was too large to review as one change, so I closed it down to a tracking draft and split out the parts that stand on their own. Three PRs, each single-file and independent of the other two:

  • fix(cua-driver): carry full 32-bit snapshot generation in element tokens #2608element_token's snapshot generation is truncated to 16 bits in two places, so with LRU_CAP_PER_PID = 8 each mint has an 8/65536 chance of colliding with a live entry. A collision resolves the token to a different snapshot, which reaches the caller as a silent misclick rather than an error. Carries the full 32 bits; the prefix widens to 8 hex chars, still inside the 8–16 character budget the module documents. cua-driver-core 415 passed.

  • fix(cua-driver): verify set_value writes with an AXValue read-back #2621set_value reports success on the AXUIElementSetAttributeValue return code alone. type_text's own description already documents that web content "accepts a write and echoes it back through AXValue while the renderer/DOM never observes it"; set_value had no equivalent guard. Adds a read-back with verified / changed in structuredContent. platform-macos 189 passed.

  • feat(cua-driver): expose element AX actions in structured elements #2622 — the AX walk already collects each element's exposed actions and renders them into tree_markdown, but the structured elements array never carried them, so a caller has to guess an action name for perform_action. Same gap that was already closed for value, with the same reasoning as the comment above that field. platform-macos 184 passed.

Three things from #2210 I am deliberately not carrying forward, with reasons in that PR's body: the UUID token format (conflicts with the documented 8–16 char / greppable goals), allow_pixel_fallback(via_token) (couples addressing to dispatch strategy, and the requirement behind it belongs in the downstream client), and node_identity (it stored element_ptr as u64, which cannot survive a re-snapshot and so did not deliver what the title claimed).

Two items I originally listed for extraction turned out not to be upstream defects at all — token_for is a pure formatter that cannot panic, and the element-action TOCTOU applies to the token path #2210 proposed rather than to anything in main. I'd rather say that than open PRs claiming to fix bugs that aren't there.

No rush on these; happy to split further or adjust any of them.

@hqhq1025

Copy link
Copy Markdown
Contributor Author

Separate ask, unrelated to the change itself: none of the pull_request workflows have run on this branch. Every run sits at conclusion: action_required with zero jobs, so GitHub never creates a check run and gh pr checks reports "no checks reported". Only contributor-attribution shows up, because it triggers on pull_request_target.

Could you approve workflows for this PR (and #2621 / #2622) when you get a chance? Same request as on #2166.

Checks I ran locally in the meantime:

cargo test -p cua-driver-core                                    415 passed, 0 failed
cargo test -p platform-macos                                     183 passed, 0 failed
cargo run -p cua-driver-contract --bin cua-contract-gen -- all --check     up to date
cargo run -p cua-driver-bindgen --bin cua-driver-abi-header -- --check     up to date
cargo fmt -p cua-driver-core -- --check                          clean

Worth noting for the macOS-heavy parts: the repo has ci-rust-linux and ci-rust-windows but no macOS unit/E2E lane, so a green CI would not evidence macOS behaviour either way. #2608 is the one that benefits most from CI here, since it is pure cua-driver-core and exercised on both existing lanes.

Correct the module contract and rationale after widening snapshot generations from 16 to 32 bits. The process-global counter is monotonic, so the old collision probability model did not apply.

Co-authored-by: Francesco Bonacci <195596869+f-trycua@users.noreply.github.com>
@f-trycua
f-trycua requested a review from injaneity as a code owner July 28, 2026 11:10

@f-trycua f-trycua left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed after correcting the module contract and monotonic-counter rationale. Full-width tokens remain opaque and round-trip through the existing registry.

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.

2 participants