Skip to content

fix(node): correct scoped permissions wire codec#3642

Open
ryankert01 wants to merge 1 commit into
apache:masterfrom
ryankert01:codex/fix-node-permission-codec
Open

fix(node): correct scoped permissions wire codec#3642
ryankert01 wants to merge 1 commit into
apache:masterfrom
ryankert01:codex/fix-node-permission-codec

Conversation

@ryankert01

@ryankert01 ryankert01 commented Jul 10, 2026

Copy link
Copy Markdown
Member

Which issue does this PR address?

Closes #3643

Rationale

The Node SDK could not create or update users with stream- or topic-scoped permissions because its wire codec did not match the canonical Rust permission format.

What changed?

Nested permission serialization omitted the required continuation marker after each topic and stream, so the server rejected any permission payload with a non-empty streams array. The decoder also read continuation markers without advancing past them, which dropped or misaligned multiple entries.

Serialization and deserialization now mirror the Rust codec, and createUser no longer appends a length and payload when permissions are absent. Canonical byte-vector unit tests cover multiple streams and topics, while the user E2E test verifies scoped permissions through create, update, and get.

Local Execution

  • Passed
  • Pre-commit hooks did not run in the isolated worktree. The following checks were run manually:
    • npm run lint
    • npm run build
    • npm run test:unit (75 passed)
    • node --import @swc-node/register/esm-register --test src/e2e/tcp.user.e2e.ts (9 passed against a local server)
    • cargo test -p iggy_binary_protocol primitives::permissions::tests::roundtrip_with_streams_and_topics -- --exact

AI Usage

  1. OpenAI Codex.
  2. Investigation, implementation, regression tests, review, and PR description.
  3. The generated changes were verified with exact canonical wire vectors, the full Node unit suite, ESLint, the TypeScript build, a live user E2E test, and the canonical Rust permission test.
  4. Yes.

@codecov

codecov Bot commented Jul 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.05%. Comparing base (6f1d548) to head (2ea89eb).

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #3642      +/-   ##
============================================
+ Coverage     73.99%   74.05%   +0.06%     
  Complexity      937      937              
============================================
  Files          1301     1301              
  Lines        147393   147400       +7     
  Branches     122945   122954       +9     
============================================
+ Hits         109063   109163     +100     
+ Misses        34858    34763      -95     
- Partials       3472     3474       +2     
Components Coverage Δ
Rust Core 74.29% <ø> (ø)
Java SDK 62.44% <ø> (ø)
C# SDK 72.18% <ø> (ø)
Python SDK 92.17% <ø> (ø)
PHP SDK 84.29% <ø> (ø)
Node SDK 92.36% <100.00%> (+1.09%) ⬆️
Go SDK 42.87% <ø> (ø)
Files with missing lines Coverage Δ
foreign/node/src/wire/user/create-user.command.ts 100.00% <100.00%> (ø)
foreign/node/src/wire/user/permissions.utils.ts 100.00% <100.00%> (+26.72%) ⬆️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ryankert01
ryankert01 marked this pull request as ready for review July 10, 2026 06:05
@github-actions github-actions Bot added the S-waiting-on-review PR is waiting on a reviewer label Jul 10, 2026
@ryankert01
ryankert01 marked this pull request as draft July 10, 2026 06:13
@github-actions github-actions Bot removed the S-waiting-on-review PR is waiting on a reviewer label Jul 10, 2026
@ryankert01
ryankert01 marked this pull request as ready for review July 13, 2026 20:51
@github-actions github-actions Bot added the S-waiting-on-review PR is waiting on a reviewer label Jul 13, 2026
@ryankert01
ryankert01 force-pushed the codex/fix-node-permission-codec branch from 0c00e16 to 7c162b8 Compare July 13, 2026 20:52
Scoped permissions omitted list continuation markers, causing the server to reject any user permissions containing streams. Encode and consume the canonical markers, correct absent-permission framing, and cover the wire shape and end-to-end round trip.
@ryankert01
ryankert01 force-pushed the codex/fix-node-permission-codec branch from 7c162b8 to 2ea89eb Compare July 17, 2026 12:09

@T1B0 T1B0 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

looks good ! nice new test cases. LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review PR is waiting on a reviewer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Node SDK rejects stream- and topic-scoped user permissions

2 participants