chore(deps): dependency update (safe set) + remove unused + upgrade triage#4
Closed
msallin wants to merge 24 commits into
Closed
chore(deps): dependency update (safe set) + remove unused + upgrade triage#4msallin wants to merge 24 commits into
msallin wants to merge 24 commits into
Conversation
A stray leading or trailing space in the username (easily introduced by browser autofill, paste, or mobile autocapitalize) was stored verbatim in security.json. The login lookup compares the typed name against the stored one byte-for-byte, so the account became impossible to log into: enableSecurity returns 200 but every subsequent login fails with "Invalid username/password". Trim the username in addUser before the duplicate check and before storing it, reject a username that is empty after trimming, and trim the name at login so a stray space typed into the login box still matches.
- @signalk/nmea0183-signalk ^3.19.1 -> ^3.20.0 (root + streams) - @signalk/signalk-to-nmea0183 ^1.16.1 -> ^1.17.2 - @signalk/nmea0183-utilities ^1.1.0 -> ^1.1.1 (streams)
…tail Opening a plugin detail page unmounted the app list, so the selected view (All/Installed/Updates) and the search term were lost on "Back to Store", always resetting to All with no search. Hold both in the store instead of component state so they survive the unmount/remount round trip. closes SignalK#2750
At runtime `app.debug` is a debug-module Debugger (createDebug -> coreDebug), but ServerAPI typed it as a bare function. That meant the recommended `if (app.debug.enabled)` guard for expensive log arguments did not type-check for plugins, forcing local type workarounds. Type `debug` as a callable carrying `enabled: boolean`, document the guard pattern, and add a type-contract test. No new dependency; the package stays self-contained. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Truthy checks on latitude/longitude rejected valid 0 values, breaking relative-position subscriptions at the equator or prime meridian. Use Number.isFinite so 0 is accepted for the subscription origin and for incoming vessel positions. fixes SignalK#2717 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- @types/uuid 8.3.1 -> 10.0.0 (types only; usage limited to v4/validate) - ansi-to-html 0.6.14 -> 0.7.2 (admin-ui log rendering)
registerDeltaInputHandler is documented to intercept deltas "before they are processed by the server", but handleMessage ran the handler chain last — after ingestDelta, the unfilteredDelta emit and toPreferredDelta. So a handler saw priority-stripped values, and its modifications never reached the unfiltered stream or the cache's freshness stamping. The ordering has been this way since source priority was introduced; the recent rework made toPreferredDelta a real filter and added the cache / unfiltered emits ahead of it, exposing the latent contract violation. Make DeltaChain a pure handler runner: process(msg, dispatch) runs the handlers then hands the result to a caller-supplied dispatch. handleMessage now runs the chain first on the raw delta and performs ingestDelta, unfilteredDelta, toPreferredDelta and the final fan-out in the dispatch callback, so every downstream step sees the post-handler delta.
* fix(nmea-tcp): remove event listeners on stop
The send listener registered on app.signalk ('nmea0183') and app
('nmea0183out') in start() was never removed in stop(). Restarting the
interface left the old listeners attached, so every NMEA sentence was
broadcast to connected clients once per restart.
Hoist the listener reference into the interface closure and detach it in
stop().
closes SignalK#2533
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
addValue splits a delta path on '.' and walks it with
previous[part] = previous[part] || {}. A segment of __proto__,
constructor, or prototype resolves to an existing prototype object,
so the walk reaches and mutates Object.prototype process-wide, and
deeply dotted paths nest without bound. Reachable from any
write-authorized delta or notification raise.
Reject deltas whose path contains one of these reserved segments
before building the tree. They are never valid Signal K paths.
fixes SignalK#2768
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ll model A meta delta only updated the metadata registry, never the live tree leaf. When an N2K device echoes a path a plugin also owns (e.g. PGN 126720 -> electrical.displays.raymarine.*), the device value creates the leaf and the plugin's later supportsPut (a meta-only delta) never reached it, so clients reading the leaf (e.g. KIP) saw no supportsPut. Merge meta onto an existing value-bearing leaf when a meta delta arrives. Identity-less contexts and meta-only paths are left to the registry alone, so no orphan or phantom tree node is created. DELETE of metadata now also clears the field from the leaf so the full model stays consistent.
…works on slow links (SignalK#2779) * fix(appstore): relax Happy Eyeballs connect timeout so app store works on slow links Node >= 20 caps each Happy Eyeballs per-address connect attempt at 250 ms. On high-latency cellular/satellite links the SYN round-trip to the app store hosts (signalk.org, registry.npmjs.org, api.github.com, the CDNs) exceeds that, so every attempt aborts and the app store appears intermittently offline regardless of the AbortController timeout above it. Install a global undici dispatcher at startup with autoSelectFamilyAttemptTimeout=5000, preserving IPv6->IPv4 fallback while giving slow links room to complete the handshake. This covers every fetch() in the server, including both app store code paths. * fix(appstore): lower Happy Eyeballs connect timeout to 1000 ms 5000 ms only ever delayed fallback from a stalled address, not normal connects, so a down app-store host could hang for up to 5 s per address tried. 1000 ms still covers a bad satellite SYN round-trip while keeping that worst case an order of magnitude shorter. --------- Co-authored-by: dirkwa <dirkwahrtheit@gmail.com>
The "Simulate App Store install" and "Install plugin into SignalK" steps captured the tarball with PLUGIN_TGZ=$(npm pack ...), assuming stdout is only the filename. A plugin "prepare" script (e.g. tsc) prints a lifecycle banner to stdout, polluting the capture, so the following npm install resolves a bad path (ENOENT, exit 254). On npm >= 11 (Node 24) --ignore-scripts suppresses the pack-time prepare, so this only fails on the Node 22 legs, where prepare runs despite --ignore-scripts. It affects any TypeScript plugin with a prepare build step. Read the filename from `npm pack --json` instead, tolerating a leading banner — the same robust approach the "Verify npm pack includes all required files" step already uses. fixes SignalK#2777
…2771) * feat(appstore): surface npm install log for failed installs The server already captured npm output for every install/remove in modulesInstalledSinceStartup but never exposed it, leaving users with a bare Failed badge and no way to diagnose or report the failure. Add GET /skServer/appstore/installLog/:name (admin-gated like the rest of /appstore) returning the captured output and exit code. The Failed badge in the list/detail views and the Install failed pill on grid cards now open a modal showing the log, with copy-to-clipboard and a download fallback. Copying works without a secure context via the execCommand fallback shared with DataBrowser, now extracted to utils/clipboard.ts. Dropping z-index 0 from the card body/footer is required so the failed pill (and the author button) can stack above the card's stretched link and receive clicks. --------- Co-authored-by: dirkwa <dirkwahrtheit@gmail.com>
* fix(security): block SSRF in remote connection endpoints The testSignalKConnection, requestAccess and checkAccessRequest endpoints passed an attacker-controlled host straight to http(s).request, letting the server be driven to loopback, the cloud metadata service (169.254.169.254) and other non-routable destinations (GHSA-q59x-jc9f-gfqf). Validate the destination before connecting: reject special-use addresses (loopback, link-local/metadata, unspecified, multicast, reserved) for both IP literals and resolved hostnames, pinning the connection to a checked address to prevent DNS rebinding. Private LAN ranges stay reachable so the intended use - connecting to another Signal K server on the boat network - keeps working. Also require requestId to be a UUID to stop path traversal in checkAccessRequest. * fix(security): harden remote connection endpoints after review Follow-up hardening of the SSRF fix for the remote-connection endpoints, addressing maintainer review: - Validate request bodies with TypeBox (the existing serverroutes pattern) instead of ad-hoc checks: host must be a non-empty string, port a number or numeric string in range, and requestId a UUID - which subsumes the earlier requestId path-traversal guard. - Return a single generic connection error instead of echoing the raw transport error, so the endpoints can no longer be used to map internal network topology by error differentiation. The real cause is still logged server-side via debug; post-authentication states stay distinct. - Reflect only the fields the admin UI consumes from remote responses (server id/version, access-request state/requestId/permission/token) rather than passing the remote body through verbatim. - Block the NAT64 well-known prefix (64:ff9b::/96) and deprecated IPv4-compatible (::/96) IPv6 forms, which otherwise encode loopback and metadata addresses past the guard on translating networks. --------- Co-authored-by: dirkwa <dirkwahrtheit@gmail.com>
es-check defaults to a 'script' sourceType, so an ESM plugin (package.json "type": "module") has its top-level import/export parse as a script and falsely report ES2024+ syntax in the Cerbo GX / Node 20 ES2023 check — even when the plugin only uses ES2015 module syntax and is otherwise ES2023-clean. es-check does not infer module mode from package.json "type", so pass --module when it is set. Verified on an ESM plugin: 'es-check es2023 index.js' errors in script mode; 'es-check es2023 --module index.js' reports no errors. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…oot (SignalK#2744) * fix(path-metadata): resolve metadata by path independent of context root Metadata is a path-keyed, context-independent namespace: a path's units and description are the same regardless of which context it appears under (vessels.<id>, meteo.<id>, aircraft.<id>, a bare meteo, ...). Re-key the identity-scoped seed entries (/vessels|/aircraft|/aton|/sar) to a single path-only matcher so a lookup under any context root resolves the same spec meta, and key runtime meta additions by path alone so a meta delta under one context populates the path for all contexts. Keep allMetadata on the original on-disk keys and move runtime per-path clones to a separate map, so getAllMetadata() and the /paths endpoint keep their exact shape and the Path Reference UI is unchanged. * fix(path-metadata): keep per-path clones ahead of the spec wildcard Two follow-ups on the path-keyed redesign: - internalGetMetadata unshifted its per-path clone to the FRONT of the lookup array (it was pushed to the back). A value delta clones the spec entry; a later PUT meta override merges into that same clone, but with the clone behind the generic spec wildcard getMetadata returned the un-overridden spec entry — the unit-preferences displayUnits override (km/h) was lost and fell back to the category preset. - getMetadata falls back to a literal-path lookup when the context-stripped form misses, so non-context root entries (/self, /version) still resolve when looked up bare, and a bare two-segment context resolves too. * docs(path-metadata): correct the toLookupPath contract comment Describe getMetadata's two-stage lookup (context-stripped form via toLookupPath, then a literal-path fallback) and drop the stale claim that the unconditional two-segment strip is always safe — it is not for bare singletons or short-segment paths, which the literal fallback handles. --------- Co-authored-by: dirkwa <dirkwahrtheit@gmail.com>
Add optional sourceRef to PathSpec, allowing callers to filter historical data by source. The source is specified inline in the paths query parameter using | as separator, e.g. `navigation.speedOverGround:average|n2k-on-ve.can0.115`. - Add `sourceRef?: SourceRef` to PathSpec interface - Add `sourceRef?: SourceRef` to ValueList entries in ValuesResponse - Parse | separator in splitPathExpression - Update OpenAPI documentation for the paths parameter Fixes SignalK#2706.
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
msallin
force-pushed
the
chore/update-dependencies
branch
from
9ea33c7 to
c4d5d02
Compare
msallin
changed the title
Bump the lower bounds of dependencies to the latest versions that already satisfy the existing semver ranges (npm update --save). No functional change: with no committed lockfile, CI already resolves these on a fresh install. Deliberate workspace/version-policy ranges (workspace "*", n2k-signalk ">=", instrumentpanel "0.x", streams "6.7.x") are left untouched. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Bump dependencies whose new major builds and tests green, either with no code change or with a small, type-level adaptation: No code change: - marked 17 -> 18, primus 7 -> 8, swagger-ui-express 4 -> 5 - serialport 11 -> 13 (optional, root + streams) - Font Awesome 6/5 -> 7 (admin-ui; widen fontawesome-free peer to ^7) - html-react-parser 5 -> 6, react-infinite-scroll-component 6 -> 7, react-markdown 9 -> 10, jsdom 27 -> 29 - validate-peer-dependencies 1 -> 2 - @types/node 18 -> 24 (typedoc-theme; align with engines/CI) - globals 16 -> 17 With type-level adaptation: - cookie 0.7 -> 1.1: parse() now returns string | undefined; widen the two request cookie maps. Drop @types/cookie (cookie v1 ships types). - bcryptjs 2 -> 3: v3's bundled Callback type makes the result param optional; mark the five hash/compare callbacks accordingly. Drop @types/bcryptjs (v3 ships types). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
These are declared but never imported, configured, or invoked, so drop them instead of bumping: - nock, zod-validation-error (root): no import/require anywhere; no zod dependency exists for zod-validation-error to act on. - lint-staged (root): no .lintstagedrc, no "lint-staged" config key, no husky/pre-commit hook, never invoked. - rollup-plugin-visualizer (admin-ui): not in vite.config; the bundle-analyzer script uses vite-bundle-visualizer instead. - geojson-validation (resources-provider-plugin): unused since the resource validation moved to the OpenAPI schema; also remove its orphaned src/@types/geojson-validation.d.ts ambient stub. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
msallin
force-pushed
the
chore/update-dependencies
branch
from
c4d5d02 to
ddf0c38
Compare
Owner
Author
|
Superseded by the upstream PR SignalK#2793 (same work, rebased onto the latest upstream master). |
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
5 participants
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.
Draft for review. Updates every dependency that can be updated safely, removes the ones that are dead weight, and triages the rest by effort/risk so the hard ones can be split into their own focused PRs (per this repo's "one logical change per PR" policy).
What's in this PR (3 commits)
1 — refresh in-range floors: bump lower bounds to the latest version already allowed by each semver range (
npm update --save). No functional change (no committed lockfile, so CI already resolves these). Deliberate workspace/version-policy ranges (*,>=,0.x,2.28.x,6.6.x) left untouched.2 — safe major upgrades: majors that build + test green, either with no code change or a small type-level adaptation:
fontawesome-freepeer to^7parse()returnsstring | undefined; widened 2 cookie maps; dropped@types/cookieCallbackresult is optional; adapted 5 hash/compare callbacks; dropped@types/bcryptjs3 — remove unused dependencies: declared but never imported/configured, so dropped rather than bumped:
nock,zod-validation-error,lint-staged(root),rollup-plugin-visualizer(admin-ui),geojson-validation+ its@typesstub (resources-provider).Verified: server
tscbuild, all workspace builds (admin-ui Vite, typedoc-theme on @types/node 24, AssemblyScript SDK), admin-ui vitest (226), server-api (42), path-metadata (8), eslint, prettier. (Local Windows run; server mocha + a streams path-separator test are known Windows-only false negatives — Linux CI is authoritative.)Open upstream Dependabot PRs
Mapping the 9 open dependency-bump PRs on
SignalK/signalk-serveragainst this work:Superseded — safe to close:
^13)^5)^0.12.3Reject — too big to bundle (each needs its own focused PR, see triage below):
TS1479build error +vendor.d.tsrewritebin/signalk-server-setuprequire()with no test coverageenginesstill admitsDeferred — needs its own PR (effort / risk)
These have real breaking changes against how we use them and should not ride along:
*routes (throws at startup); 5 sites read removed numbered params. Hits auth/PUT/resources/applicationData paths.eslint-plugin-reacthas no eslint-10 peer yet;@eslint-reactv5 is ESM-only and crashes the CommonJS flat config at load. Blocked upstream.ArrayFieldTemplate/ArrayFieldItemTemplatecontract changed; the plugin-config form renderer + SCSS need a rewrite. Feature-critical, no tests.src/zip.tsmust switch tonew ZipArchive(...); needs backup/restore test.baseUrl/moduleResolution nodebecome (silenceable) deprecations. Own changelog entry.react-compilerstill pins@babel/types7. Needs a real Vite build + admin-UI smoke.src/security.tscert generation must go async; PKCS#8 key format flip; TLS-startup test.enginesadmits 22.0–22.11 whererequire()throws.bin/signalk-server-setuprequire()s it at module top-level (shipped CLI, no test coverage).bin/signalk-server-setupconsumer.TS1479build error;packages/streamsvendor.d.tsambient needs rewriting.Blocked
chai-thingsandchai-json-equalhave no ESM successors. This is a test-framework migration (chai→node:assert/vitest), not a version bump.Also worth a look (out of scope here)
@signalk/signalk-schemais pinned1.0.xwhilepackages/streamswants^1.5.0(latest is 1.8.2) — looks like a stale pin.