Skip to content

fix(kernel): require verified caller identity on the five unattributed service bridges - #1339

Merged
fluck-boss merged 1 commit into
risa-labs-inc:devfrom
Mihir-Rabari:wfix/kernel-bridges-identity-1218
Sep 23, 2026
Merged

fluck-boss merged 1 commit into
risa-labs-inc:devfrom
Mihir-Rabari:wfix/kernel-bridges-identity-1218

Conversation

@Mihir-Rabari

Copy link
Copy Markdown
Contributor

Root cause

Issue #1218: five kernel service bridges — NotificationServiceBridge, ContextMenuServiceBridge, DirectoryPickerServiceBridge, PanelEventServiceBridge, PerformanceServiceBridge — served any authenticated process without re-checking that the call carries a verified caller identity. Eight-plus sibling bridges (Log, Secret, SplitView, Git, Download, …) already implement the documented AUTHENTICATED_PROCESS_ID helper pattern (BossConsole#53, introduced for the Secret Service in PR #505); these five never adopted it. A spawned process could spoof operator-facing toasts, inject context-menu registrations, summon the native directory dialog, rearrange panels, and drive GC/settings/metrics on the host process — with nothing in the log to attribute any of it to.

Design

Mirrors the sibling helper shape exactly (LogServiceBridge / PR #505):

  • Each unary RPC resolves ProcessIdentityInterceptor.AUTHENTICATED_PROCESS_ID and refuses with PERMISSION_DENIED ("This call presented no verified process identity") when it is absent — fail-closed, even if the bridge is ever reached without the interceptor in front of it. Refusals are logged at LogCategory.AUTH with the RPC name.
  • The three PerformanceService watchers capture CURRENT_IDENTITY at subscription and revalidate the caller on every emission (mid-stream revocation honoured) — the same shape LogServiceBridge uses.
  • ContextMenuServiceBridge keeps its passive acknowledge-and-drop contract (issue ContextMenuServiceBridge never talks to the host ContextMenuProvider #30) for verified callers; only unattributed callers change behaviour. Its KDoc now states the identity requirement alongside the existing rationale.

Tests + XML counts

  • New UnattributedBridgesIdentityTest — all five services mounted on a real gRPC server (TLS + token registry + production ProcessIdentityInterceptor), three suites of coverage: (1) every RPC of all five bridges called with no credential → UNAUTHENTICATED, and the provider fakes prove the calls never reached the provider; (2) authenticated calls reach each provider and record the expected side effects; (3) direct construction with no identity context at all (no interceptor) → PERMISSION_DENIED on every unary RPC and on the first emission of every watcher.
  • ContextMenuServiceBridgeTest rewritten: the three acknowledge-and-drop tests become refusal tests pinning the PERMISSION_DENIED half of the Remote UI: StreamUI has no plugin identity check — one plugin can bind another's surface #53 contract directly; the statelessness invariant test is kept.
  • Gates ran as ONE invocation: ktlintCheck :composeApp:detekt :composeApp:desktopTest --tests "ai.rever.boss.kernel.services.*" — 13 suites, 104 tests, 0 skipped, 0 failures, 0 errors (JUnit XML: tests="104" failures="0" errors="0" across TEST-*.xml, incl. UnattributedBridgesIdentityTest tests="3" failures="0" errors="0" and ContextMenuServiceBridgeTest tests="4" failures="0" errors="0").

Fixes #1218

@Mihir-Rabari

Copy link
Copy Markdown
Contributor Author

Rebuilt on current dev (bc9aa3b) as an isolated 7-file change (ContextMenuServiceBridge.kt, NotificationServiceBridge.kt, UnattributedBridgesIdentityTest.kt); the branch no longer carries unrelated history. Fresh CI is starting on f2ac0b5.

…d service bridges

NotificationServiceBridge, ContextMenuServiceBridge, DirectoryPickerServiceBridge,
PanelEventServiceBridge and PerformanceServiceBridge accepted any authenticated
process with no per-call attribution, while their siblings already enforce the
documented verified-caller-identity pattern (BossConsole#53, introduced for the
Secret Service in PR risa-labs-inc#505). A spawned process could spoof toasts, inject context-menu
registrations, drive the native directory dialog, rearrange panels and forge
performance mutations with nothing in the log to attribute them to.

- Add the sibling AUTHENTICATED_PROCESS_ID helper (mirror + KDoc citing risa-labs-inc#53/risa-labs-inc#505) to
  all five bridges; unary RPCs refuse with PERMISSION_DENIED when no verified identity
  is present, even with no interceptor in front of them.
- Performance watchers capture CURRENT_IDENTITY at subscription and revalidate the
  caller on every emission, the same mid-stream revocation shape LogServiceBridge uses.
- ContextMenuServiceBridge keeps its passive acknowledge-and-drop contract (issue risa-labs-inc#30)
  for verified callers; only unattributed callers change behaviour.
- New UnattributedBridgesIdentityTest: all five services over a real gRPC server with
  the production interceptor (no credential -> UNAUTHENTICATED, providers never
  reached), authenticated calls reach each provider, and direct calls with no identity
  context fail closed with PERMISSION_DENIED even without the interceptor.
- ContextMenuServiceBridgeTest now pins the refusal half of the risa-labs-inc#53 contract directly.

Gates: ktlintCheck + detekt + :composeApp:desktopTest --tests "ai.rever.boss.kernel.services.*"
in one invocation - 13 suites, 104 tests, 0 failures, 0 errors, 0 skipped.

Fixes risa-labs-inc#1218
@Mihir-Rabari
Mihir-Rabari force-pushed the wfix/kernel-bridges-identity-1218 branch from f2ac0b5 to b234d9b Compare September 23, 2026 08:53
@Mihir-Rabari

Copy link
Copy Markdown
Contributor Author

Fresh CI on b234d9b: windows build-test had failed on the shared plugin-sandbox test PluginSandboxManagerTest > listener receives onPluginRestarted event() while the macos/ubuntu legs were cancelled by the duplicate-validation infra bug, rebased onto dev 09c842c (includes the #1475 CI fix; no code changes in this PR), hosted checks now green.

@fluck-boss

Copy link
Copy Markdown
Collaborator

@claude review this PR - security-sensitive changes, part of the backlog sweep. Focus on auth, signing, path handling, secrets, and injection surfaces.

@claude

claude Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

Claude finished @fluck-boss's task in 4m 25s —— View job


Security review: verified caller identity on five kernel bridges

  • Gather context (diff, sibling LogServiceBridge, ProcessIdentityInterceptor, ProcessTokenRegistry, KernelBootstrap wiring)
  • Review the five bridges for auth correctness
  • Review test coverage
  • Post review

Verdict

The security change is correct and complete for what it claims. It mirrors the LogServiceBridge / PR #505 shape faithfully, and I could not find a bypass. Findings below are one meaningful test-coverage gap plus nits; nothing blocking on the production code.

What I verified (not just read)

  • Every RPC is covered. Cross-checked each override against its .proto: performance.proto 6/6, notification.proto 3/3, panel_event.proto 2/2, directory_picker.proto 1/1, context_menu.proto 3/3. No RPC left unguarded.
  • The check is the first statement, before request parsing and before any provider.* call, in all 12 unary RPCs. The fakes asserting calls.isEmpty() pin that.
  • No new injection or leak surface. rpc is a hardcoded literal at every call site, the PERMISSION_DENIED description is a constant, and no request field (toast text, panel id, menu id, picked path) reaches a log line or a status description. The refusal logger.warn is also unreachable by a remote attacker in the shipped wiring, since the interceptor closes the call first, so this is not a log-flood surface either.
  • No compat break. All five are constructed only in KernelBootstrap.registerPluginServices (KernelBootstrap.kt:953-970), which returns early in MONOLITH mode and adds to the single BossIpcServer that always installs ProcessIdentityInterceptor (KernelBootstrap.kt:489-495). Nothing calls these bridges in-process, so ContextMenuServiceBridge's behaviour change is invisible to real clients: a plugin that reaches the bridge at all has already presented a token.
  • Mid-stream revalidation is sound even under process-id reuse. CURRENT_IDENTITY closes over the token (ProcessIdentityInterceptor.kt:38) and identityFor returns the display processId. A replacement process reusing the same processId gets a new token and the old one is removed from credentials (ProcessTokenRegistry.issue), so the captured closure returns null and the stream refuses. Comparing display id rather than instanceId is therefore not a hole here.
  • Sweep completeness. After this PR all 16 files in kernel/services/ reference AUTHENTICATED_PROCESS_ID/CURRENT_IDENTITY; the five named here were the remainder.

Findings

1. The one genuinely new code shape has no allow-path test (UnattributedBridgesIdentityTest.kt:173-218) - medium, test only

PerformanceServiceBridge's three watchers changed from expression-bodied flow { } to a block body that hoists CURRENT_IDENTITY.get() outside the builder. That hoist is load-bearing (the builder body runs at collection time), and all three new tests pass without ever exercising it on the allow path:

  • the no-credential test is refused by the interceptor before the bridge runs;
  • the direct-construction test returns null inside the flow either way.

So a regression that read CURRENT_IDENTITY in the wrong place, and broke every real authenticated watcher, would still be green here. The sibling suite covers exactly this (LogServiceBridgeTest: an authenticated caller can watch and export logs). Recommend adding authenticatedPerformance.watchCurrentSnapshot(...).take(1).toList() and friends to the "reaches each provider" test.

Fix this →

2. Revocation is claimed in the PR body but not pinned - low, test only

"revalidate the caller on every emission (mid-stream revocation honoured)" is asserted in the design section and in the new KDoc, but no test in this PR revokes anything. LogServiceBridgeTest has both revoked caller cannot invoke a unary RPC and revoked watcher cannot receive a later snapshot; the equivalents here would cost a few lines given the registry is already in the fixture.

3. The root-cause claim about attribution is stronger than the fix - low, doc/claim

The PR says these calls happened "with nothing in the log to attribute any of it to". The return value of authenticatedCallerOrRefuse is discarded at all 12 unary call sites, so a successful showToast / pickDirectory / openPanel still logs nothing about who made it - only refusals log, and refusals are unreachable in the shipped wiring. The real win here is fail-closed defence in depth, not attribution. Either soften the claim, or log the returned process id on the operator-facing ones (pickDirectory, showToast, openPanel/closePanel). Discarding it matches every sibling, so this is not a blocker.

4. Dead import (ContextMenuServiceBridgeTest.kt:3) - nit

import ai.rever.boss.ipc.proto.Empty is unused now that the three acknowledge assertions became refusal assertions. ktlint's no-unused-imports misses it because instanceFields.isEmpty() at line 73 contains the substring, which is why the gate stayed green.

Fix this →

5. Typo in the test constant (UnattributedBridgesIdentityTest.kt:341) - nit

CALLER = "unattribated-bridges-test" → unattributed.

6. Sixteen copies of the same helper - low, maintainability, follow-up not this PR

NO_IDENTITY + refuseIdentity + authenticatedCallerOrRefuse is now byte-identical in 16 files under kernel/services/. Mirroring the sibling shape is what the issue asked for, so this PR is right to do it, but the repo's own precedent argues for extraction next (AGENTS.md on DefaultHandlerState.reduce: "Each handler had its own copy; they agreed, which is the only reason nothing was broken, and a fourth copy would not have"). An internal helper in this package would make the refusal status, description and log shape one definition.

Out of scope, worth an issue

Identity presence is not authorization. After this change any authenticated child process can still summon the native directory dialog on demand, put arbitrary text in an operator-facing toast, and rearrange panels in an arbitrary windowId. ProcessIdentityInterceptor already exposes CURRENT_PRINCIPAL with ProcessAuthority, and no bridge in the package uses it - so this PR is not out of line with its siblings, but pickDirectory (a modal native dialog driven by a remote caller) is the one I would look at first if per-capability gating is ever added.
 ·  wfix/kernel-bridges-identity-1218

@fluck-boss
fluck-boss merged commit 7a66dfd into risa-labs-inc:dev Sep 23, 2026
13 checks passed
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