fix(kernel): require verified caller identity for the active-tabs bridge

[Antriksh1984]

Summary

ActiveTabsServiceBridge had no caller-identity check on any of its seven RPCs. Any process able to open a connection to the kernel IPC server - not only the plugins the host itself loaded - could:

• watchActiveTabs to see every open tab's title and URL across every window and workspace - browsing history and open documents, disclosed with no credential;

• createBrowserTab to force the browser to navigate to an arbitrary URL;

• closeTab/selectTab to manipulate what the user is currently looking at.

• Gate every RPC behind authenticatedCallerOrRefuse(), the same PERMISSION_DENIED-on-no-verified-identity pattern PluginUIServiceBridge established for BossConsole#53.

• The one watch stream (WatchActiveTabs) captures identity synchronously before returning the flow and re-checks it on every emission, so a revoked token stops a live stream rather than only new calls - matching the shape PR #550's review settled on for this bridge's siblings.

Test plan

• New ActiveTabsServiceBridgeTest runs a real gRPC server behind ProcessIdentityInterceptor: every RPC refused with PERMISSION_DENIED for an anonymous caller and the fake provider never invoked; an authenticated caller can watch/read tabs and select/create/close tabs normally.
• ./gradlew :composeApp:compileKotlinDesktop :composeApp:compileTestKotlinDesktop
• ./gradlew :composeApp:desktopTest --tests "ai.rever.boss.kernel.services.ActiveTabsServiceBridgeTest"
• ./gradlew :composeApp:detekt

Review attribution and current validation

