Conversation
65610d2 to
ee6f619
Compare
b06ea70 to
51d921b
Compare
8b1f913 to
bb93d53
Compare
bb93d53 to
1c6bf4c
Compare
andrexibiza
left a comment
There was a problem hiding this comment.
Reviewed the Layer 3 delta at exact head 1c6bf4cc1a5294d7387bb5dd9eb020dedcef1b44, after #97744 head 9560439e970ca390d1aab4feb7eb649b754da10e. Exact-head CI, Docker, and Nix are green.
P1 security — near-expiry renewal silently authorizes policy and capability drift
The PR contract says execution-policy or capability changes pause the route for explicit reauthorization. The renewal path currently does the opposite:
gateway/platforms/api_server_room_grants.py::_handle_room_member_grant_refresh()authenticates the still-live old dispatch grant, recomputes the target's current execution policy, and mints a replacement grant with that new policy digest. It never compares the new digest with the digest authorized by the old grant.tui_gateway/hosted_room_peer_http.py::refresh_grant()then probes the live capability endpoint with the replacement and returns the new catalog.tui_gateway/hosted_room_service.py::_RouteStatusPeerClientpersists that replacement and rewrites the pendingHostedMemberDispatchto the refreshed catalog and execution-policy digests before dispatch.
Reproduction shape: issue a grant/catalog under policy A; change the target policy or advertised capability catalog to B; let the grant enter the automatic refresh window; dispatch the next exact task. The home gateway renews, adopts B, rewrites the frozen dispatch digests, and executes without the explicit reauthorization this PR promises.
Required repair:
- renewal must preserve the previously authorized policy/catalog, or return a typed
room_reauthorization_requiredresult when either digest changed; - the home side must never patch a pending dispatch to a newly observed digest. Any drift must mark the route
needs_reauthorizationand leave the original work frozen; - add near-expiry tests for policy-only drift, capability-only drift, simultaneous drift, unchanged renewal, restart after drift, and response-lost renewal replay.
Additional landing blockers
- The inherited #97712 replay page still counts Unicode characters instead of UTF-8 bytes.
- This delta modifies
gateway/hosted_rooms.pyandtui_gateway/server.pyafter they exceed the repository's hard 2,000-physical-line ceiling. Those touched seams must be sharded.
This head is not merge-ready. A repaired head needs fresh exact-head proof before downstream Desktop, Files, or Messaging layers consume it.
1c6bf4c to
2d7a346
Compare
2d7a346 to
5089c19
Compare
andrexibiza
left a comment
There was a problem hiding this comment.
Re-reviewed the rewritten Layer 3 carrier at exact head 5089c1910bd50aab508b2b68228d90e8b1d4c5b0 and rechecked the prior near-expiry reauthorization blocker through the target grant handler, peer HTTP client, route-status wrapper, and current regressions.
The prior P1 is repaired on this object. Grant refresh now rejects target execution-policy drift against the previously authorized execution_policy_digest; the home-side refresh passes the frozen capability/policy digests, compares any refreshed catalog to those exact digests, marks the route needs_reauthorization, and refuses dispatch instead of rewriting the pending dispatch to newly observed authority. The regression matrix covers policy-only drift, capability-only drift, and simultaneous drift and proves no refreshed route is published and no peer dispatch occurs. The unchanged path persists the replacement grant before remote admission.
The current three-commit train has exact hosted proof on every surviving object:
f5908c18d86a4d4a572a464be2d00e9eb8cec3ee: CI33332784224, Docker33332783891, Nix33332783919— success.ee2f98efe1fb9f0f5f9a8e98be5d78b962217654: CI33336745613, Docker33336745271, Nix33336745272— success.5089c1910bd50aab508b2b68228d90e8b1d4c5b0: CI33336746960, Docker33336746521, Nix33336746522— success.
I do not see the prior Layer 3 policy/capability-drift defect surviving on 5089c191....
This PR is still not landing-ready for two upstream/structural reasons that exact-head green does not erase. First, its current history is f5908c18... → ee2f98ef... → 5089c191..., while current Layer 1 #97712 has since moved to ad949774f0072499c56c8e0e86fb056cb877b315 on top of f5908c18...; Layer 3 therefore is not a cumulative descendant of the current Layer 1 carrier and cannot transfer current Layer 1 proof forward until the stack is restacked. Second, this cumulative diff still inherits the #97712 modification to hermes_cli/web_server.py, which remains above the repository's 2,000-physical-line touched-file ceiling. Current live main@d63f996a757f6255fc1454239616ab4b4435e0f5 is also two path-disjoint Photon/docs commits beyond the submitted base, so the final accepted stack still needs current-main recomposition.
Required closure: finish the Layer 1 structural extraction, restack Layers 2–3 on the final accepted #97712 object without losing this fail-closed reauthorization repair, then reacquire CI/Docker/Nix for every surviving commit in the rebuilt train. On the current 5089c191... Layer 3 delta itself, I do not see another source-level authorization blocker.
andrexibiza
left a comment
There was a problem hiding this comment.
Landing-edge correction to my immediately preceding review: main moved while that review was being read back, and Layer 1 has now been salvaged directly into upstream.
Current main is ed3562bbbcd8a5323be96b81812291faae99e010. #97712 is now closed/unmerged, but its accepted foundation landed as cbc67b939f71389494b842b5499ebe104ebda46e followed directly by maintainer follow-up cc4b5ba1fc48e86df99f139a46232fd5debf1834, which carries replay-page authority lineage and the sabotage-sensitive UTF-8 byte-bound proof.
That supersedes two landing statements in review 5062754503:
- Layer 3 no longer needs to wait for #97712 itself to become an accepted current-main carrier; main now owns the Layer 1 foundation.
- The inherited
hermes_cli/web_server.py>2K delta is not a downstream diff blocker once this stack is correctly rebased and the already-landed Layer 1 ancestor is dropped from the PR history.
The Layer 3 source verdict is unchanged: the prior policy/capability auto-reauthorization P1 is repaired on 5089c191..., and the three currently surviving branch commits had exact-green CI/Docker/Nix when checked.
The required closure is now a real restack onto salvaged main, not a wait on #97712: rebuild #97744 on main, then rebuild this Layer 3 commit on that Layer 2 object, preserving the fail-closed reauthorization repair. Do not replay f5908c18... as a new duplicate ancestor. During the rebase, preserve current-main compression changes in gateway/run.py; main modified that seam after this stack was cut. Reacquire CI/Docker/Nix on every surviving rebuilt commit before transferring any prior green receipt.
This correction is about the moving landing edge only; it does not retract the current-head source review.
|
Maintainer-salvage note: the current squashed Layer-3 head does not preserve the source authors, but the source-authored carrier train is still available on The commits to preserve before the David-authored RoomLink reconciliation are:
The later commits through For Layer 4, the #96162 behavior is separately available as a current-main TSX forward-port authored by jugol at I will not rewrite the public heads while the maintainer sweep is active; these archive refs exist only so the salvage can cherry-pick the actual authors. |
|
@teknium1 — Current-main salvage evidence is now available in draft #99159. Its first commit For the actual Layer-3 merge carrier, preserve the source-authored train from |
… drift Follow-ups on the salvaged #97797 transport: - Blocker 2 from the #97681 exact-head review claimed near-expiry refresh silently mints against the target's CURRENT policy. On this head the handler DOES refuse drift (_require_unchanged_execution_policy -> 403 room_reauthorization_required), but nothing pinned the handler-level behavior: removing the drift check still passed the entire grants suite (the check was only unit-tested in isolation). New HTTP-level regression test drives /v1/room-members/grants/refresh with a drifted-policy grant and requires the 403; sabotage-verified (check removed -> test fails). - cancel() conflict resolution: keeps our race-retry routing loop from #99099 with this layer's peer-stop acknowledgement body inside it (peer receipt -> settle completion -> local interrupt escalation). - docs: NAT one-way-reachability note in bot-mode.md — Desktop is a viewer, not a relay; put room authority on the host everyone can reach (field finding from /bin/bash on #97681).
… drift Follow-ups on the salvaged #97797 transport: - Blocker 2 from the #97681 exact-head review claimed near-expiry refresh silently mints against the target's CURRENT policy. On this head the handler DOES refuse drift (_require_unchanged_execution_policy -> 403 room_reauthorization_required), but nothing pinned the handler-level behavior: removing the drift check still passed the entire grants suite (the check was only unit-tested in isolation). New HTTP-level regression test drives /v1/room-members/grants/refresh with a drifted-policy grant and requires the 403; sabotage-verified (check removed -> test fails). - cancel() conflict resolution: keeps our race-retry routing loop from #99099 with this layer's peer-stop acknowledgement body inside it (peer receipt -> settle completion -> local interrupt escalation). - docs: NAT one-way-reachability note in bot-mode.md — Desktop is a viewer, not a relay; put room authority on the host everyone can reach (field finding from /bin/bash on #97681).
|
Merged via #99244 (rebase-merge, your commit and authorship preserved) — #99244, on main at 1cf3639. Follow-ups that rode on top:
Also heads-up: your remaining branches now rebase onto the replication/takeover layer from #99047 ( |
… drift Follow-ups on the salvaged NousResearch#97797 transport: - Blocker 2 from the NousResearch#97681 exact-head review claimed near-expiry refresh silently mints against the target's CURRENT policy. On this head the handler DOES refuse drift (_require_unchanged_execution_policy -> 403 room_reauthorization_required), but nothing pinned the handler-level behavior: removing the drift check still passed the entire grants suite (the check was only unit-tested in isolation). New HTTP-level regression test drives /v1/room-members/grants/refresh with a drifted-policy grant and requires the 403; sabotage-verified (check removed -> test fails). - cancel() conflict resolution: keeps our race-retry routing loop from NousResearch#99099 with this layer's peer-stop acknowledgement body inside it (peer receipt -> settle completion -> local interrupt escalation). - docs: NAT one-way-reachability note in bot-mode.md — Desktop is a viewer, not a relay; put room authority on the host everyone can reach (field finding from /bin/bash on NousResearch#97681).
… drift Follow-ups on the salvaged NousResearch#97797 transport: - Blocker 2 from the NousResearch#97681 exact-head review claimed near-expiry refresh silently mints against the target's CURRENT policy. On this head the handler DOES refuse drift (_require_unchanged_execution_policy -> 403 room_reauthorization_required), but nothing pinned the handler-level behavior: removing the drift check still passed the entire grants suite (the check was only unit-tested in isolation). New HTTP-level regression test drives /v1/room-members/grants/refresh with a drifted-policy grant and requires the 403; sabotage-verified (check removed -> test fails). - cancel() conflict resolution: keeps our race-retry routing loop from NousResearch#99099 with this layer's peer-stop acknowledgement body inside it (peer receipt -> settle completion -> local interrupt escalation). - docs: NAT one-way-reachability note in bot-mode.md — Desktop is a viewer, not a relay; put room authority on the host everyone can reach (field finding from /bin/bash on NousResearch#97681).
The user problem
#97744 lets a Group Chat continue on one gateway after Desktop closes. A Group
Chat containing Bots from two gateways still needs a safe way for the home
gateway to dispatch an exact member turn, recover it after response loss, and
observe Stop or completion without using Desktop as a courier.
This PR adds the scoped cross-gateway Group Chat transport, internally called RoomLink. It is the direct, text-first layer under the later Desktop continuity UX.
Important
This PR is stacked on #97712 and #97744. Review only the commits after the
current #97744 head
ee2f98efe1. It does not enable a new Desktop creation path by itself.Cross-gateway transport contract
/v1/runsadmission and status using one idempotency identity andimmutable request fingerprint.
loss or process restart.
epoch, member, target install, target profile, permissions, execution policy,
and expiry.
reservation. A status-only grant cannot mint new dispatch authority.
capability change pauses the route for explicit reauthorization.
and exact-run Stop.
than weakening the grant or silently running a different Bot.
Reliability behavior
bounded retry.
durable dispatch identity; it is never sent through a second route.
execution generation is terminal.
work.
cannot commit new room state.
Deliberate boundaries
is reachable from every participant, that gateway should hold authority;
reverse-dial or a relay is a later NAT-traversal increment.
introduce a new crypto system.
Those capabilities remain separate layers in #97681.
Source composition
The current squashed head carries the final behavior but does not preserve the
source authors in Git history. The maintainer salvage should use the archived
source-authored train at
dokterdok:archive/bot-mode-roomlink-source-authorship-20260831:e79a1c41ab: feat(api): durable idempotency for POST /v1/runs #88408 durable run idempotency;3141f21324+c38d416590: feat(peer): async run/status with idempotent /v1/runs admission #94336 CLI/tests and docs, excluding itssuperseded process-local API implementation;
148e839da7: fix(peer): route hermes peer dm through the credential-redirect guard #88819 credential-safe redirect port;c5404ada03: fix(bot-mode): pin the registry-owning profile on peer DM deliveries #93952 registry-owning profile routing.The later commits through
b06ea70847contain the David-authored extraction,RoomLink integration, and P1/P2 hardening. This preserves the actual authors
without assigning the replacement server implementation to the wrong source.
Validation
Current head
5089c1910b, stacked directly on #97744 headee2f98efe1:5089c1910bgit diff --checkThe final branch preserves the source contributor commits and adds only scoped composition, hardening, reconciliation, and hermetic tests. The current head authenticates before reserving work, bounds peer reads by bytes and wall clock, fails closed on policy drift and revoked grants, uses structured revocation errors, and preserves exact policy numbers across JavaScript.
The standalone head has green required CI and 161 focused / 672 broader tests. The complete integration tree in #98307 has repeated same-network TLS text/history, restart, Stop, route revocation, and offline recovery on real gateways without changing this transport boundary.
Related work
Type of change