fix(kernel): require verified caller identity for the Supabase proxy and Download bridges

[Antriksh1984]

Summary

Two more unauthenticated IPC bridges closed, following the same BossConsole#53 pattern as SecretServiceBridge/RoleManagementServiceBridge.

• SupabaseServiceBridge is a generic, unrestricted Postgrest proxy: it runs every call on the signed-in user's own RLS-scoped Supabase session. rpc() reaches the exact same create_secret/update_secret/share RPCs the Secret Manager UI calls (see AGENTS.md's "Decoding Supabase payloads" section), and select() can read any table RLS exposes to that user - this bridge is not scoped to one feature, it is the whole Postgrest surface. Neither select nor rpc checked caller identity: any local process reaching the kernel's IPC socket had the same reach as a legitimately installed plugin, attributable to nobody. This does not change what an authenticated caller may do (every in-process plugin already has identical unrestricted access via PluginContext.supabaseDataProvider - an existing, accepted design decision) - it only closes whether the caller is even a real, spawned BOSS process at all.

• DownloadServiceBridge.openFile/revealInFolder shell out to the OS's own "open with default application" command (open/start/xdg-open) for any path that exists on disk, with zero check that the path has anything to do with a download. Before this fix, an unauthenticated caller on the IPC bridge could make BOSS execute or open any file on disk via the OS default-handler mechanism - for an executable, start "" file.exe and a double-click are the same action. This bridge path provides no executable-consent step. This PR confines access to tracked downloads; it does not add executable consent for authenticated callers.

Changes

• Both bridges now require ProcessIdentityInterceptor.AUTHENTICATED_PROCESS_ID on every RPC, refusing with PERMISSION_DENIED and logging via BossLogger when absent - matching SecretServiceBridge's established authenticatedCallerOrRefuse shape.
• SupabaseServiceBridge additionally logs every call (caller, rpc, target) for audit purposes, since this proxy has no read-only concept - a select is as capable of exposing a secret as rpc("get_user_secrets", ...).
• DownloadServiceBridge.openFile/revealInFolder get a second, narrower guard: the requested path is canonicalized and compared against every currently-tracked DownloadItemData.destinationPath, refusing any path not in that set. This turns "open any file that exists" into "open a file BOSS itself downloaded" - the only thing either RPC's name actually promises.

Test plan