Antriksh (@Antriksh1984) authored the original production change and original tests. The original commits were replayed onto dev to remove conflicts caused by squash-merged shared history; author metadata is preserved. Reviewer follow-up adds unary and deterministic live-stream revocation tests, centralized stream refusal diagnostics and explicit emission-driven revocation documentation. These repairs are not original contributor work. The replayed current head requires its own CI; old-head green checks do not validate the new integration. Idle streams are not proactively disconnected. Shared transport/helper work (#575) remains separate. No UI was changed or visually exercised.

Final disposition (2026-09-14): current head 0139bcff5cc66324fba0e8a466b0deef3f091611 passed all ten hosted checks. Downloaded Linux/macOS/Windows artifacts each confirm five bridge tests, zero failures/errors/skips. Current Claude review found no blocking defect; reconciliation is recorded in the review comment. Merged into dev with the shared merge lock and exact-head guard, squash 2714a6ab56030c69337c92a5f375865e6ec484f7. No native UI exercise or external-runtime rollout claimed.

[kshivang]

@claude Please review current head 32ec17224bf8420e2ff11f22a6af48f8b8ecc462 against dev for correctness, security, regressions and relevant test coverage. Only this PR’s own commits were replayed onto current dev to remove squash-history conflicts; original contributor authorship is preserved. Check synchronous identity capture, all RPC gates, and unary/live-stream revocation. This is the single current-head review request for this sweep. New-head CI is pending.

[claude]

Claude finished @kshivang's task in 4m 1s —— View job

---

Review of 32ec172 against dev

• Read the PR diff (ActiveTabsServiceBridge.kt +60/-2, new ActiveTabsServiceBridgeTest.kt)
• Compare against sibling bridges and ProcessIdentityInterceptor semantics
• Check synchronous identity capture on the watch stream
• Check every RPC gate, unary + live-stream revocation
• Regression check: interceptor wiring and production client paths
• Assess test coverage

Verdict: correct, and faithful to the pattern the siblings settled on. No blocking defects found. Five non-blocking observations below, one of which I'd take before merge (the missing mutation audit line).

---

What I verified

All seven RPCs are gated. active_tabs.proto:13-34 declares WatchActiveTabs, RefreshTabs, SelectTab, GetTabUrl, GetFaviconCacheKey, CreateBrowserTab, CloseTab. Each of the six unary overrides calls authenticatedCallerOrRefuse as its first statement, before any provider touch; the stream is gated inside the flow. No method on the generated base class is left ungated.

Synchronous identity capture is right. ActiveTabsServiceBridge.kt:33 reads CURRENT_IDENTITY.get() outside the flow { } builder, so the key is read while the interceptor's Context is still current on the calling thread rather than on whatever thread eventually collects. This is byte-for-byte the shape DownloadServiceBridge.watchDownloads (:40-49) uses and the reason PluginUIServiceBridge.streamUI (:154-159) documents for passing identity as a plain parameter.

Unary vs stream key choice is correct. Unary reads AUTHENTICATED_PROCESS_ID (a per-call snapshot), the stream reads the CURRENT_IDENTITY closure and re-resolves on every emission — exactly what SecretServiceBridge:221-224 prescribes for a streaming RPC on these bridges.

Live-stream revocation actually terminates. ProcessTokenRegistry.revoke removes the token→id mapping, so the captured closure re-resolves to null, null != caller holds, and the refusal is raised before emit — so a revoked watcher cannot receive the snapshot that triggered the check. Worth noting for future readers: because a token maps to exactly one process id and issue() invalidates the predecessor, the closure can only ever return the same id or null. The != is therefore a revocation check, not an identity-change check, which is why reusing the "no current verified process identity" message is accurate today.

val smart cast at :39 is sound — currentIdentity?.invoke() ?: refuseIdentity(...) returns Nothing on the null branch, so currentIdentity is non-null below. Cosmetically different from the sibling's currentIdentity?.invoke(), behaviourally identical.

No in-repo regression. In KERNEL mode KernelBootstrap:427/486 always constructs the ProcessTokenRegistry and hands it to BossIpcServer, which installs the interceptor (BossIpcServer.kt:76) on the ServerBuilder — so it applies to services added after start() through the fallback registry too, which is how registerPluginServices adds this bridge. Children get BOSS_PROCESS_TOKEN from ProcessSpawner.kt:102 and attach it via ChildProcessBootstrap's ProcessTokenClientInterceptor. MONOLITH never registers the bridge. Out-of-repo (boss-microkernel-runtime) clients that build their own channel without the client interceptor will now be refused — but that is already true of the Download, Secret and Role bridges, so this adds no new class of breakage.

---

Observations

1. Mutating RPCs are gated but not attributed. selectTab, createBrowserTab and closeTab discard the caller id the helper returns (:74, :98, :107). Both siblings that gate mutations log an attributed audit line — SecretServiceBridge.logMutation (:236-244), RoleManagementServiceBridge:213. createBrowserTab is the most audit-worthy call on this bridge (a caller-supplied URL that drives the user's browser) and there is currently no record of who asked for it, only that somebody authenticated did. Suggest an INFO line with LogSanitizer.maskUriParams(url). Fix this →

2. The two stream refusal paths are indistinguishable in the log. The commit is titled "diagnose stream refusals", but refuseIdentity("watchActiveTabs") emits the identical line whether the stream was refused at bind (never had a credential) or died mid-stream at revocation. Those send an operator to different places. A "stage" to "bind" / "revocation" key in the structured map would close that for the cost of one parameter.

3. Class KDoc summary. :15 puts the revocation caveat above "Kernel-side bridge for ActiveTabsService", so the first paragraph — the rendered summary — is now a caveat rather than a description. Move it below the summary paragraph (it reads correctly as a body note).

4. The stream's "never reaches the provider" assertion doesn't hold for the stream. ActiveTabsServiceBridgeTest.kt:102 asserts provider.calls.isEmpty() after all seven refusals, but FakeActiveTabsProvider.activeTabs is a plain val property and records nothing — so the assertion proves the six unary paths stayed clear and says nothing about the watch path. The code is correct (the refusal precedes provider.activeTabs.collect), but the test's message overstates what it checks. Recording the access — e.g. override val activeTabs get() = _activeTabs.also { calls += "activeTabs" }, or an onStart on the exposed flow — would make the stream claim real.

5. Two small coverage gaps. refreshTabs and getFaviconCacheKey have only refusal coverage; neither has a positive-path assertion, so a gate that also broke the happy path for those two would not be caught. Both are one line each in the existing "authenticated caller" test. (I would not add a cross-identity isolation test here — PluginUIServiceBridgeTest already covers that shape and this bridge has no per-caller state.)

---

Not checked

I did not build or run the suite in this job — new-head CI is pending and a full :composeApp:desktopTest here would need the protoc/Compose toolchain warm. The test mirrors DownloadServiceBridgeTest (real server + ProcessIdentityInterceptor, supervisorScope so the failing async doesn't tear down the scope, withTimeout guard), and the revocation test looks deterministic rather than racy: the client received.receive() proves the first snapshot was already emitted server-side before revoke + setTabs, so StateFlow conflation can't swallow the second value.
· branch fix/active-tabs-caller-identity

[kshivang]

@Antriksh1984 Your seven RPC gates correctly protect private tab metadata and tab control, and all original work/author metadata is preserved in the clean dev replay. Current head is cbd2c3a483972c7a34b529ebbff07da3729e57f7.

The current Claude review found no blocking defect. I addressed its concrete follow-ups: select/create/close now record caller + RPC at INFO without storing URLs or titles; bind and revocation refusals have separate structured stages; the KDoc summary and synchronous capture comment are corrected; the fake now records activeTabs access, and refresh/favicon happy paths are asserted. Earlier reviewer work added unary/live-stream revocation coverage. These are reviewer repairs, excluded from your original contribution score and credited in the description.

Current-head local ActiveTabsServiceBridgeTest passes 5/5 with zero failures/errors/skips; ktlint and detekt pass through the mandatory build-lock wrapper. Original-head hosted XML was also verified on Linux/macOS/Windows. Current-head hosted code-quality/compatibility and lightweight checks pass, but Ubuntu/Windows builds are in progress and macOS is queued at this handoff. Prior-head green checks cannot validate this audit/test follow-up. This is a CI scheduling/validation blocker, not an author correctness defect.

Next steps: let the current-head required platform builds and summary complete; inspect any actual failure before attributing it; then recheck dev mergeability and the latest review against this small follow-up, and restore ready for serialized dev-only merge. No unresolved substantive source defect is asserted. The review suggestion to log a sanitized URL was satisfied more conservatively by logging attribution without any URL. Shared transport/helper inventory stays separate (#575). No UI delta or native visual test applies.

Per the bounded final-disposition instruction, converting to draft while required CI is incomplete. Original score: 9/10, excluding reviewer repairs. No checks bypassed and no unvalidated merge attempted.

[kshivang]

@claude Please review current head 0139bcf against current dev, including contributor updates since the previous handoff. Verify retained identity/revocation protections, regressions and tests. Verify retained mutation audit, refusal stages and current test coverage.

[claude]

Claude finished @kshivang's task in 3m 48s —— View job

---

Review of 0139bcf against dev

• Diff the two changed files against origin/dev (ActiveTabsServiceBridge.kt +74/-2, new ActiveTabsServiceBridgeTest.kt +257)
• Verify all seven RPC gates and synchronous identity capture
• Verify unary + live-stream revocation still terminates
• Verify the mutation audit and the new refusal stage key survived the replay
• Verify the five follow-ups from the previous handoff are all present
• Regression check: interceptor wiring, bridge registration, in-repo and out-of-repo clients
• Assess current test coverage

Verdict: identity and revocation protections are intact, the contributor updates since the last handoff all landed, and I found no blocking defect. Five non-blocking observations, the first of which I'd take because the stage key is currently mislabelling the majority of refusals.

---

Retained protections - verified

All seven RPCs gated. active_tabs.proto:15-33 declares WatchActiveTabs, RefreshTabs, SelectTab, GetTabUrl, GetFaviconCacheKey, CreateBrowserTab, CloseTab. Each of the six unary overrides calls authenticatedCallerOrRefuse as its first statement, before any provider touch (:69, :75, :82, :91, :100, :110); the stream is gated inside the flow at :36-38, before provider.activeTabs is read at :39. No method on the generated base is left open.

Synchronous capture is correct. :34 reads CURRENT_IDENTITY.get() outside the flow { } builder, so the Context key is read on the call's own thread rather than on whatever thread eventually collects. Byte-for-byte the shape LogServiceBridge:36, GitServiceBridge:32 and DownloadServiceBridge:40 use.

Key choice per RPC kind is right. Unary reads AUTHENTICATED_PROCESS_ID (a per-call snapshot, :126); the stream reads the captured CURRENT_IDENTITY closure and re-resolves on every emission (:40), so a revoked token stops a live stream. Exactly what SecretServiceBridge:222-225 prescribes.

Live-stream revocation actually terminates, and the != is sound. ProcessTokenRegistry.revoke (:79-81) removes the token→id mapping, so the captured closure re-resolves to null, null != caller holds, and refuseIdentity runs before emit - a revoked watcher cannot receive the snapshot that triggered the check. Because issue() invalidates the predecessor token (:52) and a token maps to exactly one id, the closure can only ever return the same id or null, so the != is a revocation check rather than an identity-change check and the "no current verified process identity" wording stays accurate.

Smart cast at :40 is sound - currentIdentity?.invoke() ?: refuseIdentity(...) returns Nothing on the null branch, so currentIdentity is non-null below.

Mutation audit retained and conservative. logMutation (:128-133) logs rpc + caller at INFO under LogCategory.AUTH, wired into selectTab/createBrowserTab/closeTab only, before the provider call ("requested" is the accurate verb). It carries no URL, no title and no tab id - a stricter answer than the sanitized-URL suggestion, and the right one for a bridge whose whole disclosure risk is URLs. Reads stay unaudited, matching SecretServiceBridge (getUserSecrets etc. are ungated for audit too).

Refusal stages retained. refuseIdentity(rpc, stage = "bind") with "revocation" passed at :41, both in the structured map.

Previous handoff's five follow-ups are all in. Audit line (1) ✅; separate bind/revocation stages (2) ✅; KDoc summary now leads with "Kernel-side bridge for ActiveTabsService" at :15 with the caveat demoted to :17 (3) ✅; the fake records stream access via a recording getter at ActiveTabsServiceBridgeTest.kt:201-205, so assertTrue(provider.calls.isEmpty()) at :102 is now a real claim about the watch path (4) ✅; refreshTabs and getFaviconCacheKey have positive assertions at :120-123 (5) ✅.

No in-repo regression. In KERNEL mode KernelBootstrap:422 always constructs the ProcessTokenRegistry and :486 hands it to BossIpcServer, which installs the interceptor on the ServerBuilder (BossIpcServer.kt:76) - so it covers services added after start() through the fallback registry (:75), which is how KernelBootstrap:979 adds this bridge. Children get BOSS_PROCESS_TOKEN from ProcessSpawner.kt:102 and attach it via ChildProcessBootstrap.kt:85. MONOLITH returns before registering anything. Refusal description matches all six siblings verbatim.

Test coverage. Five tests: all seven refused anonymously with the provider proven untouched; all seven exercised positively; unary revocation (refreshTabs); stream revocation, deterministic - received.receive() proves the first snapshot was emitted server-side before revoke + setTabs, so StateFlow conflation cannot swallow the second value, and supervisorScope keeps the failing async from tearing down the scope. Line lengths are within the 140 .editorconfig limit and @Suppress("TooManyFunctions") matches GitServiceBridge:27 / LogServiceBridge:31.

---

Observations

1. stage = "bind" is the default, so every unary refusal is logged with a stream-lifecycle word. authenticatedCallerOrRefuse (:126) calls refuseIdentity(rpc) and takes the default, so a refused getTabUrl or closeTab - six of the seven RPCs - logs stage=bind when nothing was being bound. The key exists precisely to send an operator to different places, and this sends five-sixths of refusals to the stream story. One argument at the unary call site (stage = "unary", or rename the default to "call" and pass "bind" explicitly at :38) closes it. Fix this →

2. ProcessIdentityInterceptor's KDoc still names three authenticated bridges. ProcessIdentityInterceptor.kt:19-24 says "thirteen other service bridges as of writing, none of them authenticated today" and names PluginUI, Secret and RoleManagement as "the three that currently make it". NO_IDENTITY now appears in seven bridges, and Download/Git/Log already falsified that list before this PR. It is the canonical place a reader learns which bridges check identity, and it is now wrong by a factor of two. Pre-existing drift rather than something this PR introduced, but this is the third PR to walk past it.

3. The seventh copy of the helper block, and it has now diverged. authenticatedCallerOrRefuse + refuseIdentity + NO_IDENTITY + the CURRENT_IDENTITY capture idiom are duplicated in all seven bridges. Until this PR they were identical; the stage parameter (and the slightly different "no *current* verified process identity" wording) means this one is no longer. That is an argument for #575 landing sooner rather than a defect here - but whoever extracts the shared helper needs to carry the stage key forward rather than flatten to the older six.

4. The audit line's content is convention-only. No test asserts that logMutation emits rpc + caller, nor that it omits URL and title. The PR description's "no URLs/titles" claim is upheld by review alone, and a later edit adding "url" to request.url would pass every test. SecretServiceBridgeTest doesn't pin its own logMutation either, so this is consistent - but a log-capture assertion is worth more on this bridge than on that one, since URL disclosure is the exact threat the gate exists for.

5. Out-of-repo clients fail silently, not loudly. ActiveTabsProviderProxy is defined in plugin-platform/plugin-api-ipc and constructed nowhere in this repo - the only consumers are in boss-microkernel-runtime. Its watchActiveTabs swallows every non-cancellation exception and retries forever with a 30s cap (:50-57), and the six unary methods catch (_: Exception) into null/false. So a runtime that builds its channel without ProcessTokenClientInterceptor presents as "the tab list is permanently empty, closeTab silently returns false", plus one host WARN per retry indefinitely, with nothing surfaced to the plugin. PERMISSION_DENIED is a permanent error that this retry loop treats as transient. Not a new class of breakage - already true of the Download, Git, Log, Secret and Role bridges - and not fixable in this PR, but worth recording against #575.

---

Not checked

I did not build or run the suite in this job: git fetch and Gradle are outside this runner's approved commands here, so the analysis is static. The author reports :composeApp:compileKotlinDesktop, compileTestKotlinDesktop, the five-test ActiveTabsServiceBridgeTest run, ktlint and detekt all passing on the current head; the hosted platform builds are the outstanding gate, and prior-head green checks do not validate this follow-up. I also could not diff 0139bcf against the previously reviewed cbd2c3a - the branch is a single replayed commit, so the contributor updates are folded in and I verified them by reading the current state rather than by diff.
· branch fix/active-tabs-caller-identity

[kshivang]

@Antriksh1984 Re-reviewed current head 0139bcff5cc66324fba0e8a466b0deef3f091611. Your seven identity gates and original tests are retained byte-for-byte alongside the previously credited reviewer audit/revocation repairs. Current Claude review finds no blocking defect. All ten current-head checks pass; downloaded Linux/macOS/Windows XML confirms five bridge tests on each, zero failures/errors/skips.

Review reconciliation: stage=bind is retained as initial identity binding, with rpc identifying unary versus watch calls; revocation distinguishes failure after a stream was admitted. Renaming the initial-stage label is optional diagnostic polish, not an authorization or regression defect. The suggested URL audit remains deliberately omitted: caller + RPC attribution protects privacy better. Additional log-capture assertions are optional test strengthening; source explicitly contains no URL/title/tab id in the audit map. The claimed three-bridge interceptor inventory is stale relative to refreshed dev (#651 is now confirmed merged); shared helper/runtime retry and version-skew work remains read-only #575 context. No claim of external runtime or native UI validation.

This is distinct from #634 and the merged Git/Log/Download/Secret bridges, not a duplicate. Original score remains 9/10 (impact 3, correctness 3, completeness/testing 1, maintainability 2), excluding reviewer repairs. Ready for the serialized dev-only merge after a fresh locked state/mergeability check.