Skip to content

Commit 09066d4

Browse files
committed
fix(craft): record receipts for ALWAYS-policy writes
The ALWAYS branch forwards without session resolution, so an admin-relaxed write executed with no receipt at all. A write now resolves its session best-effort and records, reads keep the cheap path, and an unattributable write goes unrecorded rather than blocked.
1 parent 5f9bcdc commit 09066d4

2 files changed

Lines changed: 106 additions & 0 deletions

File tree

backend/onyx/sandbox_proxy/addons/gate.py

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
from onyx.configs.constants import NotificationType
2727
from onyx.db.engine.sql_engine import get_session_with_tenant
2828
from onyx.db.enums import (
29+
ActionEffect,
2930
ApprovalDecidedVia,
3031
ApprovalDecision,
3132
EndpointPolicy,
@@ -520,6 +521,39 @@ async def _dispatch_approved_request(
520521
)
521522
await self._record_receipts(flow, ctx, matched_actions, approval_id)
522523

524+
async def _record_always_write_receipts(
525+
self,
526+
flow: http.HTTPFlow,
527+
sandbox: ResolvedSandbox,
528+
matched_actions: AllMatchedActions,
529+
) -> None:
530+
"""An admin-set ALWAYS policy must not exempt a write from the
531+
activity record. Best-effort: the ALWAYS path deliberately skips
532+
session resolution, so a write resolves it here and an unattributable
533+
one goes unrecorded rather than blocked."""
534+
if not any(
535+
action.effect is ActionEffect.WRITE for action in matched_actions.actions
536+
):
537+
return
538+
try:
539+
session_id = self._resolve_gated_session(flow, sandbox)
540+
except Exception:
541+
logger.exception(
542+
"receipt_session_lookup_error tenant=%s sandbox=%s host=%s",
543+
sandbox.tenant_id,
544+
sandbox_log_label(sandbox),
545+
flow.request.host,
546+
)
547+
return
548+
if session_id is None:
549+
return
550+
await self._record_receipts(
551+
flow,
552+
sandbox.with_session(session_id),
553+
matched_actions,
554+
approval_id=None,
555+
)
556+
523557
async def _record_receipts(
524558
self,
525559
flow: http.HTTPFlow,
@@ -766,6 +800,7 @@ async def _resolve_and_match(
766800
),
767801
credential_outcome_label(injection),
768802
)
803+
await self._record_always_write_receipts(flow, sandbox, matched_actions)
769804
return None
770805

771806
# ASK: resolve the originating session before prompting. An

backend/tests/unit/sandbox_proxy/test_gate_receipts.py

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,3 +195,74 @@ def test_hooks_ignore_unrecorded_flows(
195195
asyncio.run(addon.error(tflow.tflow()))
196196

197197
assert calls == []
198+
199+
200+
def test_always_policy_writes_still_record(
201+
monkeypatch: pytest.MonkeyPatch,
202+
) -> None:
203+
addon = _addon()
204+
recorded: list[Any] = []
205+
206+
async def _fake_record(*args: Any, **kwargs: Any) -> None:
207+
recorded.append((args, kwargs))
208+
209+
session_id = uuid4()
210+
monkeypatch.setattr(addon, "_record_receipts", _fake_record)
211+
monkeypatch.setattr(
212+
addon, "_resolve_gated_session", lambda _flow, _sandbox: session_id
213+
)
214+
flow = tflow.tflow()
215+
sandbox = _ctx().without_session()
216+
217+
asyncio.run(addon._record_always_write_receipts(flow, sandbox, _MATCHED))
218+
219+
assert len(recorded) == 1
220+
ctx = recorded[0][0][1]
221+
assert ctx.session_id == session_id
222+
223+
224+
def test_always_policy_reads_skip_session_resolution(
225+
monkeypatch: pytest.MonkeyPatch,
226+
) -> None:
227+
addon = _addon()
228+
229+
def _must_not_resolve(_flow: Any, _sandbox: Any) -> None:
230+
raise AssertionError("reads must not pay session resolution")
231+
232+
monkeypatch.setattr(addon, "_resolve_gated_session", _must_not_resolve)
233+
read_only = AllMatchedActions(
234+
actions=(
235+
MatchedAction(
236+
action_type="slack.messages.read",
237+
display_name="Read messages",
238+
description="Read messages.",
239+
policy=EndpointPolicy.ALWAYS,
240+
effect=ActionEffect.READ,
241+
),
242+
),
243+
target=GatedTarget(kind=GatedAppKind.EXTERNAL_APP, id=1, app_name="Slack"),
244+
)
245+
246+
asyncio.run(
247+
addon._record_always_write_receipts(
248+
tflow.tflow(), _ctx().without_session(), read_only
249+
)
250+
)
251+
252+
253+
def test_always_policy_unattributable_write_goes_unrecorded(
254+
monkeypatch: pytest.MonkeyPatch,
255+
) -> None:
256+
addon = _addon()
257+
258+
async def _must_not_record(*_args: Any, **_kwargs: Any) -> None:
259+
raise AssertionError("unattributable writes record nothing")
260+
261+
monkeypatch.setattr(addon, "_record_receipts", _must_not_record)
262+
monkeypatch.setattr(addon, "_resolve_gated_session", lambda _flow, _sandbox: None)
263+
264+
asyncio.run(
265+
addon._record_always_write_receipts(
266+
tflow.tflow(), _ctx().without_session(), _MATCHED
267+
)
268+
)

0 commit comments

Comments
 (0)