v0.7.6#17
Merged
Merged
Conversation
bwp91
commented
May 24, 2026
- fix(handshake): correct Windows platform detection regex
- fix(handshake): surface server's auth rejection reason
- fix(introspect): clone object before mutating during sub-path recursion
- fix(introspect): include first sub-node in introspection results
- fix(stdifaces): aggregate exported children using loop variable
- fix(stdifaces): write the value supplied by the caller in Properties.Set
- fix(bus): typo in emit-undefined-signal error message
- fix(bus): forward opts when bus() is called without new
- fix(bus): drop double serial increment after signal emit
- fix(bus): expose error body string as err.message
- fix(bus): reply with UnknownInterface when interface missing on expor…
- fix(bus): handle void method returns and missing result signatures
- fix(bus): treat returned Error as a D-Bus error reply
- fix(server-handshake): drop debug console.log spam
- fix(put): correct floatle encoding and remove debug logs
- fix(index): include arg0 in unixexec arguments
- fix(message): honor message endianness on receive
- fix(dbus2js): align xml2js config with introspect.js
- fix(address-x11): lazy-load x11 with clear error and document as opti…
- fix(bus): only wrap obj.emit once per object across re-exports
- fix(portforward): move top-level side-effect script out of lib/
- chore(security): create exported-object maps with null prototype
- chore(examples): load jQuery over HTTPS in dbusproxy demo
- chore(ci): set least-privilege permissions on Node Test workflow
- chore(ci): bump release workflow action versions
- chore: dependency updates
The regex /$win/ uses end-of-string anchor before "win", which can never match. Windows users always fell through to HOME, breaking DBUS_COOKIE_SHA1 cookie file lookup. Use /^win/ instead. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The check `!methods.empty` is always true (arrays have no `.empty` property), so the dead else-branch never returned the server's rejection line to the caller. Authentication failures always surfaced as the generic "No authentication methods left to try" instead of the actual REJECTED line. Use methods.length and wrap the line in an Error so callers see a real message. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`Object.assign(obj, {})` writes into its first argument, so subObj
was the same reference as obj. The subsequent `subObj.name += ...`
permanently mutated the caller's DBusObject path. Use
`Object.assign({}, obj)` to make a real shallow copy.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The loop started at index 1 with a comment saying "skip the root node", but the root <node> was already unwrapped one line above. xmlnodes is the array of child <node> elements, so starting at 1 silently dropped the first child. Start at 0. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Throwing inside the xml2js parseString callback escaped past the caller's error-handling path and would surface as an uncaught exception. Mirror the err-first callback style used immediately above and on lines 33/111/132. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The introspection handler iterated over exportedObjects with a loop variable `path` but looked up bus.exportedObjects[msg.path] and stored to nodes[msg.path] inside the loop. When both the requested path and child paths were exported, every iteration overwrote the same key with the same parent object and the children were never recorded. Treat path === msg.path as the "this is the requested object" case and let the existing else branch enumerate true children. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Release-focused PR (“v0.7.6”) that bundles a set of bug fixes and hardening improvements across handshake/auth, message parsing, bus behavior, introspection tooling, and CI.
Changes:
- Fixes core protocol handling (message endianness, handshake behavior/errors, property setting, void returns, UnknownInterface, Error returns).
- Hardens exported object maps against prototype pollution and removes debug logging / console spam.
- Updates tooling/workflows and adds regression tests + fixtures covering the fixed behaviors.
Reviewed changes
Copilot reviewed 35 out of 38 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| test/test_security_proto_pollution.js | Adds regression coverage for null-prototype exported object maps. |
| test/test_introspect.js | Extends introspection fixture coverage to include sub-nodes and validates returned node list. |
| test/test_fix_stdifaces_set.js | Regression test ensuring Properties.Set writes caller-provided variant value. |
| test/test_fix_stdifaces_children.js | Regression tests for introspection XML including exported child sub-paths. |
| test/test_fix_server_handshake_quiet.js | Ensures server handshake no longer logs to console during normal operation. |
| test/test_fix_put_floatle.js | Ensures Put.floatle matches Buffer IEEE754 encoding and offset handling. |
| test/test_fix_message_endianness.js | Validates DBusBuffer endianness wiring and message parser endianness propagation. |
| test/test_fix_introspect_parse_error.js | Ensures XML parse errors surface via callback (no uncaught throws). |
| test/test_fix_introspect_clone.js | Ensures introspection recursion clones objects rather than mutating caller input. |
| test/test_fix_handshake_windows.js | Guards Windows platform detection regex behavior. |
| test/test_fix_handshake_auth_reject.js | Ensures auth rejection reason is surfaced as an Error message. |
| test/test_fix_bus_void_return.js | Ensures void/empty-signature methods don’t marshal undefined into replies. |
| test/test_fix_bus_unknown_interface.js | Ensures UnknownInterface error is returned when interface missing on an exported path. |
| test/test_fix_bus_serial.js | Ensures signal emission increments serial exactly once per signal. |
| test/test_fix_bus_error_return.js | Ensures returning an Error produces a D-Bus error reply (including dbusName override). |
| test/test_fix_bus_error_message.js | Ensures error replies expose err.message as a string and preserve full body as err.body. |
| test/test_fix_bus_emit_wrap_once.js | Ensures emit wrapper is applied once across re-exports and doesn’t duplicate dispatch. |
| test/test_fix_bus_emit_typo.js | Ensures emit-without-name throws “undefined signal” (typo regression). |
| test/test_fix_address_x11.js | Ensures address-x11 lazy-load behavior and actionable error when x11 isn’t installed. |
| test/fixtures/introspection/with-subnodes.xml | Adds fixture XML containing child nodes. |
| test/fixtures/introspection/with-subnodes.json | Adds expected JSON for sub-node introspection fixture. |
| package.json | Adds x11 as documented optional dep and bumps dev tooling versions. |
| package-lock.json | Updates lockfile to reflect dependency/tooling upgrades. |
| lib/stdifaces.js | Fixes child aggregation in introspection and corrects Properties.Set behavior. |
| lib/server-handshake.js | Removes console logging during server handshake. |
| lib/put.js | Fixes float little-endian encoding via Buffer.writeFloatLE. |
| lib/message.js | Fixes parsing by honoring endianness flag for header/body and DBusBuffer usage. |
| lib/introspect.js | Fixes error propagation, object cloning during recursion, and sub-node inclusion. |
| lib/handshake.js | Fixes Windows platform detection and surfaces REJECTED reason as Error. |
| lib/dbus-buffer.js | Adds endianness-aware reads and correct 64-bit word ordering for BE. |
| lib/bus.js | Fixes option forwarding, exportedObjects hardening, serial increment, UnknownInterface, void returns, Error returns, and wraps emit once. |
| lib/address-x11.js | Lazy-loads optional x11 dependency with clear install guidance. |
| index.js | Includes arg0 in unixexec argument collection. |
| examples/portforward.js | Moves portforward example out of lib/ to avoid side effects. |
| examples/dbusproxy/index.html | Switches jQuery load to HTTPS. |
| bin/dbus2js.js | Aligns xml2js parsing assumptions with introspection output structure. |
| .github/workflows/run-test.yml | Sets least-privilege workflow permissions. |
| .github/workflows/release.yml | Bumps action versions used in release workflow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The Set branch hard-coded `impl[propertyName] = 1234` and ignored the variant the caller sent. Extract the value from the variant body (msg.body[2] = [parsedSignatureTree, [value]]) and assign it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
"signa" -> "signal". Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The fall-through `return new bus(conn)` dropped the second argument, silently discarding caller-supplied options (auth methods, direct, ayBuffer, ReturnLongjs). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The signal message already used `serial: self.serial++` in its construction, so the trailing `self.serial++` after sending the message was an unnecessary second increment that wasted a serial number per emitted signal. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The error callback received `{ name, message: args }` where args
was the raw D-Bus body array. Idiomatic JS expects err.message to
be a string, so callers doing `err.message.includes(...)` or
similar would crash. Use the first body element (the conventional
error text) for err.message and expose the full body via err.body.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ted object A method call against an existing exported path with an unknown interface logged to stderr and fell through to the methodCallHandlers lookup, eventually replying with the misleading UnknownService error. Send the spec-correct UnknownInterface error and stop. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Previously the reply gated body inclusion on `methodReturnResult !== null`, which meant: - a handler that returned undefined produced body=[undefined], which threw inside marshall and never replied to the caller; - a handler that legitimately returned null produced a malformed reply with the declared signature dropped. Only attach signature/body when the interface declares a return signature *and* the handler returned a defined non-null value. Also catch marshall errors so the caller receives a proper DBus error instead of an unhandled promise rejection. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
examples/return-types.js documents `return new Error(...)` as the way to send a DBus error from a service handler, but the bus treated any return value (including Error) as a success and tried to marshall it as the declared return type, producing an unhandled promise rejection. Detect Error instances on the success path and route them through sendError, mirroring the rejection path's dbusName/message handling. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Every server-mode handshake printed the client hello line and the post-OK BEGIN line to stdout, polluting host application logs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The hand-rolled IEEE 754 encoding was broken at multiple levels: the sign bit and exponent were combined with `&` instead of `|`, the mantissa's lower 16 bits were hard-coded to 0, and the offset was advanced by an extra 4 bytes after writing — leaving a 4-byte gap that would corrupt the next field. It also called console.dir/console.log on every invocation. Replace with Buffer.writeFloatLE which is correct, fast and built-in. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The D-Bus address spec for unixexec: allows arg0 as the conventional argv[0] passed to the spawned child, in addition to arg1, arg2, ... The loop started at n=1 and silently dropped arg0. Start at n=0. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The first byte of every D-Bus message header selects little- or
big-endian byte order ('l' / 'B'), but unmarshalMessages and the
standalone unmarshall always used readUInt32LE and would
misinterpret big-endian peers. Detect the flag, pass a
littleEndian option through to DBusBuffer, and switch all 16/32/
64-bit and double reads on it. Long.fromBits is called with words
swapped when reading BE so its low/high arguments stay correct.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
dbus2js used the legacy xml2js '0.1' defaults (attributes under '@'), while lib/introspect.js uses the current defaults (attributes under '$'). Standardize on the current defaults so both code paths read introspection XML the same way. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…onal The module top-level required `x11`, which is not in the package's dependencies, so importing the module always threw a generic "Cannot find module 'x11'" error. Lazy-load the peer inside the function and surface a clear, actionable error message. Document `x11` alongside `abstract-socket` in the optionalDependencies comment. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Each exportInterface call wrapped obj.emit again, so re-exporting the same object on a second path/interface produced a chain of nested wrappers. A single signal emit then walked every wrapper, duplicating local listeners and burning extra serial numbers. Track the wrap with an __dbusInterfaces marker on the object, push (iface, path) pairs onto it, and let one wrapper dispatch to every matching registered interface. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
lib/portforward.js was a runnable demo, not library code: at require time it imports `abstract-socket` (now an optional dep that this fork removed), reads DBUS_SESSION_BUS_ADDRESS, and binds port 3334. Because package.json#files publishes lib/* it shipped to npm and would crash any consumer who imported it. Move it to examples/ where the other runnable scripts live. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bus.exportedObjects and the per-path entry map (where iface.name is the key) are now created with Object.create(null) so a malicious or accidental interface name like "__proto__" cannot reach the global Object.prototype, and remote peers cannot probe inherited properties via msg.path or msg.interface lookups. Resolves CodeQL alert js/prototype-polluting-assignment in exportInterface. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The demo loaded jQuery from ajax.googleapis.com over plain HTTP, which made it trivially MITM-able. Switch the schema to https. Resolves CodeQL alert js/functionality-from-untrusted-source. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Without an explicit permissions block, GITHUB_TOKEN inherits the repo-default scope (often write-all). The reusable build/test job only needs to read the repo, so pin the workflow to contents: read. Resolves CodeQL alert actions/missing-workflow-permissions. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
NorthernMan54
approved these changes
May 25, 2026
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.