fix(api): v1 'default' source alias deep-scopes by the resolved id, not the literal

[Yeraze]

Root cause

The v1 API mounts canonical per-source routes at /api/v1/sources/:sourceId/... and supports a default alias that resolves to the primary/default source. The attachSource middleware (src/server/routes/v1/sourceParam.ts) resolves the alias, attaches the resolved Source to req.source, and normalises req.params.sourceId to the concrete resolved id.

The bug: Express re-derives req.params for each mergeParams sub-router from the matched URL. So handlers inside the sub-routers (nodesRouter, messagesRouter, etc. — all mounted as nested Router({ mergeParams: true })) that read req.params.sourceId saw the raw URL literal "default", NOT the id attachSource wrote. req.source (a request-level property) survives the re-derivation and carries the resolved source.

Consequence: requesting /api/v1/sources/default/nodes returned 200, but the handler deep-scoped its DB query by the literal string "default", which matches no rows when the actual default source id differs — silently returning empty results.

Verified empirically with a minimal Express repro: inside a nested mergeParams sub-router, req.params.sourceId === "default" while req.source.id === "<concrete>".

Fix

Added resolvedSourceIdFromPath(req) to sourceParam.ts. It prefers req.source?.id (survives re-derivation), falls back to the raw path param, and returns undefined on the legacy root mounts. Routed every per-router getScopedSourceId helper plus the status and actions handlers through it.

The existing ?sourceId= query fallback branches (which serve the legacy root mounts) are left fully intact — only the path-derivation line changed. No new any (a narrow typed MaybeRequestWithSource accessor is used).

Files touched:

• sourceParam.ts — new resolvedSourceIdFromPath helper + MaybeRequestWithSource type
• nodes.ts, channels.ts, messages.ts, network.ts, packets.ts, telemetry.ts, traceroutes.ts, positionHistory.ts — getScopedSourceId now derives the path id via the helper
• status.ts — inline path-scope now via the helper
• actions.ts — 4 handlers now read the resolved id via the helper (was req.params.sourceId as string)
• sourceParam.defaultAlias.test.ts — new regression test

Red → green proof

New test sourceParam.defaultAlias.test.ts reproduces the exact production wiring (attachSource + nested mergeParams sub-router) and seeds a source whose concrete id (src-primary-0001) differs from the literal "default", with nodes.getAllNodes returning rows only for the concrete id.

• Red (helpers reverted): scopes the nodes query by the resolved concrete id → failed (response data empty; query scoped by "default").
• Green (fix applied): both assertions pass — seeded rows returned, getAllNodes called with the concrete id and never with "default".

Verification

• Full Vitest suite: 9611 passed, 0 failed, 0 suite failures (JSON reporter, success: true).
• tsc -p tsconfig.server.json --noEmit: clean.
• npm run lint:ci: exit 0 (no baseline change).

Note for reviewers

The held PR #4189 removes the ?sourceId= query fallbacks (legacy root mounts) from these same getScopedSourceId helpers. This PR only changes the path-derivation line and adds an import, so #4189 should need a trivial rebase.

🤖 Generated with Claude Code

[github-actions]

Claude finished @Yeraze's task —— View job

---

PR Review: fix(api): v1 'default' source alias deep-scopes by the resolved id

Tasks:

• Gather context and understand the PR
• Review core fix in sourceParam.ts
• Review test coverage in sourceParam.defaultAlias.test.ts
• Review router changes for consistency
• Review actions.ts changes
• Post final review

---

Summary

This is a well-targeted, correctly-reasoned bug fix. The root cause diagnosis is accurate (Express re-derives req.params for each nested mergeParams sub-router from the matched URL, stomping the normalised value attachSource wrote) and the fix is the right approach — anchor to req.source.id which is a request-level property that survives re-derivation.

Code Quality ✅

sourceParam.ts — resolvedSourceIdFromPath helper (src/server/routes/v1/sourceParam.ts:172-176)

The implementation is clean and correctly typed. MaybeRequestWithSource is an appropriately narrow interface rather than an as any cast — good practice. The two-line function is easy to reason about:

export function resolvedSourceIdFromPath(req: Request): string | undefined {
  const fromSource = (req as MaybeRequestWithSource).source?.id;
  if (typeof fromSource === 'string' && fromSource) return fromSource;
  return typeof req.params.sourceId === 'string' ? req.params.sourceId : undefined;
}

The fallback to req.params.sourceId when req.source is absent correctly preserves legacy root-mount behaviour where attachSource never ran.

Router files — The getScopedSourceId pattern is applied consistently across all 8 sub-routers (nodes.ts, channels.ts, messages.ts, network.ts, packets.ts, telemetry.ts, traceroutes.ts, positionHistory.ts). Each change is a one-line import addition + one-line helper body change. Uniform and minimal.

actions.ts — The actions file is slightly different: each handler calls attachSource itself inline (not at the router level), then calls resolvedSourceIdFromPath(req) to get the resolved id. This is correct — attachSource has already run by the time the handler body executes, so req.source is populated. The as string cast is safe given attachSource guarantees resolution before next().

Potential Issues / Minor Observations

1. status.ts — getNode missing sourceId scope (src/server/routes/v1/status.ts:32)

const node = await databaseService.nodes.getNode(Number(localNodeNum));

This call doesn't pass statusSourceId. If MeshMonitor has multiple sources with the same local node number (unlikely but architecturally possible), it could return the wrong node's longName/shortName. This is a pre-existing issue, not introduced by this PR, but worth noting since status.ts was touched.

2. Test — positionHistoryRouter mounted before nodesRouter (sourceParam.defaultAlias.test.ts:85-87)

app.use(
  '/api/v1/sources/:sourceId/nodes',
  attachSource('nodes', 'read'),
  positionHistoryRouter,
  nodesRouter
);

The comment says "Exact production wiring from index.ts" — this should be verified to actually match production mount order. If it doesn't, the test is still valid for the regression it covers, but the comment would be misleading.

3. Test — no non-admin user path covered

The test only exercises the admin path (ADMIN = { id: 1, isAdmin: true }). A non-admin user going through resolveDefaultForUser's permission loop would also arrive at req.source.id being set, but the test doesn't verify this code path. Given the bug is in resolvedSourceIdFromPath (not in permission resolution), this is acceptable — a non-admin path test would add confidence but isn't strictly required for this regression.

4. Test — vi.hoisted used correctly

The vi.hoisted pattern is used correctly to ensure CONCRETE_ID and mock functions are available when vi.mock factories run. This is the right Vitest pattern for constants referenced inside vi.mock callbacks.

Security ✅

No new security concerns. The fix narrows data exposure (queries now correctly scope by the real source id rather than a string that matches nothing), which is strictly an improvement.

Performance ✅

No performance regression. resolvedSourceIdFromPath is O(1) — a property read and a string check.

Overall

This is a correct, minimal, well-tested fix for a real data-exposure bug (empty results when using the default alias). The red→green proof in the PR description accurately describes the regression test behaviour.

Approve with one non-blocking suggestion: consider adding statusSourceId to the getNode call in status.ts:32 while it's being touched, to close the latent cross-source ambiguity — but this is a pre-existing issue and out of scope for this fix.

[Yeraze]

Re the non-blocking note: agreed status.ts's getNode cross-source ambiguity is a pre-existing separate issue — leaving it out of this fix per the review's own scoping. Merging.