• New SupabaseServiceBridgeTest and DownloadServiceBridgeTest, both real gRPC-server-and-client tests (matching PluginUIServiceBridgeTest's pattern via ProcessIdentityInterceptor/ProcessTokenRegistry/ProcessTokenClientInterceptor):
  • An anonymous caller is refused PERMISSION_DENIED on every RPC, with the underlying provider asserted never invoked - a refusal that still reached the provider would defeat the point.
  • An authenticated caller's calls reach the provider with arguments intact.
  • DownloadServiceBridgeTest additionally covers an authenticated caller naming a path outside the tracked-downloads set (refused) and inside it (allowed).
• ./gradlew :composeApp:compileKotlinDesktop :composeApp:compileTestKotlinDesktop clean.
• ./gradlew ktlintCheck detekt clean.
• Full new-test suites green locally.

Attribution and current validation

Antriksh (@Antriksh1984) authored the identity gates, Supabase audit logging, tracked-download path restriction and original 15 wire tests. The clean dev replay retains that contribution as a3f41305e. Prior reviewer work (5efc93b90, e53e7e8d0, 358cf43e0) added per-emission revocation checks, synchronous context capture, canonical-path forwarding and blank-path regressions. Antriksh subsequently fixed the reviewer assertion formatting and KDoc at 283763122. These repairs are separately attributed and do not change the original contribution score.

Current head 2837631222dca7c3f956607900e82c76704a87f1: hosted Build & Test succeeds on Linux, macOS and Windows; quality and all other checks are green. Downloaded current-head Ubuntu test XML verifies 14 DownloadServiceBridge tests and 4 SupabaseServiceBridge tests, zero failures/errors/skips. Current-head Claude review reports no blocker: #550 (comment) . See the sweep review comment for the authoritative review link and disposition.

The prior draft validation handoff is superseded by this current-head evidence. Local rerun remains queued under the shared build lock, so no local pass is claimed. No app launch or live database deployment occurred. These host-side IPC changes have no visual UI delta.

Limitations: stream revocation takes effect before the next emission, not proactive connection teardown; the download list is a sampled snapshot; canonical-path checking does not prevent same-user filesystem races or add executable consent. Shared transport/helper consolidation remains separate #575 work.

[kshivang]

@claude Please review current head 5c54c59 against dev, focusing on caller identity, streaming revocation, tracked-download path checks and meaningful regression coverage. This is the first review request for this head.

[claude]

Claude finished @kshivang's task in 7m 25s —— View job

---

Review of 5c54c59

• Locate the head commit and scope the review
• Caller identity on both bridges
• Streaming revocation
• Tracked-download path confinement
• Regression coverage quality

The checkout is shallow, so git diff origin/dev...HEAD has no merge base (the 100-file PR diff is mostly main-ward drift). I reviewed 5c54c59 itself - the four files it touches - plus the call sites it depends on (ProcessIdentityInterceptor, ProcessTokenRegistry, KernelBootstrap, DownloadDataProviderImpl, FileSystemUtils, FluckEngine). I could not run Gradle in this environment, so nothing below is empirically verified; where that matters I say so.

Verdict: the identity gating is right in shape and the path confinement is a genuine narrowing worth having. Two things I'd want changed before merge (#1 and #2), because together they mean the one streaming RPC on this bridge is both the least protected call and the only one no test exercises in the direction that breaks users.

---

1. watchDownloads checks identity once, at collection time, and never revalidates

DownloadServiceBridge.kt:40-42 reads the identity inside the flow { } builder:

override fun watchDownloads(request: Empty): Flow<DownloadListResponse> =
    flow {
        authenticatedCallerOrRefuse("watchDownloads")
        provider.downloads.collect { … }
    }

Two separate problems, one fix.

Revocation. ProcessIdentityInterceptor publishes a second key for exactly this case, and its KDoc says why: "Revalidate at stream binding, since a call may wait across token revocation." CURRENT_IDENTITY is unused here. Concretely: KernelBootstrap.kt:174-180 drops registry handles only for filterNot { it.isAlive } ("preserving live children if termination failed or is pending") and then revokes the token of every child in the cohort, survivors included. A child that outlived forced termination therefore keeps a live channel and a live watchDownloads stream, gets PERMISSION_DENIED on every new unary call, and goes on receiving every download's url, fileName and absolute destinationPath indefinitely - which is also precisely the reconnaissance an openFile caller wants, since the guard in #3 is "name a path from this list".

Propagation. This is the only place in the repo that reads a gRPC Context.Key after the RPC method body has returned. PluginUIServiceBridge.kt:153-159 documents the opposite convention and explains it: "Captured synchronously, before anything below suspends… that Context is not automatically carried into a coroutine launched later on its own." Whether grpc-kotlin keeps the intercepted Context current through collection of the returned flow is an assumption this commit makes and no test checks. If it doesn't hold, every legitimate plugin's download stream is refused.

Both go away with the shape the UI bridge already uses - capture synchronously, fail closed on a null lambda (the interceptor may not be installed), and re-ask per emission:

override fun watchDownloads(request: Empty): Flow<DownloadListResponse> {
    val identity = ProcessIdentityInterceptor.CURRENT_IDENTITY.get()
    return flow {
        identity?.invoke() ?: refuse("watchDownloads")
        provider.downloads.collect { downloads ->
            identity.invoke() ?: refuse("watchDownloads")   // revocation mid-stream
            emit(…)
        }
    }
}

Fix this →

2. Nothing tests an authenticated watchDownloads

DownloadServiceBridgeTest covers the anonymous refusal (:136) but never collects the stream as an authenticated caller. That refusal test passes whether the gate works or whether the gate rejects everyone - so the suite is green in both worlds, including the one where #1's propagation assumption is false and out-of-process plugins lose their download stream entirely. PluginUIServiceBridgeTest already has the precedent for testing identity on a streaming RPC (:566, process A cannot impersonate process B through StreamUI). One first() on an authenticated stub settles it.

3. The canonical path is checked, the raw path is used

trackedDownloadPathOrRefuse canonicalizes both sides for the comparison (:129-134) and then returns requestedPath unchanged (:143), which FileSystemUtils.openFile turns into Runtime.exec(arrayOf("open", file.absolutePath)). So the resolution that authorized the call is not the resolution that executes it: /tmp/link -> ~/Downloads/report.pdf passes the guard, and swapping the symlink between the check and the exec opens whatever it now points at. The window is small and the caller must already hold a valid process token, so this is hardening rather than a hole - but return requestedCanonical.path closes it for free and makes the code match its own KDoc's "canonical-path compared" claim.

4. The tracked set comes from a 150 ms-sampled, EDT-maintained snapshot

provider.downloads.value is not the live download table. DownloadManager.kt:31 is .sample(150.milliseconds), and DownloadDataProviderImpl.kt:74-78 re-collects that on Dispatchers.Main into its own StateFlow. The same file's removeDownload explicitly refuses to use it, for this reason (:89-91): "Read the live map, not the throttled downloads flow, which lags by up to a sampling window and can still be missing a download that just started."

Consequence: revealInFolder for a download that just started can come back PERMISSION_DENIED, and a busy EDT widens that window arbitrarily. A plugin that only opens what it saw on its own watchDownloads stream is consistent (same source), so this is survivable - but the refusal is indistinguishable from a real one, and "the security check reads the throttled UI snapshot" is worth either a line of KDoc or a provider-level query against the live map.

5. A blank path collapses to the working directory

File("").canonicalFile is the process CWD, on both sides of the comparison. A tracked item whose destinationPath is blank, plus a request of "" or ".", satisfies the guard and the host opens or reveals its own working directory. savePath makes a blank destinationPath unlikely in practice, but the guard should not depend on that: reject a blank request.path up front and skip blank destinationPath entries in the any { }.

6. The prose claims a protection this tree does not have

The commit message and the class KDoc both say this closes a "complete bypass of #484's executable-download-warning consent dialog". There is no such dialog. FluckEngine.kt:3264-3269:

if (downloadSettings.warnForExecutables && FileNameSanitizer.isExecutableFile(sanitizedFileName)) {
    // TODO: Show user warning dialog (for now, just proceed)
}

And after this change an authenticated caller can still openFile a tracked .exe with no consent step, because a tracked download is whatever BOSS downloaded - including something a caller that can drive a browser tab caused to be downloaded. The confinement is still right ("open a file BOSS downloaded" is the only thing the RPC's name promises), but given how much this repo leans on its own comments being true, I'd reword to claim the narrowing and not the consent gate.

7. Dangling KDoc cross-references, and a stale inventory

Both new classes say "the same requirement and helper shape as [SecretServiceBridge] - see that class's KDoc for the full rationale." On this branch SecretServiceBridge.kt has no KDoc, no helper, and no identity check - it is still fully anonymous, and its own createSecret/updateSecret reach the same RPCs this PR gates on the Supabase side. A reader following that link lands on the opposite of what it promises. Either the sibling #53 fix needs to land first (or be named as "PR #NNN" instead of a KDoc link), or the rationale should live in one of these two classes.

Relatedly, modules/boss-ipc/.../ProcessIdentityInterceptor.kt:18-22 still reads "fourteen other service bridges as of writing, none of them authenticated today" and "PluginUIServiceBridge is the one that currently makes it" - wrong by two after this commit, and that paragraph is a security inventory rather than colour.

8. Three copies of authenticatedCallerOrRefuse, already diverging

PluginUIServiceBridge:127, SupabaseServiceBridge:127, DownloadServiceBridge:152 - same name, same NO_IDENTITY string, and already three different behaviours (CURRENT_IDENTITY vs AUTHENTICATED_PROCESS_ID; LogCategory.UI vs AUTH). That divergence is how finding #1 happened. If #53 is going to reach the remaining bridges, this wants to be one helper in boss-ipc next to the interceptor, so a revocation-aware version lands everywhere at once instead of per-bridge.

Coverage gaps worth closing

Gap	Why it matters
authenticated watchDownloads	finding #2 - the suite is green either way today
revoked token → refused	ProcessTokenRegistry.revoke then call; the interceptor's "stale token after a respawn" case is untested at the bridge level, on both bridges
symlink and .. paths	the KDoc explicitly claims "a symlink or a .. cannot walk outside it"; nothing pins it, and #3 shows the claim is only half-true
revealInFolder tracked happy path	openFile has one (:193), revealInFolder only has refusals
refusal descriptions	no test asserts NO_IDENTITY vs NOT_A_TRACKED_DOWNLOAD, so the two guards are indistinguishable to the suite
select filters / range	"arguments intact" (:119) asserts table and columns only. Worth noting the pre-existing rangeFrom >= 0 && rangeTo > 0 at SupabaseServiceBridge.kt:62 silently drops a 0..0 range - not introduced here, still unpinned

Add these tests →

What's right

• Wire-level tests through a real ProcessIdentityInterceptor rather than direct Kotlin calls - the only way this gate is actually under test - and the provider.calls.isEmpty() assertion on every refusal, which is the assertion that would catch a check placed after the side effect.
• Fail-closed on a null identity, with the refusal logged before the throw.
• Identity is checked before the path guard in openFile/revealInFolder, so an anonymous caller learns nothing about what is tracked.
• Canonicalization does close the escape direction (.., or a symlink pointing outside the tracked set), which is the half that matters most.
• SupabaseServiceBridge.logCall logs the table/function name and never parametersJson - correct under AGENTS.md's "Decoding Supabase payloads" rule, since a create_secret call carries the password in its parameters. One thought: it is INFO on every call of a proxy a plugin may poll, so expect volume.
• The class KDocs are genuinely good on why this bridge is not a narrower grant than the secret bridge, and on not relitigating in-process plugins' equivalent access.
  · branch fix/ipc-bridge-caller-identity-supabase-download

[kshivang]

Bounded response to the completed review of original 5c54c59: current pushed head is 42a09fd681faa232cf9ada13690048052d25ad54, now based on dev with only the authored service change and reviewer repairs.

1. Stream identity is captured synchronously before returning the flow, fails closed, and is rechecked before every snapshot. The revocation test first receives a legitimate snapshot, then revokes the token and asserts the next snapshot cannot arrive. This covers findings 1 and 2 without relying on anonymous refusal alone.
2. The provider receives the canonical path that passed the check. A real-file alias test covers both open and reveal (finding 3).
3. The sampled snapshot limitation is documented: a new item remains refused until it appears in the provider snapshot (finding 4). This intentionally fails closed and does not change the pinned provider API.
4. Blank request paths and blank tracked destinations are rejected and pinned, including the working-directory alias (finding 5).
5. KDoc and description now state the actual narrowing and do not claim executable-consent enforcement; the branch refers explicitly to PR fix(kernel): require a verified caller identity for the secrets and RBAC IPC bridges #505 rather than a not-yet-present SecretService KDoc (findings 6/7).

I did not expand this into the shared authentication-helper refactor (finding 8): #575 already supplies a distinct transport/ownership redesign. Repeating that implementation here would create competing security contracts. The old interceptor comment inventory and pre-existing Supabase 0..0 range behavior are recorded as follow-up context, not fixes this branch claims to deliver. Additional optional permutations are useful but do not replace the focused positive/refusal regression cases above. Canonical paths still do not guarantee filesystem race immunity against same-user mutation.

Original gates and tests remain credited to Antriksh (@Antriksh1984); the repairs above are reviewer work. Original-head CI passed; current-head CI and the shared-lock focused tests plus ktlint/detekt are pending. No new test pass or merge approval is claimed.

@claude Please perform one final targeted verification of these bounded repairs at 42a09fd68. This is the changed-head verification after the completed original review, not a repeat request on the old head. Any remaining substantive blocker will be handed back under the user's bounded-review policy.

[kshivang]

@Antriksh1984 Your Supabase/download caller gates, audit logging and tracked-download restriction close useful access gaps. The original 15 wire tests and original commit remain credited to you; the rebased commit preserves your author metadata. Reviewer repairs are separately recorded in the PR description and review response.

Final pushed head is 8a8718a843b085f320937d3245f41c7ee9460b9e. The final delta from 42a09fd68 is test expression wrapping only. The bounded repair addresses the completed review's stream revocation/positive-stream coverage requirements, canonical-path forwarding, blank paths and misleading consent statements. Three added tests bring the focused request to 18. No further substantive correctness defect has been established on this repaired code, but it has not completed validation.

The remaining blocker is infrastructure/review completion: current-head CI is queued, local focused tests plus ktlintCheck/detekt have not acquired the shared build lock, and the single final changed-head Claude request has not completed. Original-head green CI does not validate reviewer changes. This is not evidence that your original implementation failed CI, and reviewer work must not be assigned to you as original work.

Next steps: obtain passing current-head DownloadServiceBridgeTest/SupabaseServiceBridgeTest and quality checks, inspect the final requested review, address any substantiated findings, and recheck dev integration before undrafting. The separate #505 vault/RBAC implementation is now merged into dev; this PR remains distinct. The sampled provider snapshot and same-user filesystem race limitations are documented, and broad helper/version/range refactors are not silently included.

Per the user's bounded-disposition policy I am converting this open PR to draft after the handoff rather than bypassing pending checks or spending hours polling. Repairs are pushed and preserved; no merge of this unvalidated head occurred.

[claude]

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

---

Verification of the bounded repairs

• Establish the real head and the diff against origin/dev
• Finding 1/2 - synchronous capture, fail-closed, per-snapshot recheck, revocation test
• Finding 3 - canonical path forwarded to the provider
• Finding 4 - sampled-snapshot limitation documented
• Finding 5 - blank path / blank destination rejected
• Findings 6/7 - prose no longer claims executable consent; no dangling "see that class's KDoc"
• Regression coverage quality

Head note: the pushed tip here is 8a8718a, one commit past the 42a09fd you named; it is whitespace-only (style(tests): align download regression expression wrapping, two val x = assertFailsWith {…} / async {…} rewraps). I verified 8a8718a. origin/dev...HEAD still has no merge base in this shallow checkout, so I diffed e1926cc..HEAD (the dev commit the branch is based on); that yields exactly the four claimed files, +727/-12, with no main-ward drift. Good.

Verdict: all five claimed repairs are present and correct in the code. No remaining substantive blocker from me. One prose defect was introduced by the finding-6 reword and is worth a one-line fix; everything else below is a recorded non-blocker.

---

Repair-by-repair

Finding	Claim	Verified
1 - sync capture	CURRENT_IDENTITY.get() in the method body, before the flow is returned	✅ DownloadServiceBridge.kt:41, matching PluginUIServiceBridge.kt:159's convention and its stated reason
1 - fail closed	null lambda or null identity refuses	✅ :43-45. A missing interceptor and an unknown token both land on PERMISSION_DENIED/NO_IDENTITY
1 - revocation	re-asked per snapshot	✅ :48 re-invokes and compares against the identity captured at bind (!= caller, not merely a null test - a rotated token, which ProcessTokenRegistry.issue invalidates for the same processId, also refuses)
2 - positive + revoked stream	one legitimate snapshot, then revoke, then no more	✅ DownloadServiceBridgeTest.kt:165-187. Deterministic, not timing-dependent: received.receive() proves the server-side collect bound and delivered before revoke, and provider.downloads is a MutableStateFlow so setDownloads guarantees the next emission attempt. tryReceive().isFailure is the assertion that actually distinguishes "refused" from "refused after leaking one more snapshot"
3 - canonical forwarded	provider gets the path that passed the check	✅ :157 checkNotNull(requestedCanonical).path; pinned for both RPCs at :190-200 via a ./name alias, and :250 updated to expect tracked.canonicalPath. This also makes the macOS /var → /private/var case explicit rather than incidental
4 - snapshot lag	documented, fails closed	✅ :131-133. Accurate against DownloadDataProviderImpl.kt:72-78 (Main-dispatched re-collect of a sample(150ms) flow) and consistent with that file's own removeDownload comment. Not widened to a live-map query - right call, since DownloadDataProvider is the pinned plugin API
5 - blank collapse	""/. cannot authorize the CWD	✅ two independent guards: :140 takeIf { it.isNotBlank() } on the request, :146 destinationPath.isNotBlank() on each tracked item. :203-215 pins both together (blank tracked destination + "" and "." requested) and asserts the refusal description, so this case cannot pass by hitting the identity gate instead
6/7 - prose	no consent-dialog claim, no dangling KDoc link	✅ the "complete bypass of #484's consent dialog" claim is gone; the sibling is cited as PR #505 (DownloadServiceBridge.kt:20, SupabaseServiceBridge.kt:25, :124). The surviving [SecretServiceBridge] reference at SupabaseServiceBridge.kt:18 is a grant-scope comparison, not a promise about that class's KDoc - fine as written

Two things I checked that you did not claim, because they would have been silent regressions:

• The interceptor is actually on the server these two bridges land on. registerPluginServices adds them post-start, so they go into BossIpcServer's fallbackHandlerRegistry - but builder.intercept(...) is a server-wide interceptor and applies to fallback-resolved methods too (BossIpcServer.kt:70-72), and KernelBootstrap.kt:407/471 always constructs the registry in non-MONOLITH mode. So a legitimate child is not refused.
• The client attaches the credential channel-wide, not per-service. ChildProcessBootstrap.kt:85 intercepts the whole kernel channel, so SupabaseDataProviderProxy/DownloadDataProviderProxy inherit it with no change. A runtime old enough to lack BOSS_PROCESS_TOKEN would already be failing PluginUIService.StreamUI, so this commit does not add a new compatibility cliff.

One new nit, introduced by the finding-6 reword

DownloadServiceBridge.kt:26-28 is now ungrammatical - the sentence lost its subject in the edit:

for an executable, start "" file.exe and a double-click are the same action, and this is the path does not provide an executable-consent dialog.

Reads as "and this path does not provide an executable-consent dialog". Given how much this repo leans on its own comments being the record, it is worth the one-line fix rather than leaving a mangled sentence in the paragraph that exists specifically to state the limitation accurately. Fix this →

Recorded non-blockers (no action asked)

• Revocation takes effect at the next emission, not at revoke time. A revoked child's stream stays open and idle until provider.downloads changes. No data crosses in that window, so the disclosure is closed; the stream just is not proactively torn down. Fine as a property, and arguably the only thing achievable without a revocation signal to subscribe to.
• The child reconnects on refusal. DownloadDataProviderProxy.kt:35-52 retries watchDownloads forever with 1s→30s backoff and swallows the exception, so a revoked-but-surviving child polls the kernel at ≤30s intervals and keeps its last snapshot in memory. Bounded and pre-existing; worth knowing when reading the logs after a failed termination.
• [FileSystemUtils.openFile] at :22 is not resolvable from this file - FileSystemUtils lives in ai.rever.boss.platform and is not imported. Harmless (Dokka is not gated), and the claim it makes is accurate (DownloadDataProviderImpl.kt:151-157 delegates straight to it).
• Stale ProcessIdentityInterceptor KDoc inventory (:18-22, still "fourteen other service bridges… none of them authenticated today" / "PluginUIServiceBridge is the one that currently makes it") and the rangeFrom >= 0 && rangeTo > 0 drop of a 0..0 range: accepted as your recorded follow-up context. The interceptor paragraph is a two-sentence edit whenever someone is next in that file.
• Finding 8 (one shared authenticatedCallerOrRefuse in boss-ipc): deferring to fix(ipc): authenticate process calls and survive a dead repair client #575 rather than shipping a fourth divergent copy-then-refactor is a reasonable call. The divergence that caused finding 1 is now gone in behaviour, which was the substantive part.

What I did not verify

I could not run Gradle in this environment (the invocations are blocked here), so compilation, ktlintCheck, detekt and the test suites are unverified by me - your pending CI on this head remains the only evidence. Statically I did confirm no line in the four files exceeds the 140-character .editorconfig limit, that config/detekt/detekt.yml exempts **/desktopTest/** from TooManyFunctions/LongParameterList, that watchDownloads stays under detekt's default 60-line LongMethod threshold and at (not over) the default ThrowsCount max of 2, and that no U+2014 was added. I am not claiming a test pass and not approving the merge.
· branch fix/ipc-bridge-caller-identity-supabase-download

[Antriksh1984]

Pushed fe10bee9 fixing the one remaining code-quality blocker: detekt's default MaxLineLength (120, narrower than ktlint's 140-char .editorconfig limit) was rejecting a wrapped assertEquals in DownloadServiceBridgeTest, and finished the finding-6 KDoc reword the bot review flagged as ungrammatical. No behavior change. Current head fe10bee9 is 9/9 green and mergeStateStatus: CLEAN. Restoring ready-for-review.

[Antriksh1984]

All five open PRs went CONFLICTING/DIRTY simultaneously after dev picked up #450 (the main.kt startup-phase refactor) plus several other merges - same tangled-history symptom as #484 hit earlier: git log origin/dev..HEAD on each branch showed 100+ unrelated commits, meaning a stray old merge or an out-of-date rebase point, not a real content conflict with the actual feature diff.

Fixed by rebuilding this branch: identified the commits genuinely unique to this PR, cherry-picked them in order onto a fresh origin/dev, and dropped the rest. Content is unchanged - same diff, same review history applies. Verified clean compile, detekt and the relevant test suites before pushing. mergeable is back to MERGEABLE with no conflicts.

[kshivang]

@claude Please review current head 2837631222dca7c3f956607900e82c76704a87f1 against dev for correctness, security and regressions. Please account for your previous review and the contributor’s subsequent formatting-only fix and clean replay; do not repeat resolved findings. Current-head hosted CI is green. This is the single review request for this sweep.

[claude]

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

---

Review of 2837631 against origin/dev

• Establish the diff against origin/dev after the replay
• Confirm the previously verified repairs survived the replay
• Check the formatting-only / KDoc delta
• Look for anything new: correctness, security, regressions

The replay is clean. For the first time in this PR's history there is a real merge base (f91633d), git diff origin/dev...HEAD yields exactly the four claimed files at +729/-12, and git log HEAD..origin/dev is 0 - the branch contains dev, no main-ward drift, no stray old merge. The tangled-history symptom from the earlier comment is gone.

Verdict: no blocker. Two cosmetic nits below, neither worth holding a merge for. I did not re-litigate findings 1-7 from the earlier reviews; I re-verified each repair against the current tree from first principles rather than diffing against the replayed-away head (8a8718a/fe10bee9 are not reachable in this shallow clone), which is the stronger check anyway.

---

The delta since my last verified head

2837631 is the only commit I had not seen, and it is exactly what the commit message claims:

• DownloadServiceBridge.kt:27-28 - the mangled sentence is gone, now "for an executable, start "" file.exe and a double-click are the same action, and this path provides no executable-consent step." Reads correctly and still states the limitation rather than overclaiming. Finding 6's reword is finished.
• DownloadServiceBridgeTest.kt:199-202 - the 128-char assertEquals is wrapped. Nothing else in the test changed.

No behaviour change, confirmed line by line.

Re-verification of the repaired code on this base

Property	Status on 2837631
Stream identity captured synchronously	✅ DownloadServiceBridge.kt:40, before the flow is returned
Fails closed on a missing interceptor	✅ :42-44 - null lambda and null identity both land on PERMISSION_DENIED
Revocation and token rotation end the stream	✅ :47 re-invokes CURRENT_IDENTITY per snapshot and compares != caller. Since CURRENT_IDENTITY is { registry.identityFor(token) }, this catches both revoke and the re-issue a respawn performs (ProcessTokenRegistry.kt:52)
Canonical path forwarded, not the raw one	✅ :156 checkNotNull(requestedCanonical).path, pinned for both RPCs at DownloadServiceBridgeTest.kt:189-203
Blank path / blank destination cannot authorize the CWD	✅ two guards (:139, :145), pinned at :205-218 with the refusal description asserted, so the case cannot silently pass through the identity gate instead
Sampled-snapshot limitation documented	✅ :130-132
Prose no longer claims executable consent	✅ see above

Two things I checked against the new dev base rather than taking forward, because the #450/#505 merges could have invalidated them:

• The interceptor still reaches these bridges. BossIpcServer.kt:76 installs ProcessIdentityInterceptor only when tokenRegistry != null, and the interceptor is server-wide so it covers fallbackHandlerRegistry-resolved late services (:75). KernelBootstrap.kt:422 unconditionally constructs the registry in KERNEL mode and :486 passes it, while registerPluginServices returns early in MONOLITH. So a legitimate child is not refused, and there is no path where these bridges land on an un-intercepted server.
• ProcessTokenRegistry makes the revocation test deterministic, not timing-lucky. revoke and identityFor are both @Synchronized, so the test's revoke → setDownloads ordering has a real happens-before with the server-side re-check. Combined with received.receive() proving the server collector had already bound, and MutableStateFlow guaranteeing the next emission attempt, tryReceive().isFailure at :180 is a sound assertion rather than a race.

Nit 1 - a dead detekt baseline entry, left behind by this PR

composeApp/detekt-baseline.xml:672 suppresses:

MaxLineLength:DownloadServiceBridge.kt$DownloadServiceBridge$override suspend fun pauseDownload(request: DownloadIdRequest): OperationResult

On dev that method was a single 140-ish-char expression body; this PR turns it into a block body (DownloadServiceBridge.kt:86-89), so nothing matches that ID any more. detekt does not fail on unused baseline entries, which is why CI is green - it is just dead weight in a file that is easier to keep honest than to clean later. (The three sibling entries at :669-671 for the DownloadStatusData when arms are still live and still needed.)

Fix this →

Nit 2 - the one bridge with a streaming RPC is the one whose helper KDoc omits the streaming caveat

Both siblings that landed with #505 warn on the helper itself:

SecretServiceBridge.kt:222 / RoleManagementServiceBridge.kt:192 - "Unary RPCs only: AUTHENTICATED_PROCESS_ID is a per-call snapshot… a streaming RPC must capture CURRENT_IDENTITY instead."

DownloadServiceBridge.kt:159-164 does not, and it is the only one of the three where a streaming RPC sits in the same class as the unary helper - so it is the one place where someone adding a second stream could reach for authenticatedCallerOrRefuse and get exactly the bug finding 1 was. One sentence, copied from the sibling.

Fix this →

Changed context since the last review, recorded not asked

• Finding 7 is resolved by dev, not by this branch. fix(kernel): require a verified caller identity for the secrets and RBAC IPC bridges #505 landed, so SecretServiceBridge now has a real KDoc and a real helper - the [SecretServiceBridge] grant-scope reference at SupabaseServiceBridge.kt:18 no longer points at the opposite of what it promises.
• Finding 8's duplication count went 3 → 5. authenticatedCallerOrRefuse now exists in PluginUI, Secret, RoleManagement (from dev) plus the two here, all sharing the same NO_IDENTITY string. Deferring the shared helper to fix(ipc): authenticate process calls and survive a dead repair client #575 remains the right call - notably, fix(kernel): require a verified caller identity for the secrets and RBAC IPC bridges #505 also inlined its own gRPC harness in SecretServiceBridgeTest, so this PR matches the house pattern rather than adding a new one, and there is no shared fixture on dev it should have reused.
• ProcessIdentityInterceptor's KDoc inventory is wrong in a new way. :17 still says "thirteen other service bridges as of writing, none of them authenticated today" and :23 names PluginUI/Secret/RoleManagement as "the three that currently make it" - after this PR it is five of fifteen. Still a two-sentence edit for whoever is next in that file; it is a security inventory, so it is worth someone actually doing rather than deferring indefinitely.
• Previously recorded and unchanged: revocation takes effect at the next emission rather than proactively tearing the stream down; DownloadDataProviderProxy reconnects on refusal with ≤30s backoff; [FileSystemUtils.openFile] at :22 is not import-resolvable for Dokka; the rangeFrom >= 0 && rangeTo > 0 drop of a 0..0 range is pre-existing; canonical-path checking is not filesystem-race immunity against another same-user process.

What I did not verify

Gradle invocations are blocked in this environment, so compilation, ktlintCheck, detekt and the test suites are unverified by me - your green hosted CI on this head is the evidence for those. Statically I did confirm: no U+2014 in the four files; nothing over 120 characters except the three DownloadStatusData lines that are already baselined; and the registration/interceptor/credential wiring described above. I am not approving the merge.
· branch fix/ipc-bridge-caller-identity-supabase-download

[kshivang]

@Antriksh1984 Your clean replay and final formatting/KDoc fix are verified. The current Claude review (#550 (comment)) reports no substantive blocker; all earlier security findings remain resolved. I independently checked the current diff, interceptor/channel wiring, main/dev equivalence in the affected paths and actual CI XML (18 passing focused tests, no skips). All current-head hosted checks are green.

The two cosmetic suggestions are recorded as nonblocking follow-ups: the obsolete MaxLineLength baseline ID no longer matches any finding and therefore suppresses no current violation; the unary helper is only called by unary RPCs, while watchDownloads already captures CURRENT_IDENTITY synchronously and has a real revocation regression. Neither changes current correctness or requires reopening the validated implementation. Broader helper/inventory consolidation belongs with the separate #575 transport work. Emission-time revocation, sampled-path lookup and same-user filesystem races remain explicit limitations.

Your original gates/audit/path restriction and tests, prior reviewer repairs, and your final follow-up are separately credited in the description. This is suitable for dev integration, subject to the serialized final state/check/mergeability recheck. No main merge, app launch or database deployment.