fix(kernel): require verified caller identity for the Git bridge

[Antriksh1984]

Summary

GitServiceBridge had no caller-identity check on any of its sixteen RPCs. Any process able to open a connection to the kernel IPC server - not only the plugins the host itself loaded - could read the open project's full commit history and working-tree status, and could discardChanges (destroying uncommitted work), checkout, cherryPick or revert against the project's real git repository with no credential at all.

• Gate every RPC behind authenticatedCallerOrRefuse(), the same PERMISSION_DENIED-on-no-verified-identity pattern PluginUIServiceBridge established for BossConsole#53.
• The four watch streams (WatchFileStatus, WatchCommitLog, WatchIsGitRepo, WatchIsLoading) capture identity synchronously before returning the flow and re-check 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 (DownloadServiceBridge/SupabaseServiceBridge).

Test plan

• New GitServiceBridgeTest 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 file status/commit log and read the current project path; an authenticated caller can stage, checkout and discard changes normally.
• ./gradlew :composeApp:compileKotlinDesktop :composeApp:compileTestKotlinDesktop
• ./gradlew :composeApp:desktopTest --tests "ai.rever.boss.kernel.services.GitServiceBridgeTest"
• ./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. The prior reviewer revocation tests, refusal diagnostics and documentation repairs are retained unchanged; the six RunConfig / five Git tests passed on all three supported OSes at the previous repair heads. Current replayed head passes all ten hosted checks. Focused local bridge tests and detekt pass; Log also passes local ktlint. Current Claude review reports no blocking correctness/security defect. Idle streams are not proactively disconnected. Shared transport/helper work (#575) remains separate. No UI was changed or visually exercised.

Compatibility: a runtime missing the process credential is refused, consistent with #505/#550. Existing proxy watch loops retry with backoff; this can produce recurring AUTH warnings and an empty panel for an old or misconfigured runtime. Identity establishes a caller, not per-operation capabilities.

[kshivang]

@claude Please review current head 099511d0a1158d54f7b5675194d6d4b540f4ab44 against dev for correctness, security and regressions. Please check every RPC identity gate, stream revocation/context capture, provider compatibility and regression coverage. Current-head hosted CI is green. This is the single review request for this sweep.

[claude]

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

---

Review of 099511d vs origin/dev

• Gather context (diff vs origin/dev, bridge + test + interceptor + proxy)
• Check every RPC identity gate
• Check stream revocation / context capture
• Check provider compatibility
• Check regression coverage

Verdict: no blocking correctness or security defect. The gate is complete and fails closed. Six findings below, all non-blocking; the two I'd actually act on before merge are the missing revocation test and the unlogged stream refusals, because together they make a misconfigured runtime undiagnosable.

---

What I verified

Every RPC is gated. git.proto declares 16 RPCs (the description says fourteen); all 16 have an override and all 16 refuse. 12 unary via authenticatedCallerOrRefuse (GitServiceBridge.kt:123-183), 4 streams via the inline guard (:30, :60, :92, :107). Nothing falls through to provider before the check, and no proto RPC is left to the generated UNIMPLEMENTED default.

The unary/stream split is the right one. SecretServiceBridge.kt:222-225 explicitly prescribes it - AUTHENTICATED_PROCESS_ID is a per-call snapshot, a streaming RPC must read CURRENT_IDENTITY - and this is the first bridge to actually need it. The capture is synchronous, before the flow {} body can run on another thread, matching PluginUIServiceBridge.kt:153-159.

The capture works in practice, not just on paper. grpc-kotlin invoking the server-streaming impl inside the intercepted Context is load-bearing and non-obvious; the positive test (GitServiceBridgeTest.kt:117-142) runs over a real server and would fail with PERMISSION_DENIED if the Context weren't current, so that's pinned rather than assumed.

Revocation semantics check out. ProcessTokenRegistry.issue invalidates the previous token for the same process id, so a respawn's stale token resolves to null, != caller, and the old stream dies. Comparing the resolved identity rather than just non-null is also correct - a token reissued to a different id is refused too.

Fails closed both ways: no interceptor installed → CURRENT_IDENTITY.get() is null → refuse; no token → AUTHENTICATED_PROCESS_ID unset → refuse.

---

Findings

1. Stream refusals are silent on both sides (observability, medium).
GitServiceBridge.kt:35, 38 (and :65/68, :97/100, :112/115) throw with no logger.warn, while the unary helper at :194 logs one. The client half swallows it: GitDataProviderProxy.kt:61, 80, 99, 118 are catch (_: Exception) with exponential backoff to 30s, looping forever. So a runtime that connects without a credential shows an empty git panel, retried four times a minute, with no line in the host log and none in the plugin log. Routing the stream refusal through a logging helper costs nothing and is the difference between a five-minute diagnosis and an afternoon.
Fix this →

2. No regression coverage for revocation at all (test coverage, medium).
The PR's distinguishing claim over its siblings is "a revoked token stops a live stream rather than only new calls" - and nothing tests it, unary or streaming. Both sibling bridges already have the unary case: SecretServiceBridgeTest.kt:238 and RoleManagementServiceBridgeTest.kt:230, same test name in both. Two tests worth adding: unary after tokenRegistry.revoke(CALLER), and a live watchFileStatus that receives one value, gets revoked, and fails PERMISSION_DENIED on the emission after the next setFileStatus. The second one also documents finding 3's semantics in executable form.
Fix this →

3. Revocation is enforced on the next emission, not on revocation (worth a KDoc line, low).
provider.fileStatus is a StateFlow: if the working tree never changes again, a revoked caller's stream stays open indefinitely. Nothing leaks - the check precedes emit, so no post-revocation data reaches the wire - but the connection and the subscription survive, and "a revoked token stops a live stream" reads stronger than what the code does. One sentence in the class KDoc saying the check is emission-driven would stop the next reader assuming revocation reaps streams.

4. Four copies of the guard (maintainability, low).
:30-120 is the same eight lines four times. A fifth watch RPC can ship ungated and nothing catches it. One private fun <T, R> identityGuarded(upstream: Flow<T>, transform: (T) -> R): Flow<R> collapses all four and gives finding 1 a single place to log from - the same "one definition so the two cannot drift" shape this repo's AGENTS.md keeps arguing for.

5. ProcessIdentityInterceptor's KDoc is now stale (docs, low).
ProcessIdentityInterceptor.kt:17-24 still says "thirteen other service bridges ... none of them authenticated today" and names PluginUIServiceBridge, SecretServiceBridge and RoleManagementServiceBridge as "the three that currently make it". Git is the fourth. That KDoc is the canonical list of who checks identity, so it's the one place drift actually costs something. (SecretServiceBridge.kt:26 carries a similar count.)

6. The fake's "never reaches the repository" assertion is weaker than its message (test, low).
GitServiceBridgeTest.kt:248 - getCurrentProjectPath doesn't append to calls, and the watch flows are plain StateFlows that record no subscription. So assertTrue(provider.calls.isEmpty()) at :114 cannot observe a leak through those five RPCs. The assertRefused wrapper still catches an ungated RPC, so this is belt-and-braces only - but recording the read would make the assertion mean what it says.

---

Provider compatibility

Host side is clean. BossIpcServer is constructed with a registry only at KernelBootstrap.kt:486, which is the only construction that ever gets a GitServiceBridge (:977); registerServiceBridges returns early in MONOLITH, so there is no in-process regression. Children spawned through ProcessSpawner get BOSS_PROCESS_TOKEN, and ChildProcessBootstrap.kt:85 attaches the client interceptor.

The one real exposure is version skew, and it's inherited from #505 rather than introduced here: ChildProcessBootstrap.processToken is optional by design (:56-64 - "a process started outside ProcessSpawner ... simply connects without one"), so a runtime built against a pre-#505 boss-ipc - the out-of-repo boss-microkernel-runtime - now loses git entirely. Git is a far more visible surface than the vault or the RBAC tables, so this one is more likely to be noticed as "the git panel is broken" than the previous two gates were. Worth a line in the PR description or release notes; finding 1 is what would make it diagnosable when it happens.

Not re-run here

I read the code rather than re-running the suite; taking the stated green CI for ./gradlew :composeApp:desktopTest --tests "...GitServiceBridgeTest" and :composeApp:detekt at face value. Nothing in the findings above depends on that.
• branch fix/git-bridge-caller-identity

[kshivang]

@Antriksh1984 Thank you for the concrete IPC security work. Current repair head: 345ee24d7d5a99e19272a98554da3980ca701ee6.

Your implementation gates all 16 Git RPCs, including destructive operations, with the correct synchronous stream context capture. Reviewer commit adds unary revocation and deterministic live watchFileStatus revocation regressions, routes every stream refusal through a structured AUTH log, documents emission-time revocation, and makes the fake provider record getCurrentProjectPath reads. The description's RPC count is corrected from 14 to 16.

No unresolved substantive production correctness defect is established after these repairs. The remaining blocker is validation of the new reviewer head. Next steps: obtain a green GitServiceBridgeTest run (now five tests), root quality checks and required multi-platform CI, review this bounded delta, and recheck integration with current dev before marking ready. A missing/old runtime credential will be refused, consistent with #505; stream refusals are now diagnosable in the host log. Watch flows still have no subscription counter in the fake, but the wire refusal tests fail if any value leaks; broader fake instrumentation is optional.

The latest requested Claude review is #635 (comment). Shared authentication-helper consolidation and stale transport inventory comments remain #575 context; no cross-group PR was changed. Avoid introducing another generic flow abstraction in this bounded security patch: the existing synchronous capture sites remain explicit, and refusal logging is centralized locally.

Original-head CI and actual XML were verified green on Linux, macOS and Windows. That evidence does not validate these new reviewer changes. Updated-head CI is queued/in progress; local Gradle validation could not acquire the shared build lock during this bounded review. This is an infrastructure/validation handoff, not evidence of an original contributor CI failure. All repairs are reviewer work and are not counted toward your original contribution score. No app instance or live database was launched or stopped.

Per the assigned bounded-disposition policy, I am converting this PR to draft after this handoff. No checks were bypassed and no unvalidated merge was attempted.

[Antriksh1984]

Confirmed current head 345ee24d (with the revocation tests, structured AUTH logging on the stream guards, and fake-provider fix from the review) is 10/10 green and mergeStateStatus: CLEAN against dev. Restoring ready-for-review.

[kshivang]

@claude Please review current head 83a848d0cd1b140159f28bee9b5c60a61a4371b8 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 7m 3s —— View job

---

Review of 83a848d vs origin/dev

• Gather context (diff vs origin/dev, bridge, tests, interceptor, registry, proto, client proxy)
• Verify the replay preserved the reviewer repairs from 345ee24
• Check every RPC identity gate
• Check synchronous identity capture + unary/live-stream revocation
• Check regression/test coverage
• Check provider compatibility and dev integration

Verdict: no blocking correctness or security defect. The gate is complete, fails closed, and the replay onto current dev is clean. Five non-blocking findings; the only one I'd act on inside this PR is the mid-stream log reason (#1), which is a two-line change to the diagnostics this PR just added.

---

What I verified on this head

16/16 RPCs gated, matched against the proto rather than the description. git.proto declares exactly 16 rpc lines; 12 unary call authenticatedCallerOrRefuse as their first statement (GitServiceBridge.kt:132-207) and the 4 streams guard inline (:32-36, :61-65, :94-98, :110-114). No RPC reaches provider before the check, and none is left to the generated UNIMPLEMENTED default. The count correction (14 → 16) landed.

Synchronous capture is correct and is pinned by a positive test, not assumed. Each stream reads CURRENT_IDENTITY.get() on the calling thread before return flow { … }, so the lambda never depends on the gRPC Context being current when it is collected. That is the split SecretServiceBridge.kt:222-225 prescribes (AUTHENTICATED_PROCESS_ID is a per-call snapshot; a stream must read CURRENT_IDENTITY), and it is byte-for-byte the shape DownloadServiceBridge.kt:38-49 now carries in dev. an authenticated caller can watch status and read the repository would fail PERMISSION_DENIED if the capture were wrong.

Revocation semantics are right in both directions. The recheck compares the resolved identity, not merely non-null, so a token reissued to a different process id is refused too. ProcessTokenRegistry.issue removes the previous token for the same id (ProcessTokenRegistry.kt:57-62), and the captured closure re-resolves the original token - so a respawned child's stale token resolves to null, != caller, and the live stream dies. revoke is idempotent and @Synchronized.

Fails closed on both absences. No interceptor installed → CURRENT_IDENTITY.get() is null → refuseIdentity. Interceptor installed but no/unknown token → AUTHENTICATED_PROCESS_ID unset and currentIdentity.invoke() null → refused. Nothing anywhere derives identity from the request body.

The two new regressions are the shape dev settled on, and they are deterministic. revoked credentials cannot invoke a unary operation (GitServiceBridgeTest.kt:158-166) matches SecretServiceBridgeTest.kt:238 / RoleManagementServiceBridgeTest.kt:230, and revoked watcher cannot receive a later snapshot (:167-190) is structurally identical to DownloadServiceBridgeTest.kt:164-187. The ordering is not racy: received.receive() returns only after emission #1 reaches the client, revoke completes before setFileStatus, and StateFlow conflation means no pre-revocation value can still be queued - so the emission that trips the guard is strictly post-revocation. assertTrue(received.tryReceive().isFailure) proves no snapshot crossed the wire, which is the claim that matters.

Replay integrity. origin/dev is an ancestor of HEAD; the diff is exactly the two expected files (+425/-19) with no stray dev file touched, and all three commits carry the original author. Both reviewer repairs survived: the refuseIdentity AUTH log with NO_IDENTITY matching all five siblings verbatim, the emission-time-revocation sentence in the class KDoc, and the fake's getCurrentProjectPath recording (so the provider.calls.isEmpty() assertion in the anonymous test now means what its message says for that RPC).

Provider compatibility / no monolith regression. GitServiceBridge is constructed in exactly one place (KernelBootstrap.kt:977), inside a registerServiceBridges that returns early in MONOLITH (:971), on the only BossIpcServer construction, which is given the registry (:486). Nothing in-process changes.

Style gates I could check without Gradle: no line over the 140-column .editorconfig limit in either file, @Suppress("TooManyFunctions") present with the same justifying comment the sibling bridges use. I did not run Gradle (see the bottom note).

---

Findings

1. The mid-stream refusal logs a reason that can be false (observability, low - but it is this PR's own new diagnostic).
refuseIdentity is shared by the initial capture and the per-emission recheck, and always logs "Refused $rpc: no current verified process identity on this call" (GitServiceBridge.kt:202-210). The recheck fires in two different situations: the token stopped resolving (revoked, or invalidated by a respawn) and it now resolves to a different process id. In the second case an identity is present, so the line states something untrue in the exact place an operator's diagnosis starts. Cheapest fix is a reason parameter, or a second helper for the revocation path - mapOf("rpc" to rpc) could also carry whether an identity was present, without logging the id.
Fix this →

2. Live-stream revocation is covered for one stream out of four (test coverage, low-medium).
watchCommitLog, watchIsGitRepo and watchIsLoading carry independent copies of the same eight-line guard. The anonymous test does exercise all four, so an entirely ungated stream is caught - but a copy that captures the identity and omits the per-emission recheck passes every test here. Since the maintainer explicitly declined a shared flow abstraction for this bounded patch, the test is the only remaining defense against that divergence. One parameterized/table-driven test over the four (watch call, mutate fake) pairs closes it without touching production shape.
Fix this →

3. The client half still swallows the refusal (pre-existing in dev, worth a release-note line).
GitDataProviderProxy (plugin-platform, untouched here) catches PERMISSION_DENIED as catch (_: Exception) in all four watch loops and retries with backoff to 30s forever (:56-64 and siblings), and refreshStatus, refreshLog, openFile and getCurrentProjectPath are silent no-ops on failure. gitOp does surface the message, so mutations report an error. With this PR's host-side AUTH log, a credential-less runtime is now diagnosable from the host log - the plugin-side symptom is still an empty git panel with nothing in the plugin log. The concrete instance is version skew: ChildProcessBootstrap.processToken is optional by design, so an out-of-repo boss-microkernel-runtime built pre-#505 connects with no token and loses git entirely. Git is a much more visible surface than the vault or the RBAC tables, so this is worth one line in the PR description or release notes plus, eventually, a one-time warn in the proxy.

4. This is authentication, not authorization (scope note).
Any authenticated child process can discardChanges, checkout, cherryPick or revert against the real repository - the gate establishes who is calling, not whether that identity should be allowed destructive git. That is the right scope for this PR and matches the siblings; recording it so nobody reads "gated" as "least privilege".

5. ProcessIdentityInterceptor's KDoc is now off by three (docs, low, dev drift).
ProcessIdentityInterceptor.kt:17-24 still says "thirteen other service bridges … none of them authenticated today" and names PluginUI, Secret and RoleManagement as "the three that currently make it". The real count on this head is six - Download and Supabase landed in dev via #550 without updating it, and Git is the sixth. That KDoc is the canonical list of who checks identity, so it is the one place the drift costs something. Belongs with the shared-helper consolidation in #575 rather than here, given the bounded scope.

On the fake's missing subscription counter for the watch flows: agreed with the earlier handoff that it is optional - the wire-level refusal assertions fail if any value leaks, so it is belt-and-braces only.

---

Not run here

I read the code and did not run Gradle (:composeApp:desktopTest, detekt, ktlintCheck); new-head CI is pending per the request, and none of the findings above depends on a build. Nothing in the two files exceeds the 140-column ktlint limit and the detekt suppression the sibling bridges need is present, but that is not a substitute for the real checks.
• branch fix/git-bridge-caller-identity

[kshivang]

@Antriksh1984 Current head passes all ten hosted checks; local GitServiceBridgeTest passes 5/5 and detekt passes. The current Claude review found no blocking correctness/security defect. Your 16 gates and original wire tests are retained, separately credited from prior reviewer tests/diagnostics.

Review reconciliation: the proposed different-identity log reason is not a reachable transition in the actual ProcessTokenRegistry: issue mints a fresh token and removes its predecessor; revoke removes it; the captured token returns the same id or null. The latest ActiveTabs review independently confirms that invariant. No reason-dependent production branch is needed here. All four independent watch guards were checked against the proto and source; anonymous refusal covers all four, and the deterministic wire regression covers the revocation mechanism. More per-stream mutation tests would strengthen coverage but no omitted gate was found. Shared helper/KDoc enumeration stays with #575. Existing proxy retry/version-skew behavior and authentication-versus-capability scope are documented in the description. No UI change or native visual check applies.

Proceeding with the serialized dev-only merge after a fresh exact-head/check/integration preflight. Original score remains 9/10; reviewer repairs are excluded.