diff --git a/PRIMITIVES.md b/PRIMITIVES.md index 6a9edea8..9c4e8e5b 100644 --- a/PRIMITIVES.md +++ b/PRIMITIVES.md @@ -127,7 +127,18 @@ MCP servers, validated against a toolspec. cookies + localStorage after each acting call and on graceful stop into a per-session Secret — the one Secret the runner is RBAC-allowed to write, never model-visible — and re-injects it when the pod wakes after a reap, so a login - survives pod churn (`pkg/agent/tool/browser/capture.go`). + survives pod churn (`pkg/agent/tool/browser/capture.go`). Phase D attaches a + **redacted** page screenshot to every browser approval/human-step card — the + acting-approval (`web_host#act`) plan-gate ask, `request_human_step`'s + confirm/code prompts, and the CAPTCHA notice — so the human deciding sees the + same page the agent does, not just its summary: credential-filled fields are + painted opaque black (`pkg/agent/tool/browser/screenshot_capture.go` resolves + each filled field's box via `dom.GetBoxModel` and fails closed on any + unresolved box; `pkg/agent/tool/browser/screenshot_redact.go` does the pixel + paint-over) before the PNG is ever stored, and the screenshot is attached to + `channelevents.InteractionRequestPayload.Screenshot` — never placed in the + model's context. Slack uploads it into the approval thread, the web chat + embeds it inline, and the local TUI links it via a signed artifact-view URL. - **Binaries:** `internal/cmd/streamclient/` (claims a streaming ToolCall via the gateway); tool execution is otherwise driven from `internal/cmd/runner/` diff --git a/pkg/agent/runner/browser_screenshot.go b/pkg/agent/runner/browser_screenshot.go new file mode 100644 index 00000000..88c790c7 --- /dev/null +++ b/pkg/agent/runner/browser_screenshot.go @@ -0,0 +1,202 @@ +package runner + +import ( + "bytes" + "context" + "fmt" + "io" + "log/slog" + + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + + browsertool "github.com/authzed/openagentprimitives/pkg/agent/tool/browser" + spiceboxv1alpha1 "github.com/authzed/openagentprimitives/pkg/apis/v1alpha1" + "github.com/authzed/openagentprimitives/pkg/channels/channelevents" + "github.com/authzed/openagentprimitives/pkg/memory" + memartifact "github.com/authzed/openagentprimitives/pkg/memory/kinds/artifact" + "github.com/authzed/openagentprimitives/pkg/platform/artifacts" + "github.com/authzed/openagentprimitives/pkg/platform/artifactstore" +) + +// screenshotInlineMaxBytes is the ArtifactRenderSpec.Payload inline cap +// (pkg/apis/v1alpha1/artifactrender_types.go:57-65: "Payload inline, ≤256 +// KiB. Exactly one of Payload or PayloadRef MUST be set."). A redacted +// screenshot at or under this size is attached inline; a larger one is +// written to the artifact store first and referenced by PayloadRef. +const screenshotInlineMaxBytes = 256 << 10 + +// Fixed metadata for every screenshot artifact this package creates: one +// filename/alt-text for the whole feature, so it only needs saying once. +// The renderer kind and MIME are NOT literals here — see +// channelevents.ScreenshotRendererKind / ScreenshotMIME for why (this +// package is under mime_opinion_guard_test.go). +const ( + screenshotFilename = "page.png" + screenshotAltText = "current page (credential fields redacted)" +) + +// screenshotArtifactPutter is the subset of artifactstore.Store that +// storeScreenshotArtifact needs: Put, to store a screenshot too large to +// inline. Narrowed (mirroring secretOutputStatusWriter in loop_secretout.go +// and uiResourceStatusWriter in loop_uiresource.go) so a test can provide a +// minimal recorder without constructing a full artifactstore.Store. +type screenshotArtifactPutter interface { + Put(ctx context.Context, key string, r io.Reader) (artifactstore.Ref, error) +} + +// screenshotStoreDeps bundles the backend wiring storeScreenshotArtifact +// needs to turn redacted screenshot bytes into a durable ArtifactRender CR: +// the CR client, the artifact store (consulted only when the PNG exceeds the +// inline cap), and the session's UID for the CR's owner-ref. +// +// NewRenderName and NewArtifactID are optional id seams: nil means +// production (artifacts.RenderName and memory.NewID(memartifact.Kind{}), +// the same pair artifacts.Service itself defaults to) — mirroring +// artifacts.Service's own WithRenderNameMinter/newID seams so tests can +// assert on a deterministic CR name and artifact id instead of a random one. +type screenshotStoreDeps struct { + Client client.Client + ArtifactStore screenshotArtifactPutter + SessionUID types.UID + + NewRenderName func(session string) string + NewArtifactID func() string +} + +// storeScreenshotArtifact persists a redacted page screenshot (the PNG bytes +// Task 2 produces) as an ArtifactRender{Kind:"image"} CR and returns the +// AttachmentRef the interaction request payload's Screenshot field carries. +// +// Mirrors the CR-build shape in pkg/agent/tool/meta/artifact_prepare.go:264-274 +// and pkg/platform/artifacts.NewRender: owned by the session (by UID, as +// controller, blocking deletion), labeled with the artifact id. Unlike +// artifact_prepare / persistWidget (loop_uiresource.go) this does NOT poll the +// CR to Ready: a screenshot attaches to a live interaction request, not a +// model-facing handle, and AttachmentRef's own contract (see the doc comment +// on channelevents.AttachmentRef) is that the consumer re-Gets the CR and +// reads status.outputRef at delivery time — there is nothing for this call to +// wait on. +// +// A PNG at or under screenshotInlineMaxBytes is attached inline +// (Spec.Payload); a larger one is written to the artifact store first and +// referenced by Spec.PayloadRef. Exactly one of the two is ever set. +func storeScreenshotArtifact(ctx context.Context, deps screenshotStoreDeps, sess channelevents.SessionRef, png []byte) (*channelevents.AttachmentRef, error) { + // Fail closed on a nil Client with an ERROR, not a nil-interface panic: + // deps.Client is a plain client.Client interface field (Loop.Client), and a + // Loop built without one (a kubectl-driven session, today's e2e in-process + // harness — see Loop.screenshotStoreDeps' doc) must degrade to "ship no + // screenshot" like any other capture/store failure, never crash the + // dispatch goroutine calling it. + if deps.Client == nil { + return nil, fmt.Errorf("storeScreenshotArtifact: no k8s client wired (Loop.Client is nil)") + } + newRenderName := deps.NewRenderName + if newRenderName == nil { + newRenderName = artifacts.RenderName + } + newArtifactID := deps.NewArtifactID + if newArtifactID == nil { + newArtifactID = func() string { return memory.NewID(memartifact.Kind{}) } + } + + artifactID := newArtifactID() + renderName := newRenderName(sess.Name) + + spec := spiceboxv1alpha1.ArtifactRenderSpec{ + Kind: channelevents.ScreenshotRendererKind, + Filename: screenshotFilename, + AltText: screenshotAltText, + } + if len(png) <= screenshotInlineMaxBytes { + spec.Payload = png + } else { + if deps.ArtifactStore == nil { + return nil, fmt.Errorf("storeScreenshotArtifact: screenshot is %d bytes (> %d-byte inline cap) but no artifact store is wired", len(png), screenshotInlineMaxBytes) + } + key := fmt.Sprintf("%s/%s/%s/%s", sess.Namespace, sess.Name, artifactID, screenshotFilename) + ref, err := deps.ArtifactStore.Put(ctx, key, bytes.NewReader(png)) + if err != nil { + return nil, fmt.Errorf("storeScreenshotArtifact: store screenshot bytes: %w", err) + } + spec.PayloadRef = string(ref) + } + + cr := artifacts.NewRender(renderName, sess.Namespace, sess.Name, deps.SessionUID, artifactID, spec, nil) + if err := deps.Client.Create(ctx, cr); err != nil { + return nil, fmt.Errorf("storeScreenshotArtifact: create ArtifactRender: %w", err) + } + + return &channelevents.AttachmentRef{ + RenderName: cr.Name, + ArtifactID: artifactID, + MIME: channelevents.ScreenshotMIME, + Filename: screenshotFilename, + AltText: screenshotAltText, + }, nil +} + +// screenshotStoreDeps builds storeScreenshotArtifact's backend wiring from +// this Loop's own fields: l.Client creates the ArtifactRender CR — the same +// value applyUIResource's own CR create+poll uses (loop.go's +// uiResourceDeps{Client: l.Client, ...}) — and l.AgentSession's UID owns it +// (mirrors loop.go's own "var sessionUID types.UID; if l.AgentSession != nil +// { sessionUID = l.AgentSession.UID }" guard — empty when Loop.AgentSession is +// unset, e.g. a kubectl-driven session or a unit test). +// +// ArtifactStore is deliberately left nil: Loop carries no write-capable +// artifactstore.Store today. l.Artifacts (*artifacts.Service) manages +// ArtifactRender CR headers/revisions over memory.Memory, and l.ArtifactReader +// is read-only (modality.ArtifactReader) — neither is a screenshotArtifactPutter. +// That means only the <=screenshotInlineMaxBytes inline Payload path is +// reachable from this wiring; a screenshot over the cap makes +// storeScreenshotArtifact return an error, which the caller +// (captureBrowserScreenshotFn below) logs and treats as "ship no screenshot" — +// same as any other capture/store failure, per the best-effort/fail-closed +// contract. A single-viewport redacted PNG is well under 256 KiB in practice, +// so this degrades gracefully rather than requiring a new store seam to be +// threaded through Loop for this feature. +func (l *Loop) screenshotStoreDeps() screenshotStoreDeps { + var sessionUID types.UID + if l.AgentSession != nil { + sessionUID = l.AgentSession.UID + } + return screenshotStoreDeps{ + Client: l.Client, + SessionUID: sessionUID, + } +} + +// captureBrowserScreenshotFn builds the best-effort, fail-closed closure a +// runnerHost's captureScreenshot field carries: capture a redacted screenshot +// via cap, store it as an ArtifactRender through l, and return the +// AttachmentRef to attach to a browser interaction payload — or nil, having +// logged why, on ANY capture or store failure. A capture/store failure NEVER +// surfaces as an error to the caller; the approval publish always proceeds. +// +// Shared by BOTH places a browser-sourced ask gets published (D-R3: one +// browser session, one capturer, two publish call sites, because +// request_human_step is itself a browser tool): executeToolContained wires it +// onto the per-call host it builds for the web_host#act acting-approval +// (plan_phase ask), and humanStepRequester.Confirm/RequestCode wire it onto +// the host THEY bind for the login_human_step ask — two different *runnerHost +// values (the latter is bound fresh per round-trip via hostBinder, see its +// doc), but the SAME Loop and the SAME capturer, which is all this closure +// needs. +func (l *Loop) captureBrowserScreenshotFn(sess channelevents.SessionRef, cap browsertool.BrowserScreenshotCapturer) func(context.Context) *channelevents.AttachmentRef { + return func(ctx context.Context) *channelevents.AttachmentRef { + png, err := cap.CaptureRedacted(ctx) + if err != nil { + slog.Info("browser screenshot: capture failed; publishing text-only", + "session", sess.Namespace+"/"+sess.Name, "err", err.Error()) + return nil + } + ref, err := storeScreenshotArtifact(ctx, l.screenshotStoreDeps(), sess, png) + if err != nil { + slog.Info("browser screenshot: store failed; publishing text-only", + "session", sess.Namespace+"/"+sess.Name, "err", err.Error()) + return nil + } + return ref + } +} diff --git a/pkg/agent/runner/browser_screenshot_test.go b/pkg/agent/runner/browser_screenshot_test.go new file mode 100644 index 00000000..4c0c5927 --- /dev/null +++ b/pkg/agent/runner/browser_screenshot_test.go @@ -0,0 +1,174 @@ +package runner + +import ( + "bytes" + "context" + "errors" + "io" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" + + spiceboxv1alpha1 "github.com/authzed/openagentprimitives/pkg/apis/v1alpha1" + "github.com/authzed/openagentprimitives/pkg/channels/channelevents" + "github.com/authzed/openagentprimitives/pkg/platform/artifactstore" +) + +// browserScreenshotTestScheme builds a scheme carrying the ArtifactRender CRD +// type. package runner (this file) cannot reuse another file's internal-test +// scheme helper across _test.go files of the same package that declare their +// own (see apptoolsGrantTestScheme's doc comment) — each file in this +// package's test suite mints its own, so this one does too. +func browserScreenshotTestScheme(t *testing.T) *runtime.Scheme { + t.Helper() + s := runtime.NewScheme() + require.NoError(t, spiceboxv1alpha1.AddToScheme(s), "AddToScheme") + return s +} + +// fakeScreenshotPutter is a minimal screenshotArtifactPutter recorder: it +// never touches real storage, just remembers the keys/bytes it was asked to +// Put and hands back a fixed Ref. +type fakeScreenshotPutter struct { + ref artifactstore.Ref + err error + calls []struct { + key string + bytes []byte + } +} + +func (f *fakeScreenshotPutter) Put(_ context.Context, key string, r io.Reader) (artifactstore.Ref, error) { + b, err := io.ReadAll(r) + if err != nil { + return "", err + } + f.calls = append(f.calls, struct { + key string + bytes []byte + }{key: key, bytes: b}) + if f.err != nil { + return "", f.err + } + return f.ref, nil +} + +func TestStoreScreenshotArtifact_InlineVsRef(t *testing.T) { + scheme := browserScreenshotTestScheme(t) + sess := channelevents.SessionRef{Namespace: "default", Name: "s1"} + + t.Run("PNG under the 256 KiB cap is stored inline", func(t *testing.T) { + c := fake.NewClientBuilder().WithScheme(scheme).Build() + putter := &fakeScreenshotPutter{ref: "mem://should-not-be-used"} + deps := screenshotStoreDeps{ + Client: c, + ArtifactStore: putter, + SessionUID: types.UID("sess-uid-1"), + } + small := bytes.Repeat([]byte{0xAB}, 1024) // < 256 KiB + + ref, err := storeScreenshotArtifact(context.Background(), deps, sess, small) + require.NoError(t, err) + require.NotNil(t, ref) + + assert.NotEmpty(t, ref.RenderName) + assert.NotEmpty(t, ref.ArtifactID) + assert.Equal(t, "image/png", ref.MIME) + assert.Equal(t, "page.png", ref.Filename) + assert.Equal(t, "current page (credential fields redacted)", ref.AltText) + + var cr spiceboxv1alpha1.ArtifactRender + require.NoError(t, c.Get(context.Background(), client.ObjectKey{Namespace: sess.Namespace, Name: ref.RenderName}, &cr), + "the created ArtifactRender must be readable back by the returned RenderName") + assert.Equal(t, ref.RenderName, cr.Name) + assert.Equal(t, "image", cr.Spec.Kind) + assert.Equal(t, small, []byte(cr.Spec.Payload), "small PNG must be inline") + assert.Empty(t, cr.Spec.PayloadRef, "small PNG must not set PayloadRef") + assert.Equal(t, ref.ArtifactID, cr.Labels["artifact.agentprimitives.authzed.com/id"]) + assert.Empty(t, putter.calls, "artifactstore.Put must not be called for an inline-sized PNG") + }) + + t.Run("PNG over the 256 KiB cap is stored by ref", func(t *testing.T) { + c := fake.NewClientBuilder().WithScheme(scheme).Build() + putter := &fakeScreenshotPutter{ref: "mem://default/s1/screenshot.png"} + deps := screenshotStoreDeps{ + Client: c, + ArtifactStore: putter, + SessionUID: types.UID("sess-uid-1"), + } + big := bytes.Repeat([]byte{0xCD}, 300*1024) // > 256 KiB + + ref, err := storeScreenshotArtifact(context.Background(), deps, sess, big) + require.NoError(t, err) + require.NotNil(t, ref) + assert.Equal(t, "image/png", ref.MIME) + + var cr spiceboxv1alpha1.ArtifactRender + require.NoError(t, c.Get(context.Background(), client.ObjectKey{Namespace: sess.Namespace, Name: ref.RenderName}, &cr)) + assert.Nil(t, cr.Spec.Payload, "big PNG must not be inlined") + assert.Equal(t, "mem://default/s1/screenshot.png", cr.Spec.PayloadRef, "big PNG must be stored by ref") + + require.Len(t, putter.calls, 1, "artifactstore.Put must be called exactly once for an oversized PNG") + assert.Equal(t, big, putter.calls[0].bytes, "the exact PNG bytes must reach the artifact store") + }) + + t.Run("artifact store Put failure is returned, not swallowed", func(t *testing.T) { + c := fake.NewClientBuilder().WithScheme(scheme).Build() + putErr := errors.New("boom: store unavailable") + putter := &fakeScreenshotPutter{err: putErr} + deps := screenshotStoreDeps{ + Client: c, + ArtifactStore: putter, + SessionUID: types.UID("sess-uid-1"), + } + big := bytes.Repeat([]byte{0xEF}, 300*1024) + + ref, err := storeScreenshotArtifact(context.Background(), deps, sess, big) + require.Error(t, err) + assert.Nil(t, ref) + assert.ErrorIs(t, err, putErr) + }) + + t.Run("CR Create failure is returned, not swallowed", func(t *testing.T) { + createErr := errors.New("boom: create refused") + c := fake.NewClientBuilder().WithScheme(scheme). + WithInterceptorFuncs(interceptor.Funcs{ + Create: func(ctx context.Context, cl client.WithWatch, obj client.Object, opts ...client.CreateOption) error { + return createErr + }, + }). + Build() + deps := screenshotStoreDeps{ + Client: c, + SessionUID: types.UID("sess-uid-1"), + } + small := bytes.Repeat([]byte{0x01}, 16) + + ref, err := storeScreenshotArtifact(context.Background(), deps, sess, small) + require.Error(t, err) + assert.Nil(t, ref) + }) + + t.Run("deterministic name/id seams are honored", func(t *testing.T) { + c := fake.NewClientBuilder().WithScheme(scheme).Build() + deps := screenshotStoreDeps{ + Client: c, + ArtifactStore: &fakeScreenshotPutter{}, + SessionUID: types.UID("sess-uid-1"), + NewRenderName: func(session string) string { return "ar-" + session + "-fixed" }, + NewArtifactID: func() string { return "artifact-fixed" }, + } + small := bytes.Repeat([]byte{0x02}, 16) + + ref, err := storeScreenshotArtifact(context.Background(), deps, sess, small) + require.NoError(t, err) + assert.Equal(t, "ar-s1-fixed", ref.RenderName) + assert.Equal(t, "artifact-fixed", ref.ArtifactID) + }) +} diff --git a/pkg/agent/runner/browser_screenshot_wiring_test.go b/pkg/agent/runner/browser_screenshot_wiring_test.go new file mode 100644 index 00000000..2c6e0f04 --- /dev/null +++ b/pkg/agent/runner/browser_screenshot_wiring_test.go @@ -0,0 +1,329 @@ +package runner + +// browser_screenshot_wiring_test.go is the TDD suite for Task 4's D-R3 wiring: +// capture + store + attach a redacted screenshot when a BROWSER interaction is +// published, best-effort and fail-closed, and NEVER for a non-browser +// interaction. It exercises both seams the wiring touches: +// +// - The HOST-BUILDER seam: buildPlanGatePending (the web_host#act acting +// approval) and buildLoginHumanStepPending (request_human_step's +// login_human_step ask) attach payload.Screenshot from h.captureScreenshot +// when it is set and returns a non-nil ref, and leave it nil otherwise. +// - The DISPATCH seam: executeToolContained sets host.captureScreenshot ONLY +// when the dispatched tool.Tool implements browser.ScreenshotProvider with +// a non-nil capturer (D-R3) — proven end to end through the real capture + +// store path (storeScreenshotArtifact over a fake k8s client), a capture +// error (Screenshot stays nil, the ask still publishes, and the failure is +// logged), and a plain tool that never captures at all. +// +// Required assertions (brief + D-R3 ruling): +// +// (i) a browser ask with a working capturer sets payload.Screenshot +// (ii) a capture error leaves Screenshot nil AND the interaction still +// publishes (never blocks the approval), with the failure logged +// (iii) a non-browser tool never invokes any capturer at all + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "log/slog" + "sync" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + "github.com/authzed/openagentprimitives/pkg/agent/tool" + browsertool "github.com/authzed/openagentprimitives/pkg/agent/tool/browser" + "github.com/authzed/openagentprimitives/pkg/authz/guardian/approval" + "github.com/authzed/openagentprimitives/pkg/channels/channelevents" + "github.com/authzed/openagentprimitives/pkg/memory" + "github.com/authzed/openagentprimitives/pkg/platform/pipeline" +) + +// fakeCapturer is a stub browser.BrowserScreenshotCapturer: returns a scripted +// PNG or error and counts calls, so a test can prove it was (or was never) +// invoked. +type fakeCapturer struct { + mu sync.Mutex + calls int + png []byte + err error +} + +func (f *fakeCapturer) CaptureRedacted(_ context.Context) ([]byte, error) { + f.mu.Lock() + defer f.mu.Unlock() + f.calls++ + if f.err != nil { + return nil, f.err + } + return f.png, nil +} + +func (f *fakeCapturer) callCount() int { + f.mu.Lock() + defer f.mu.Unlock() + return f.calls +} + +// --------------------------------------------------------------------------- +// Host-builder seam: buildPlanGatePending / buildLoginHumanStepPending +// --------------------------------------------------------------------------- + +// TestBuildPlanGatePending_AttachesScreenshot_WhenCaptureSucceeds proves the +// web_host#act acting-approval card carries the screenshot the host's +// captureScreenshot closure returns. +func TestBuildPlanGatePending_AttachesScreenshot_WhenCaptureSucceeds(t *testing.T) { + var published channelevents.Envelope + h := planGateHost(t, &published) + ref := &channelevents.AttachmentRef{RenderName: "shot-1", ArtifactID: "art-1", MIME: "image/png", Filename: "page.png", AltText: "current page"} + var calls int + h.captureScreenshot = func(_ context.Context) *channelevents.AttachmentRef { + calls++ + return ref + } + + reqID, err := h.PublishApproval(context.Background(), planPhaseAsk()) + require.NoError(t, err) + pa, ok := h.pending().m[reqID] + require.True(t, ok) + require.NoError(t, pa.onPublish(context.Background())) + + assert.Equal(t, 1, calls, "the plan_phase builder must call captureScreenshot exactly once") + payload := decodeInteractionRequest(t, published) + require.NotNil(t, payload.Screenshot, "a working capturer must attach a screenshot") + assert.Equal(t, "shot-1", payload.Screenshot.RenderName) + assert.Equal(t, "art-1", payload.Screenshot.ArtifactID) +} + +// TestBuildPlanGatePending_NoScreenshot_WhenCaptureReturnsNil proves a capture +// that resolved to nil (the real closure already logged why) leaves +// Screenshot unset — and the card still publishes. +func TestBuildPlanGatePending_NoScreenshot_WhenCaptureReturnsNil(t *testing.T) { + var published channelevents.Envelope + h := planGateHost(t, &published) + h.captureScreenshot = func(_ context.Context) *channelevents.AttachmentRef { return nil } + + reqID, err := h.PublishApproval(context.Background(), planPhaseAsk()) + require.NoError(t, err) + pa, ok := h.pending().m[reqID] + require.True(t, ok) + require.NoError(t, pa.onPublish(context.Background()), "a nil screenshot must never block the publish") + + payload := decodeInteractionRequest(t, published) + assert.Nil(t, payload.Screenshot) +} + +// TestBuildPlanGatePending_NoScreenshot_WhenCaptureScreenshotUnset proves the +// browser-only gate at the builder: a host built for a non-browser call never +// has captureScreenshot set (executeToolContained leaves it nil), so the +// builder attempts no capture at all. +func TestBuildPlanGatePending_NoScreenshot_WhenCaptureScreenshotUnset(t *testing.T) { + var published channelevents.Envelope + h := planGateHost(t, &published) // h.captureScreenshot left nil, as for any non-browser call + + reqID, err := h.PublishApproval(context.Background(), planPhaseAsk()) + require.NoError(t, err) + pa, ok := h.pending().m[reqID] + require.True(t, ok) + require.NoError(t, pa.onPublish(context.Background())) + + payload := decodeInteractionRequest(t, published) + assert.Nil(t, payload.Screenshot) +} + +// TestBuildLoginHumanStepPending_AttachesScreenshot_WhenCaptureSucceeds mirrors +// the plan_phase case for request_human_step's login_human_step ask — the +// OTHER browser-sourced publish site D-R3 unifies via captureBrowserScreenshotFn. +func TestBuildLoginHumanStepPending_AttachesScreenshot_WhenCaptureSucceeds(t *testing.T) { + var published channelevents.Envelope + l := loginHumanStepLoop(approval.New(), func(_, _ string, env channelevents.Envelope) { published = env }) + h := newRunnerHost(l, hostSession{Namespace: "ns", Name: "s"}) + ref := &channelevents.AttachmentRef{RenderName: "shot-2", ArtifactID: "art-2", MIME: "image/png", Filename: "page.png", AltText: "current page"} + h.captureScreenshot = func(_ context.Context) *channelevents.AttachmentRef { return ref } + + reqID, err := h.PublishApproval(context.Background(), loginHumanStepAsk("approve the push")) + require.NoError(t, err) + pa, ok := h.pending().m[reqID] + require.True(t, ok) + require.NoError(t, pa.onPublish(context.Background())) + + payload := decodeInteractionRequest(t, published) + require.NotNil(t, payload.Screenshot, "a working capturer must attach a screenshot to the login_human_step card") + assert.Equal(t, "shot-2", payload.Screenshot.RenderName) +} + +// TestBuildLoginHumanStepPending_NoScreenshot_WhenCaptureScreenshotUnset proves +// the same browser-only gate for the login_human_step ask. +func TestBuildLoginHumanStepPending_NoScreenshot_WhenCaptureScreenshotUnset(t *testing.T) { + var published channelevents.Envelope + l := loginHumanStepLoop(approval.New(), func(_, _ string, env channelevents.Envelope) { published = env }) + h := newRunnerHost(l, hostSession{Namespace: "ns", Name: "s"}) + + reqID, err := h.PublishApproval(context.Background(), loginHumanStepAsk("approve the push")) + require.NoError(t, err) + pa, ok := h.pending().m[reqID] + require.True(t, ok) + require.NoError(t, pa.onPublish(context.Background())) + + payload := decodeInteractionRequest(t, published) + assert.Nil(t, payload.Screenshot) +} + +// --------------------------------------------------------------------------- +// Dispatch seam: executeToolContained's browser.ScreenshotProvider wiring +// --------------------------------------------------------------------------- + +// fakeBrowserDispatchTool is a fakeDispatchTool that ALSO implements +// browser.ScreenshotProvider, standing in for a real *browserTool (whose +// ScreenshotCapturer() returns its live *Session) without pulling in chromedp. +type fakeBrowserDispatchTool struct { + fakeDispatchTool + capturer browsertool.BrowserScreenshotCapturer +} + +func (f *fakeBrowserDispatchTool) ScreenshotCapturer() browsertool.BrowserScreenshotCapturer { + return f.capturer +} + +var _ browsertool.ScreenshotProvider = (*fakeBrowserDispatchTool)(nil) + +// screenshotAskHook publishes a fixed plan_phase ApprovalAsk at PreToolCall — +// the same host.PublishApproval → buildPlanGatePending machinery the real +// plan-gate hook drives, without reproducing its ceiling/slot logic, since +// this suite only cares whether the published payload's Screenshot field +// reflects what executeToolContained wired onto the host. The ask's own +// Timeout is set short so AwaitDecision resolves (as a fail-closed timeout) +// quickly — the test only inspects what was PUBLISHED, not the outcome. +type screenshotAskHook struct{} + +func (screenshotAskHook) Name() string { return "test_screenshot_ask_hook" } +func (screenshotAskHook) Points() []pipeline.Point { return []pipeline.Point{pipeline.PreToolCall} } +func (screenshotAskHook) Eval(_ context.Context, in pipeline.Input) pipeline.Decision { + if in.Point != pipeline.PreToolCall { + return pipeline.Decision{} + } + ask := planPhaseAsk() + ask.Timeout = 20 * time.Millisecond + return pipeline.Decision{Verdict: pipeline.Allow, Approval: &ask} +} + +// screenshotDispatchLoop builds a minimal Loop that can run executeToolContained +// through a plan_phase ask (resolving approvers and publishing), with Client +// wired to a fake k8s client so the full capture→store→attach path can +// complete (storeScreenshotArtifact creates an ArtifactRender CR). +func screenshotDispatchLoop(t *testing.T, tools []tool.Tool, published *channelevents.Envelope) *Loop { + t.Helper() + l := &Loop{ + Tools: tools, + SessionKey: memory.NamespacedName{Namespace: "ns", Name: "s"}, + Status: LocalStatusPatcher(), + Approval: approval.New(), + ChannelKind: "slack", + AgentName: "demo-agent", + Client: fake.NewClientBuilder().WithScheme(browserScreenshotTestScheme(t)).Build(), + SpiceDBLookupSubjects: func(_ context.Context, _, _ string) ([]string, error) { + return []string{"user:owner@example.com"}, nil + }, + InteractionRequestPublish: func(_ context.Context, _, _ string, env channelevents.Envelope) error { + *published = env + return nil + }, + } + reg := pipeline.NewRegistry() + reg.Register(screenshotAskHook{}, 10) + loopWithInjectedExecutor(t, l, reg) + return l +} + +// TestExecuteToolContained_AttachesScreenshot_ForScreenshotProviderToolWithWorkingCapturer +// is requirement (i): dispatching a tool that implements browser.ScreenshotProvider +// with a working capturer publishes a plan_phase ask whose payload carries the +// screenshot, through the REAL capture→store→attach path (no stubbed storage). +func TestExecuteToolContained_AttachesScreenshot_ForScreenshotProviderToolWithWorkingCapturer(t *testing.T) { + cap := &fakeCapturer{png: bytes.Repeat([]byte{0xAB}, 128)} // well under the inline cap + ft := &fakeBrowserDispatchTool{ + fakeDispatchTool: fakeDispatchTool{name: "browser_click", kind: tool.KindMCP, result: tool.Result{Content: "ok"}}, + capturer: cap, + } + var published channelevents.Envelope + l := screenshotDispatchLoop(t, []tool.Tool{ft}, &published) + sess := &tool.SessionContext{Namespace: "ns", Name: "s"} + + _, _ = l.executeToolContained(context.Background(), sess, ft, "browser_click", json.RawMessage(`{}`), "tu-1", "", containParams{}) + + assert.Equal(t, 1, cap.callCount(), "the dispatched tool's capturer must be invoked exactly once") + payload := decodeInteractionRequest(t, published) + require.NotNil(t, payload.Screenshot, "a ScreenshotProvider tool with a working capturer must attach a screenshot") + assert.Equal(t, "image/png", payload.Screenshot.MIME) +} + +// TestExecuteToolContained_NoScreenshot_WhenCaptureErrors_PublishStillHappens +// is requirement (ii): a capture error leaves Screenshot nil, the ask still +// publishes (an approval must never be blocked by a screenshot failure), and +// the failure is logged. +func TestExecuteToolContained_NoScreenshot_WhenCaptureErrors_PublishStillHappens(t *testing.T) { + logs := &guardedBuf{} + prevLogger := slog.Default() + slog.SetDefault(slog.New(slog.NewTextHandler(logs, nil))) + t.Cleanup(func() { slog.SetDefault(prevLogger) }) + + cap := &fakeCapturer{err: errors.New("cdp: target closed")} + ft := &fakeBrowserDispatchTool{ + fakeDispatchTool: fakeDispatchTool{name: "browser_click", kind: tool.KindMCP, result: tool.Result{Content: "ok"}}, + capturer: cap, + } + var published channelevents.Envelope + l := screenshotDispatchLoop(t, []tool.Tool{ft}, &published) + sess := &tool.SessionContext{Namespace: "ns", Name: "s"} + + _, _ = l.executeToolContained(context.Background(), sess, ft, "browser_click", json.RawMessage(`{}`), "tu-1", "", containParams{}) + + assert.Equal(t, 1, cap.callCount(), "the capturer must still be invoked (and fail)") + payload := decodeInteractionRequest(t, published) + assert.Nil(t, payload.Screenshot, "a capture failure must never attach a screenshot") + assert.Contains(t, logs.String(), "browser screenshot: capture failed", "a capture failure must be logged with context") + assert.Contains(t, logs.String(), "cdp: target closed") +} + +// TestExecuteToolContained_NeverCaptures_ForNonScreenshotProviderTool is +// requirement (iii): a plain tool.Tool (no browser.ScreenshotProvider) never +// gets host.captureScreenshot set, so the SAME ask-publishing hook that +// attaches a screenshot for a browser tool attaches nothing here. +func TestExecuteToolContained_NeverCaptures_ForNonScreenshotProviderTool(t *testing.T) { + ft := &fakeDispatchTool{name: "read_file", kind: tool.KindMCP, result: tool.Result{Content: "ok"}} + var published channelevents.Envelope + l := screenshotDispatchLoop(t, []tool.Tool{ft}, &published) + sess := &tool.SessionContext{Namespace: "ns", Name: "s"} + + _, _ = l.executeToolContained(context.Background(), sess, ft, "read_file", json.RawMessage(`{}`), "tu-1", "", containParams{}) + + payload := decodeInteractionRequest(t, published) + assert.Nil(t, payload.Screenshot, "a non-browser tool must never attach a screenshot") +} + +// guardedBuf is a mutex-guarded io.Writer for slog output, mirroring +// failClosedLog (loop_precondition_failclosed_test.go): slog.SetDefault makes +// the handler reachable from any goroutine still running in the package's +// suite, so a bare bytes.Buffer would race under -race. +type guardedBuf struct { + mu sync.Mutex + buf bytes.Buffer +} + +func (g *guardedBuf) Write(p []byte) (int, error) { + g.mu.Lock() + defer g.mu.Unlock() + return g.buf.Write(p) +} + +func (g *guardedBuf) String() string { + g.mu.Lock() + defer g.mu.Unlock() + return g.buf.String() +} diff --git a/pkg/agent/runner/host.go b/pkg/agent/runner/host.go index 4f9c4ed2..5d98dee5 100644 --- a/pkg/agent/runner/host.go +++ b/pkg/agent/runner/host.go @@ -9,6 +9,7 @@ import ( "time" spiceboxv1alpha1 "github.com/authzed/openagentprimitives/pkg/apis/v1alpha1" + "github.com/authzed/openagentprimitives/pkg/channels/channelevents" "github.com/authzed/openagentprimitives/pkg/memory" "github.com/authzed/openagentprimitives/pkg/memory/kinds/infoleakageaudit" "github.com/authzed/openagentprimitives/pkg/platform/pipeline" @@ -91,6 +92,18 @@ type runnerHost struct { // containParams.approvalEvent and never mutated after — fixed for the host's // lifetime, like proxyExec above. approvalEvent func(approvalObservation) + + // captureScreenshot, when non-nil, captures a redacted screenshot of the + // live browser page and stores it as an ArtifactRender, returning the + // AttachmentRef to attach to a browser interaction's payload (or nil on + // ANY capture/store failure — best-effort, never blocks the approval). + // Set once in executeToolContained ONLY when the dispatched tool + // implements browser.ScreenshotProvider with a non-nil capturer (D-R3) — + // so it is nil for every non-browser tool call, which is the browser-only + // gate: buildPlanGatePending / buildLoginHumanStepPending attach a + // screenshot only when this is set. Never mutated after being set, like + // approvalEvent above. + captureScreenshot func(context.Context) *channelevents.AttachmentRef } // setColdStartPlacement records the SessionStart hook's turn-0 placement diff --git a/pkg/agent/runner/host_approval.go b/pkg/agent/runner/host_approval.go index 213e645e..56d2b9bb 100644 --- a/pkg/agent/runner/host_approval.go +++ b/pkg/agent/runner/host_approval.go @@ -1237,6 +1237,14 @@ func (h *runnerHost) buildLoginHumanStepPending(ctx context.Context, reqID strin ExpiresAt: expiresAtPtr(hostNow().Add(h.loginHumanStepTTL())), Interruptible: true, } + // Best-effort redacted screenshot — see buildPlanGatePending's identical + // gate; request_human_step is itself a browser tool (D-R3), so the same + // host.captureScreenshot closure is set for this ask kind too. + if h.captureScreenshot != nil { + if ref := h.captureScreenshot(ctx); ref != nil { + pl.Screenshot = ref + } + } env, eerr := channelevents.BuildEnvelope(sessNS, sessName, channelevents.KindInteractionRequest, pl) if eerr != nil { return nil, fmt.Errorf("build login_human_step interaction envelope: %w", eerr) @@ -2203,6 +2211,17 @@ func (h *runnerHost) buildPlanGatePending( ExpiresAt: expiresAtPtr(hostNow().Add(h.l.resolvedApprovalTimeout())), Interruptible: false, } + // Best-effort redacted screenshot: h.captureScreenshot is set ONLY when + // THIS call's tool implements browser.ScreenshotProvider with a live + // capturer (D-R3, see executeToolContained), so a non-browser plan_phase + // ask leaves it nil and never attempts a capture. Any capture/store + // failure already returned nil (logged at the closure) — only attach when + // non-nil. + if h.captureScreenshot != nil { + if ref := h.captureScreenshot(ctx); ref != nil { + pl.Screenshot = ref + } + } env, eerr := channelevents.BuildEnvelope(sessNS, sessName, channelevents.KindInteractionRequest, pl) if eerr != nil { return nil, fmt.Errorf("build %s interaction envelope: %w", category, eerr) diff --git a/pkg/agent/runner/humanstep_deps.go b/pkg/agent/runner/humanstep_deps.go index c1ecdf0a..f011028e 100644 --- a/pkg/agent/runner/humanstep_deps.go +++ b/pkg/agent/runner/humanstep_deps.go @@ -19,6 +19,7 @@ import ( "time" browsertool "github.com/authzed/openagentprimitives/pkg/agent/tool/browser" + "github.com/authzed/openagentprimitives/pkg/channels/channelevents" "github.com/authzed/openagentprimitives/pkg/platform/pipeline" ) @@ -56,12 +57,40 @@ func NewHumanStepRequester(host approvalHost, loop *Loop, timeout time.Duration) return &humanStepRequester{binder: staticHost{h: host}, loopFn: func() *Loop { return loop }, timeout: timeout} } +// wireScreenshot attaches cap to host's captureScreenshot field, best-effort: +// if host doesn't happen to be a *runnerHost (a test fake need not implement +// it) or s.loopFn resolves no live Loop, it silently does nothing and the +// login_human_step card simply carries no screenshot — never an error, since +// nothing here can fail the round-trip. +// +// This is the SAME capturer wiring executeToolContained does for the +// web_host#act acting-approval (D-R3: request_human_step is itself a browser +// tool, so both call sites share captureBrowserScreenshotFn — one Loop +// method, two *runnerHost values, because hostBinder deliberately binds a +// FRESH host per round-trip (see its doc) rather than reusing +// executeToolContained's). +func (s *humanStepRequester) wireScreenshot(host approvalHost, cap browsertool.BrowserScreenshotCapturer) { + if cap == nil { + return + } + rh, ok := host.(*runnerHost) + if !ok { + return + } + l := s.loopFn() + if l == nil { + return + } + rh.captureScreenshot = l.captureBrowserScreenshotFn( + channelevents.SessionRef{Namespace: rh.sess.Namespace, Name: rh.sess.Name}, cap) +} + // Confirm implements browser.HumanStepRequester: bind the ONE host this // round-trip runs against, publish the login_human_step card, then block for the // decision ON THAT SAME HOST. It writes nothing — the approved bool alone tells // request_human_step whether the person completed the step. A timeout surfaces as // TimedOut (fail-closed: never a silent success). -func (s *humanStepRequester) Confirm(ctx context.Context, prompt string) (browsertool.HumanStepOutcome, error) { +func (s *humanStepRequester) Confirm(ctx context.Context, prompt string, capturer browsertool.BrowserScreenshotCapturer) (browsertool.HumanStepOutcome, error) { if s.binder == nil { return browsertool.HumanStepOutcome{}, fmt.Errorf("human step unavailable (session not ready)") } @@ -69,6 +98,7 @@ func (s *humanStepRequester) Confirm(ctx context.Context, prompt string) (browse if err != nil { return browsertool.HumanStepOutcome{}, err } + s.wireScreenshot(host, capturer) reqID, err := host.PublishApproval(ctx, pipeline.ApprovalAsk{ Kind: "login_human_step", @@ -114,7 +144,7 @@ type humanStepCodeHost interface { // expects a free-text answer back), block for the decision ON THE SAME HOST, then // read the human's typed code off that host. Writes nothing; the caller types the // code into the page. A timeout is fail-closed (TimedOut, no code). -func (s *humanStepRequester) RequestCode(ctx context.Context, prompt string) (browsertool.HumanStepCodeOutcome, error) { +func (s *humanStepRequester) RequestCode(ctx context.Context, prompt string, capturer browsertool.BrowserScreenshotCapturer) (browsertool.HumanStepCodeOutcome, error) { if s.binder == nil { return browsertool.HumanStepCodeOutcome{}, fmt.Errorf("human step unavailable (session not ready)") } @@ -122,6 +152,7 @@ func (s *humanStepRequester) RequestCode(ctx context.Context, prompt string) (br if err != nil { return browsertool.HumanStepCodeOutcome{}, err } + s.wireScreenshot(host, capturer) reqID, err := host.PublishApproval(ctx, pipeline.ApprovalAsk{ Kind: "login_human_step", @@ -167,7 +198,10 @@ func (s *humanStepRequester) RequestCode(ctx context.Context, prompt string) (br // Captcha implements browser.HumanStepRequester: surface the one-way // login_captcha notice on the live Loop. It does not block and writes nothing. -func (s *humanStepRequester) Captcha(ctx context.Context, prompt string) error { +// The capturer (the live browser session) is threaded into the notice +// publisher so the notice carries a redacted screenshot — the surface where a +// screenshot matters MOST, since a person must see the page to take over. +func (s *humanStepRequester) Captcha(ctx context.Context, prompt string, capturer browsertool.BrowserScreenshotCapturer) error { if s.loopFn == nil { return fmt.Errorf("human step captcha unavailable (session not ready)") } @@ -175,5 +209,5 @@ func (s *humanStepRequester) Captcha(ctx context.Context, prompt string) error { if l == nil { return fmt.Errorf("human step captcha unavailable (session not ready)") } - return l.publishLoginCaptchaNotice(ctx, prompt) + return l.publishLoginCaptchaNotice(ctx, prompt, capturer) } diff --git a/pkg/agent/runner/humanstep_deps_test.go b/pkg/agent/runner/humanstep_deps_test.go index c3c440d4..701d5a54 100644 --- a/pkg/agent/runner/humanstep_deps_test.go +++ b/pkg/agent/runner/humanstep_deps_test.go @@ -25,7 +25,7 @@ func TestHumanStepRequester_Confirm_PublishesLoginHumanStepAsk(t *testing.T) { host := &fakeApprovalHost{approved: true, by: "alice-canonical"} req := NewHumanStepRequester(host, nil, time.Second) - _, err := req.Confirm(context.Background(), "approve the push on your phone") + _, err := req.Confirm(context.Background(), "approve the push on your phone", nil) require.NoError(t, err) require.Equal(t, 1, host.publishCall, "Confirm must publish exactly one ask") @@ -69,7 +69,7 @@ func TestHumanStepRequester_Confirm_OutcomeMapping(t *testing.T) { host := &fakeApprovalHost{approved: tc.approved, by: tc.by, timedOut: tc.timedOut} req := NewHumanStepRequester(host, nil, time.Second) - got, err := req.Confirm(context.Background(), "approve the push") + got, err := req.Confirm(context.Background(), "approve the push", nil) require.NoError(t, err) assert.Equal(t, tc.want, got) }) @@ -83,7 +83,7 @@ func TestHumanStepRequester_RequestCode_PublishesLoginHumanStepAsk(t *testing.T) host := &fakeApprovalHost{approved: true, by: "alice-canonical", code: "123456"} req := NewHumanStepRequester(host, nil, time.Second) - _, err := req.RequestCode(context.Background(), "enter the emailed code") + _, err := req.RequestCode(context.Background(), "enter the emailed code", nil) require.NoError(t, err) require.Equal(t, 1, host.publishCall, "RequestCode must publish exactly one ask") @@ -133,7 +133,7 @@ func TestHumanStepRequester_RequestCode_OutcomeMapping(t *testing.T) { host := &fakeApprovalHost{approved: tc.approved, by: tc.by, timedOut: tc.timedOut, code: tc.hostCode} req := NewHumanStepRequester(host, nil, time.Second) - got, err := req.RequestCode(context.Background(), "enter the code") + got, err := req.RequestCode(context.Background(), "enter the code", nil) require.NoError(t, err) assert.Equal(t, tc.want, got) assert.Equal(t, tc.wantTakeCalls, host.takeCodeCall, "the code must be read off the host only on an approval") @@ -147,7 +147,7 @@ func TestHumanStepRequester_RequestCode_PublishErrorPropagates(t *testing.T) { host := &fakeApprovalHost{publishErr: errors.New("no resolvable approvers")} req := NewHumanStepRequester(host, nil, time.Second) - _, err := req.RequestCode(context.Background(), "enter the code") + _, err := req.RequestCode(context.Background(), "enter the code", nil) require.Error(t, err) assert.Contains(t, err.Error(), "no resolvable approvers") assert.Equal(t, 0, host.awaitCall, "AwaitDecision must never run after a publish error") @@ -160,7 +160,7 @@ func TestHumanStepRequester_Confirm_PublishErrorPropagates(t *testing.T) { host := &fakeApprovalHost{publishErr: errors.New("no resolvable approvers")} req := NewHumanStepRequester(host, nil, time.Second) - _, err := req.Confirm(context.Background(), "approve the push") + _, err := req.Confirm(context.Background(), "approve the push", nil) require.Error(t, err) assert.Contains(t, err.Error(), "no resolvable approvers") assert.Equal(t, 0, host.awaitCall, "AwaitDecision must never run after a publish error") @@ -176,7 +176,7 @@ func TestHumanStepRequester_Confirm_ZeroTimeout_PrefersHostResolvedTimeout(t *te } req := NewHumanStepRequester(host, nil, 0) - _, err := req.Confirm(context.Background(), "approve the push") + _, err := req.Confirm(context.Background(), "approve the push", nil) require.NoError(t, err) assert.Equal(t, 42*time.Second, host.gotTimeout) } @@ -187,7 +187,7 @@ func TestHumanStepRequester_Confirm_ZeroTimeout_FallsBackToDefault(t *testing.T) host := &fakeApprovalHost{approved: true} req := NewHumanStepRequester(host, nil, 0) - _, err := req.Confirm(context.Background(), "approve the push") + _, err := req.Confirm(context.Background(), "approve the push", nil) require.NoError(t, err) assert.Equal(t, defaultHumanStepTimeout, host.gotTimeout) } @@ -198,11 +198,11 @@ func TestHumanStepRequester_Confirm_ZeroTimeout_FallsBackToDefault(t *testing.T) func TestNewLateBoundHumanStepRequester_LoopNotReady_FailsClosed(t *testing.T) { req := NewLateBoundHumanStepRequester(func() *Loop { return nil }) - _, err := req.Confirm(context.Background(), "approve the push") + _, err := req.Confirm(context.Background(), "approve the push", nil) require.Error(t, err) assert.Contains(t, err.Error(), "not ready") - cerr := req.Captcha(context.Background(), "there is a captcha") + cerr := req.Captcha(context.Background(), "there is a captcha", nil) require.Error(t, cerr) assert.Contains(t, cerr.Error(), "not ready") } diff --git a/pkg/agent/runner/login_captcha_notice.go b/pkg/agent/runner/login_captcha_notice.go index 782d5088..69eae7c1 100644 --- a/pkg/agent/runner/login_captcha_notice.go +++ b/pkg/agent/runner/login_captcha_notice.go @@ -14,6 +14,7 @@ import ( "fmt" "strings" + browsertool "github.com/authzed/openagentprimitives/pkg/agent/tool/browser" "github.com/authzed/openagentprimitives/pkg/channels/channelevents" "github.com/authzed/openagentprimitives/pkg/channels/channelinteractions/categories" "github.com/authzed/openagentprimitives/pkg/channels/notice" @@ -30,7 +31,13 @@ const maxCaptchaPromptRunes = 500 // (kubectl-driven) has no InteractionRequestPublish wired, which is reported as // an error here and surfaced by request_human_step to the model rather than // swallowed. -func (l *Loop) publishLoginCaptchaNotice(ctx context.Context, prompt string) error { +// +// capturer is the live browser session (nil if none): the notice carries a +// redacted page screenshot so a person taking over to solve the CAPTCHA can +// see the page. Best-effort/fail-closed exactly like the acting-approval and +// login_human_step surfaces — a capture/store failure (or a nil capturer) +// yields no screenshot and the notice still publishes text-only. +func (l *Loop) publishLoginCaptchaNotice(ctx context.Context, prompt string, capturer browsertool.BrowserScreenshotCapturer) error { if l.InteractionRequestPublish == nil { return fmt.Errorf("login captcha notice unavailable (interaction publish hook not configured)") } @@ -51,6 +58,16 @@ func (l *Loop) publishLoginCaptchaNotice(ctx context.Context, prompt string) err if err != nil { return fmt.Errorf("build login captcha notice: %w", err) } + // Best-effort redacted screenshot, same closure the acting-approval and + // login_human_step surfaces use. captureBrowserScreenshotFn needs no host, + // so there is no host to thread through for this notice-only surface; a nil + // capturer or any capture/store failure resolves to nil and the notice + // publishes text-only. + if capturer != nil { + if ref := l.captureBrowserScreenshotFn(sess, capturer)(ctx); ref != nil { + pl.Screenshot = ref + } + } env, err := channelevents.BuildEnvelope(sessNS, sessName, channelevents.KindInteractionRequest, pl) if err != nil { return fmt.Errorf("build login captcha notice envelope: %w", err) diff --git a/pkg/agent/runner/login_captcha_notice_test.go b/pkg/agent/runner/login_captcha_notice_test.go index 90bf679a..5c425551 100644 --- a/pkg/agent/runner/login_captcha_notice_test.go +++ b/pkg/agent/runner/login_captcha_notice_test.go @@ -3,10 +3,12 @@ package runner import ( "context" "encoding/json" + "errors" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "sigs.k8s.io/controller-runtime/pkg/client/fake" "github.com/authzed/openagentprimitives/pkg/channels/channelevents" "github.com/authzed/openagentprimitives/pkg/channels/channelinteractions/categories" @@ -31,7 +33,7 @@ func TestPublishLoginCaptchaNotice_PublishesNotice(t *testing.T) { }, } - err := l.publishLoginCaptchaNotice(context.Background(), report) + err := l.publishLoginCaptchaNotice(context.Background(), report, nil) require.NoError(t, err) require.Equal(t, channelevents.KindInteractionRequest, published.Kind) @@ -58,7 +60,63 @@ func TestPublishLoginCaptchaNotice_NoPublisher_Errors(t *testing.T) { SessionKey: memory.NamespacedName{Namespace: "ns", Name: "s"}, // InteractionRequestPublish deliberately nil. } - err := l.publishLoginCaptchaNotice(context.Background(), "a captcha appeared") + err := l.publishLoginCaptchaNotice(context.Background(), "a captcha appeared", nil) require.Error(t, err) assert.Contains(t, err.Error(), "publish hook") } + +// TestPublishLoginCaptchaNotice_AttachesScreenshot proves that when a screenshot +// capturer succeeds, the CAPTCHA notice's payload carries the screenshot artifact ref. +func TestPublishLoginCaptchaNotice_AttachesScreenshot(t *testing.T) { + var published channelevents.Envelope + l := &Loop{ + Status: LocalStatusPatcher(), + SessionKey: memory.NamespacedName{Namespace: "ns", Name: "s"}, + Client: fake.NewClientBuilder().WithScheme(browserScreenshotTestScheme(t)).Build(), + InteractionRequestPublish: func(_ context.Context, ns, name string, env channelevents.Envelope) error { + assert.Equal(t, "ns", ns) + assert.Equal(t, "s", name) + published = env + return nil + }, + } + + err := l.publishLoginCaptchaNotice(context.Background(), "a captcha appeared", &fakeCapturer{png: []byte("fakepngbytes")}) + require.NoError(t, err) + + require.Equal(t, channelevents.KindInteractionRequest, published.Kind) + var pl channelevents.InteractionRequestPayload + require.NoError(t, json.Unmarshal(published.Payload, &pl)) + + require.NotNil(t, pl.Screenshot, "a working capturer must attach a screenshot") + assert.NotEmpty(t, pl.Screenshot.RenderName) + assert.Equal(t, "image/png", pl.Screenshot.MIME) +} + +// TestPublishLoginCaptchaNotice_CaptureError_PublishesTextOnly proves that +// when a screenshot capturer fails, the CAPTCHA notice still publishes but +// without a screenshot (best-effort capture, fail-closed publish). +func TestPublishLoginCaptchaNotice_CaptureError_PublishesTextOnly(t *testing.T) { + var published channelevents.Envelope + l := &Loop{ + Status: LocalStatusPatcher(), + SessionKey: memory.NamespacedName{Namespace: "ns", Name: "s"}, + Client: fake.NewClientBuilder().WithScheme(browserScreenshotTestScheme(t)).Build(), + InteractionRequestPublish: func(_ context.Context, ns, name string, env channelevents.Envelope) error { + assert.Equal(t, "ns", ns) + assert.Equal(t, "s", name) + published = env + return nil + }, + } + + err := l.publishLoginCaptchaNotice(context.Background(), "a captcha appeared", &fakeCapturer{err: errors.New("capture boom")}) + require.NoError(t, err, "capture error must not fail the notice publish") + + require.Equal(t, channelevents.KindInteractionRequest, published.Kind) + var pl channelevents.InteractionRequestPayload + require.NoError(t, json.Unmarshal(published.Payload, &pl)) + + assert.Nil(t, pl.Screenshot, "a capture error must leave screenshot nil") + assert.NotEmpty(t, pl.Excerpt.Content, "the text notice must still be published") +} diff --git a/pkg/agent/runner/loop_dispatch.go b/pkg/agent/runner/loop_dispatch.go index 580bf4ca..920d9518 100644 --- a/pkg/agent/runner/loop_dispatch.go +++ b/pkg/agent/runner/loop_dispatch.go @@ -14,8 +14,10 @@ import ( "github.com/authzed/openagentprimitives/pkg/agent/runner/approval/summarizer" lifecyclecore "github.com/authzed/openagentprimitives/pkg/agent/session/lifecycle" "github.com/authzed/openagentprimitives/pkg/agent/tool" + browsertool "github.com/authzed/openagentprimitives/pkg/agent/tool/browser" "github.com/authzed/openagentprimitives/pkg/agent/tool/sandbox" "github.com/authzed/openagentprimitives/pkg/authz/toolguard" + "github.com/authzed/openagentprimitives/pkg/channels/channelevents" "github.com/authzed/openagentprimitives/pkg/platform/identity" "github.com/authzed/openagentprimitives/pkg/platform/identity/credupdate" "github.com/authzed/openagentprimitives/pkg/platform/pipeline" @@ -364,6 +366,19 @@ func (l *Loop) executeToolContained( // gate in host_approval.go. host.proxyExec = p.proxyExec host.approvalEvent = p.approvalEvent + // D-R3: web_request_human_step IS a browser tool, so BOTH the acting + // approval (web_host#act, a plan_phase ask) and the human-step asks + // (login_human_step) flow through this one seam — whichever ask kind t + // ends up publishing, the SAME host carries the capturer. A non-browser + // tool doesn't implement ScreenshotProvider, so host.captureScreenshot + // stays nil and no screenshot is ever attempted for it (the browser-only + // gate, enforced here rather than in the payload builders). + if sp, ok := t.(browsertool.ScreenshotProvider); ok { + if cap := sp.ScreenshotCapturer(); cap != nil { + host.captureScreenshot = l.captureBrowserScreenshotFn( + channelevents.SessionRef{Namespace: sess.Namespace, Name: sess.Name}, cap) + } + } preIn := pipeline.Input{ Session: pipeline.SessionRef{ diff --git a/pkg/agent/tool/browser/human_step_test.go b/pkg/agent/tool/browser/human_step_test.go index e454459a..68d7fc73 100644 --- a/pkg/agent/tool/browser/human_step_test.go +++ b/pkg/agent/tool/browser/human_step_test.go @@ -29,19 +29,19 @@ type fakeHumanStep struct { captchaErr error } -func (f *fakeHumanStep) Confirm(_ context.Context, prompt string) (HumanStepOutcome, error) { +func (f *fakeHumanStep) Confirm(_ context.Context, prompt string, _ BrowserScreenshotCapturer) (HumanStepOutcome, error) { f.confirmCalls++ f.confirmPrompt = prompt return f.outcome, f.confirmErr } -func (f *fakeHumanStep) RequestCode(_ context.Context, prompt string) (HumanStepCodeOutcome, error) { +func (f *fakeHumanStep) RequestCode(_ context.Context, prompt string, _ BrowserScreenshotCapturer) (HumanStepCodeOutcome, error) { f.codeCalls++ f.codePrompt = prompt return f.codeOutcome, f.codeErr } -func (f *fakeHumanStep) Captcha(_ context.Context, prompt string) error { +func (f *fakeHumanStep) Captcha(_ context.Context, prompt string, _ BrowserScreenshotCapturer) error { f.captchaCalls++ f.captchaPrompt = prompt return f.captchaErr diff --git a/pkg/agent/tool/browser/humanstep.go b/pkg/agent/tool/browser/humanstep.go index 87789e79..d7b02350 100644 --- a/pkg/agent/tool/browser/humanstep.go +++ b/pkg/agent/tool/browser/humanstep.go @@ -17,7 +17,13 @@ type HumanStepRequester interface { // person presses it, declines, or the approval window lapses — a push approval // the person completes on their phone, say. The outcome tells the tool which // happened; it carries no secret. - Confirm(ctx context.Context, prompt string) (HumanStepOutcome, error) + // + // capturer is the SAME live session's BrowserScreenshotCapturer the caller's + // ScreenshotCapturer() returns (nil if none) — request_human_step is itself a + // browser tool (D-R3), so its login_human_step card attaches a redacted + // screenshot exactly like the web_host#act acting-approval does, best-effort + // and never blocking the ask. + Confirm(ctx context.Context, prompt string, capturer BrowserScreenshotCapturer) (HumanStepOutcome, error) // RequestCode posts a login_human_step card asking the person to type an // emailed/SMS code, and blocks until they answer, decline, or the window lapses. @@ -27,13 +33,20 @@ type HumanStepRequester interface { // credential. The code is NEVER returned as a tool result or shown to the model. // A timeout is fail-closed: TimedOut is true, Code empty, and the caller reports // a failure rather than silently succeeding. - RequestCode(ctx context.Context, prompt string) (HumanStepCodeOutcome, error) + // + // capturer: see Confirm's doc — same best-effort screenshot attachment. + RequestCode(ctx context.Context, prompt string, capturer BrowserScreenshotCapturer) (HumanStepCodeOutcome, error) // Captcha surfaces a one-way notice that a person must take over in the browser // (a CAPTCHA the agent cannot solve, or any step with no actionable kind). It // does NOT block — there is no decision leg; the agent has reported it cannot // proceed, and a human steps in out of band. - Captcha(ctx context.Context, prompt string) error + // + // capturer: see Confirm's doc. The screenshot matters MOST here — a person + // taking over to solve a CAPTCHA must see the page — so the login_captcha + // notice attaches a redacted screenshot exactly like Confirm/RequestCode, + // best-effort and never blocking the notice. + Captcha(ctx context.Context, prompt string, capturer BrowserScreenshotCapturer) error } // HumanStepCodeOutcome is the result of a RequestCode round-trip. Exactly one of diff --git a/pkg/agent/tool/browser/screenshot_capture.go b/pkg/agent/tool/browser/screenshot_capture.go new file mode 100644 index 00000000..63a77622 --- /dev/null +++ b/pkg/agent/tool/browser/screenshot_capture.go @@ -0,0 +1,137 @@ +package browser + +import ( + "context" + "encoding/json" + "fmt" + "image" + "math" + + "github.com/chromedp/cdproto/cdp" + "github.com/chromedp/cdproto/dom" + "github.com/chromedp/cdproto/page" + "github.com/chromedp/cdproto/runtime" +) + +// BrowserScreenshotCapturer captures a redacted viewport screenshot of the +// current page. Injected into the runner's interaction-publish paths; the +// browser session is the only implementation. +type BrowserScreenshotCapturer interface { + CaptureRedacted(ctx context.Context) ([]byte, error) +} + +// ScreenshotProvider is implemented by a tool whose live browser session can +// produce a redacted approval screenshot. The runner's approval-publish path +// (pkg/agent/runner) type-asserts a dispatched tool.Tool for this interface — +// the same optional-interface idiom NamedCallArgs uses (tools.go) to surface +// CurrentHost to the Permission check — rather than threading a capturer +// through a new Loop-level seam. Only *browserTool implements it; a +// non-browser tool therefore never yields a capturer and the runner never +// captures a screenshot for it. +type ScreenshotProvider interface { + // ScreenshotCapturer returns the capturer for this tool's live session, or + // nil if none is available (e.g. a browserTool synthesized without a + // session in a unit test). + ScreenshotCapturer() BrowserScreenshotCapturer +} + +// CaptureRedacted captures the viewport and paints over every credential-filled +// (blanked) field. It is FAIL-CLOSED: if any blanked field's box cannot be +// resolved it returns an error and NO bytes, so the caller ships no screenshot +// rather than one that might reveal a typed secret. +func (s *Session) CaptureRedacted(ctx context.Context) ([]byte, error) { + blanked := s.blankedNodes() + var shot []byte + var boxes []image.Rectangle + runErr := s.do(ctx, snapshotTimeout, func(c context.Context) error { + // Device pixel ratio: CaptureScreenshot pixels are DPR * CSS pixels, + // while GetBoxModel returns CSS pixels, so boxes must be scaled. + var dpr float64 = 1 + if ae := runAction(c, func(ac context.Context) error { + r, _, err := runtime.Evaluate("window.devicePixelRatio").Do(ac) + if err != nil { + // Fail closed: without a trusted DPR we could compute boxes at 1x + // against a 2x screenshot and UNDER-cover the field on a HiDPI + // display. An eval error must ship no screenshot, like a box + // resolution failure — not silently assume 1x. + return fmt.Errorf("redact: read device pixel ratio: %w", err) + } + if r != nil && r.Value != nil { + _ = json.Unmarshal(r.Value, &dpr) // best-effort; a weird value falls back below + } + if dpr <= 0 { + dpr = 1 + } + return nil + }); ae != nil { + return ae + } + for _, id := range blanked { + var box *dom.BoxModel + if ae := runAction(c, func(ac context.Context) error { + var e error + box, e = dom.GetBoxModel().WithBackendNodeID(cdp.BackendNodeID(id)).Do(ac) + return e + }); ae != nil { + // Fail closed: a blanked field we can't locate means we can't + // prove the screenshot is clean. + return fmt.Errorf("redact: box for blanked node %d: %w", id, ae) + } + // An empty rect (a non-nil box with too few quad points) is also a + // resolution failure: we cannot prove what region holds the secret, + // so fail closed rather than ship an unredacted field. + if r := boxFromModelScaled(box, dpr); r.Empty() { + return fmt.Errorf("redact: empty box for blanked node %d", id) + } else { + boxes = append(boxes, r) + } + } + if ae := runAction(c, func(ac context.Context) error { + var e error + shot, e = page.CaptureScreenshot().Do(ac) + return e + }); ae != nil { + return ae + } + return nil + }) + if runErr != nil { + return nil, runErr + } + return redactPNG(shot, boxes) +} + +// boxFromModelScaled converts a CDP content-quad box to an axis-aligned +// rectangle in screenshot pixels. The content quad is [x1,y1,x2,y2,x3,y3,x4,y4]; +// we take the bounding rect and scale by the device pixel ratio. +func boxFromModelScaled(b *dom.BoxModel, dpr float64) image.Rectangle { + if b == nil || len(b.Content) < 8 { + return image.Rectangle{} + } + xs := []float64{b.Content[0], b.Content[2], b.Content[4], b.Content[6]} + ys := []float64{b.Content[1], b.Content[3], b.Content[5], b.Content[7]} + minX, maxX, minY, maxY := xs[0], xs[0], ys[0], ys[0] + for _, x := range xs { + if x < minX { + minX = x + } + if x > maxX { + maxX = x + } + } + for _, y := range ys { + if y < minY { + minY = y + } + if y > maxY { + maxY = y + } + } + // Floor the min corners and ceil the max corners so the rect FULLY ENCLOSES + // the field after DPR scaling — a fractional scaled edge (e.g. 560.4px) must + // never leave a sub-pixel sliver of the credential field unpainted. + return image.Rect( + int(math.Floor(minX*dpr)), int(math.Floor(minY*dpr)), + int(math.Ceil(maxX*dpr)), int(math.Ceil(maxY*dpr)), + ) +} diff --git a/pkg/agent/tool/browser/screenshot_capture_chrome_test.go b/pkg/agent/tool/browser/screenshot_capture_chrome_test.go new file mode 100644 index 00000000..73b05b58 --- /dev/null +++ b/pkg/agent/tool/browser/screenshot_capture_chrome_test.go @@ -0,0 +1,168 @@ +package browser + +// screenshot_capture_chrome_test.go is the real redaction proof against a live +// Chrome (startFixture, chrome_test.go): serve a page with a solid-red text +// input, type a credential into it through the REAL fill path so the field is +// marked blanked, call CaptureRedacted, and prove the field's region is painted +// black in the returned PNG while the rest of the page is untouched. Skipped +// with a reason when no Chrome binary is present (spec §7). + +import ( + "bytes" + "context" + "encoding/json" + "image" + "image/png" + "net/http" + "testing" + "time" + + "github.com/chromedp/cdproto/cdp" + "github.com/chromedp/cdproto/dom" + "github.com/chromedp/cdproto/runtime" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + agenttool "github.com/authzed/openagentprimitives/pkg/agent/tool" + spiceboxv1alpha1 "github.com/authzed/openagentprimitives/pkg/apis/v1alpha1" +) + +// redactSecretPage serves a single text input painted solid red (both +// background and text color) and absolutely positioned at a known spot, large +// enough that its box is unambiguous. A credential typed into it is therefore a +// red rectangle in the screenshot; after redaction it must be a BLACK +// rectangle. The page background is explicit white so a control sample outside +// the field proves the redaction was targeted, not a blanket blackout. +func redactSecretPage() http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte(`
` + + `` + + ``)) + }) +} + +// browserRedactFixture boots the chrome fixture in act mode with a one-entry +// catalog for the fixture site, returning the synthesized tools, the shared +// session (for the blanked set + box resolution), and the site base URL. +func browserRedactFixture(t *testing.T) (tools []agenttool.Tool, sess *Session, siteBase string) { + t.Helper() + fx := startFixture(t, redactSecretPage()) + fx.rt.Spec.Interaction = "act" + fx.rt.IdentityName = "site-identity" + fx.rt.Catalog = []spiceboxv1alpha1.BrowserLoginRef{{ + ID: "login-site", + Label: "Site", + Hosts: []spiceboxv1alpha1.BrowserLoginHost{{Host: "site.test"}}, + }} + ff := &fakeFetcher{} + var err error + tools, err = Synthesize(fx.rt, fx.gateURL, chromeToken, false, ff.fetch(), nil, nil) + require.NoError(t, err) + require.NotEmpty(t, tools) + sess = tools[0].(*browserTool).sess + t.Cleanup(func() { sess.Close() }) + return tools, sess, fx.siteBase +} + +// TestCaptureRedacted_CoversBlankedField is the Task 2 keystone: a credential is +// typed into a field through the real fill path (which marks it blanked), and a +// subsequent CaptureRedacted paints that field's region solid black — proving no +// typed secret survives into the screenshot bytes — while leaving the rest of the +// page untouched. +func TestCaptureRedacted_CoversBlankedField(t *testing.T) { + tools, sess, siteBase := browserRedactFixture(t) + + require.False(t, run(t, toolBy(t, tools, "navigate"), map[string]any{"url": siteBase + "/"}).IsError) + + secretRef := refByName(t, tools, "SecretField") + + // Type a credential placeholder into the field via the REAL fill path; this + // marks the field blanked exactly like any filled credential. + typed := run(t, toolBy(t, tools, "type"), map[string]any{"ref": secretRef, "text": "{{login.site.test.username}}"}) + require.False(t, typed.IsError, "type into secret field: %s", typed.Content) + require.Len(t, sess.blankedNodes(), 1, "the typed-into field must be marked blanked") + + // Resolve the field's screenshot-space rect the same way CaptureRedacted does + // (content box scaled by DPR), so the sample points land inside the redacted + // region on a HiDPI display as well. + entry, ok := sess.lookupRef(secretRef) + require.True(t, ok, "the typed ref must still resolve (no navigation)") + rect := fieldScreenshotRect(t, sess, entry.backendNodeID) + require.False(t, rect.Empty(), "the field box must resolve to a non-empty rect") + + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + shot, err := sess.CaptureRedacted(ctx) + require.NoError(t, err, "CaptureRedacted must succeed with a resolvable blanked field") + require.NotEmpty(t, shot, "a redacted screenshot must have bytes") + + img, err := png.Decode(bytes.NewReader(shot)) + require.NoError(t, err) + + // Every sampled pixel across the field's region must be opaque black — the red + // secret rectangle is gone (the brief's uniform-fill-color assertion). + for _, p := range samplePoints(rect) { + r, g, b, a := img.At(p.X, p.Y).RGBA() + assert.True(t, r == 0 && g == 0 && b == 0 && a == 0xffff, + "field pixel (%d,%d) must be redacted black, got r=%d g=%d b=%d a=%d", p.X, p.Y, r, g, b, a) + } + + // Control: a pixel at the top-left page background, well outside the field, + // must be untouched white — redaction is targeted, not a blanket blackout. + cr, cg, cb, _ := img.At(2, 2).RGBA() + assert.True(t, cr == 0xffff && cg == 0xffff && cb == 0xffff, + "the page background outside the field must be untouched, got r=%d g=%d b=%d", cr, cg, cb) +} + +// fieldScreenshotRect resolves backendNodeID's content box and scales it to +// screenshot pixels exactly as CaptureRedacted does, so a test's sample points +// match the production redacted region regardless of device pixel ratio. +func fieldScreenshotRect(t *testing.T, sess *Session, backendNodeID int64) image.Rectangle { + t.Helper() + var rect image.Rectangle + ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) + defer cancel() + require.NoError(t, sess.do(ctx, shortTimeout, func(c context.Context) error { + var dpr float64 = 1 + if err := runAction(c, func(ac context.Context) error { + r, _, e := runtime.Evaluate("window.devicePixelRatio").Do(ac) + if e == nil && r != nil && r.Value != nil { + _ = json.Unmarshal(r.Value, &dpr) + } + return nil + }); err != nil { + return err + } + var box *dom.BoxModel + if err := runAction(c, func(ac context.Context) error { + var e error + box, e = dom.GetBoxModel().WithBackendNodeID(cdp.BackendNodeID(backendNodeID)).Do(ac) + return e + }); err != nil { + return err + } + rect = boxFromModelScaled(box, dpr) + return nil + })) + return rect +} + +// samplePoints returns the center plus the four inset corners of r, staying a +// few pixels inside each edge to avoid border antialiasing. +func samplePoints(r image.Rectangle) []image.Point { + const inset = 4 + minX, minY := r.Min.X+inset, r.Min.Y+inset + maxX, maxY := r.Max.X-inset-1, r.Max.Y-inset-1 + cx, cy := (r.Min.X+r.Max.X)/2, (r.Min.Y+r.Max.Y)/2 + return []image.Point{ + {X: cx, Y: cy}, + {X: minX, Y: minY}, + {X: maxX, Y: minY}, + {X: minX, Y: maxY}, + {X: maxX, Y: maxY}, + } +} diff --git a/pkg/agent/tool/browser/screenshot_redact.go b/pkg/agent/tool/browser/screenshot_redact.go new file mode 100644 index 00000000..d948a51c --- /dev/null +++ b/pkg/agent/tool/browser/screenshot_redact.go @@ -0,0 +1,38 @@ +package browser + +import ( + "bytes" + "fmt" + "image" + "image/color" + "image/draw" + "image/png" +) + +// redactPNG decodes src (a PNG screenshot), paints each rect solid opaque +// black, and re-encodes. Rectangles are in SCREENSHOT PIXEL coordinates (the +// caller is responsible for device-pixel-ratio scaling). Returns an error on a +// decode/encode failure so the caller can fail closed (ship no screenshot) +// rather than ship a partially-redacted one. +func redactPNG(src []byte, boxes []image.Rectangle) ([]byte, error) { + img, err := png.Decode(bytes.NewReader(src)) + if err != nil { + return nil, fmt.Errorf("redact: decode screenshot: %w", err) + } + // Draw onto an RGBA copy so we can composite regardless of the source type. + out := image.NewRGBA(img.Bounds()) + draw.Draw(out, out.Bounds(), img, img.Bounds().Min, draw.Src) + black := &image.Uniform{color.RGBA{0, 0, 0, 255}} + for _, b := range boxes { + r := b.Intersect(out.Bounds()) + if r.Empty() { + continue + } + draw.Draw(out, r, black, image.Point{}, draw.Src) + } + var buf bytes.Buffer + if err := png.Encode(&buf, out); err != nil { + return nil, fmt.Errorf("redact: re-encode screenshot: %w", err) + } + return buf.Bytes(), nil +} diff --git a/pkg/agent/tool/browser/screenshot_redact_test.go b/pkg/agent/tool/browser/screenshot_redact_test.go new file mode 100644 index 00000000..fc37be74 --- /dev/null +++ b/pkg/agent/tool/browser/screenshot_redact_test.go @@ -0,0 +1,59 @@ +package browser + +import ( + "bytes" + "image" + "image/color" + "image/draw" + "image/png" + "testing" + + "github.com/chromedp/cdproto/dom" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestRedactPNG_CoversBoxesAndLeavesRest(t *testing.T) { + // 20x20 image, a 10x10 red "secret" block at (0,0), blue elsewhere. + img := image.NewRGBA(image.Rect(0, 0, 20, 20)) + red, blue := color.RGBA{255, 0, 0, 255}, color.RGBA{0, 0, 255, 255} + draw.Draw(img, img.Bounds(), &image.Uniform{blue}, image.Point{}, draw.Src) + draw.Draw(img, image.Rect(0, 0, 10, 10), &image.Uniform{red}, image.Point{}, draw.Src) + var buf bytes.Buffer + require.NoError(t, png.Encode(&buf, img)) + + out, err := redactPNG(buf.Bytes(), []image.Rectangle{image.Rect(0, 0, 10, 10)}) + require.NoError(t, err) + got, err := png.Decode(bytes.NewReader(out)) + require.NoError(t, err) + // Inside the box: NOT red anymore (covered). + r, g, b, _ := got.At(5, 5).RGBA() + assert.False(t, r == 0xffff && g == 0 && b == 0, "secret pixel must be covered, got r=%d g=%d b=%d", r, g, b) + // Outside the box: still blue (untouched). + r2, g2, b2, _ := got.At(15, 15).RGBA() + assert.True(t, r2 == 0 && g2 == 0 && b2 == 0xffff, "non-secret pixel must be untouched") +} + +func TestRedactPNG_RejectsBadInput(t *testing.T) { + _, err := redactPNG([]byte("not a png"), nil) + require.Error(t, err) +} + +// Review Focus: a device-pixel-ratio > 1 means CDP screenshot pixels are +// dpr * CSS box pixels, so the redaction rect must be scaled or it covers the +// wrong region. (boxFromModelScaled is implemented in Step 5; write this test +// alongside it.) +func TestBoxFromModelScaled_AppliesDPR(t *testing.T) { + // A CSS box at (10,20)-(30,50); at dpr=2 the screenshot rect is doubled. + b := &dom.BoxModel{Content: []float64{10, 20, 30, 20, 30, 50, 10, 50}} + got := boxFromModelScaled(b, 2) + assert.Equal(t, image.Rect(20, 40, 60, 100), got) + // dpr=1 is identity. + assert.Equal(t, image.Rect(10, 20, 30, 50), boxFromModelScaled(b, 1)) + // A fractional box must round OUTWARD (floor mins, ceil maxs) so the rect + // fully encloses the field — never one pixel short on a scaled edge. + frac := &dom.BoxModel{Content: []float64{5.4, 7.2, 10.6, 7.2, 10.6, 12.8, 5.4, 12.8}} + assert.Equal(t, image.Rect(5, 7, 11, 13), boxFromModelScaled(frac, 1)) + // A degenerate box (too few quad points) is empty → caller fails closed. + assert.True(t, boxFromModelScaled(&dom.BoxModel{}, 1).Empty()) +} diff --git a/pkg/agent/tool/browser/tools.go b/pkg/agent/tool/browser/tools.go index 8bf8d87b..906debc7 100644 --- a/pkg/agent/tool/browser/tools.go +++ b/pkg/agent/tool/browser/tools.go @@ -135,8 +135,21 @@ var ( _ agenttool.OriginTool = (*browserTool)(nil) _ agenttool.Cancellable = (*browserTool)(nil) _ agenttool.NamedCallArgs = (*browserTool)(nil) + _ ScreenshotProvider = (*browserTool)(nil) ) +// ScreenshotCapturer returns this tool's session as a BrowserScreenshotCapturer +// — every tool of a toolbox shares one *Session (see the browserTool doc +// comment), so any op's tool can hand the runner a capturer for the same live +// page. Returns nil only if the tool was synthesized without a session (a unit +// test), never in production wiring. +func (t *browserTool) ScreenshotCapturer() BrowserScreenshotCapturer { + if t.sess == nil { + return nil + } + return t.sess +} + func (t *browserTool) Name() string { return t.name } func (t *browserTool) Kind() agenttool.Kind { return agenttool.KindMCP } func (t *browserTool) Description() string { return t.description } @@ -366,7 +379,7 @@ func (t *browserTool) requestHumanStep(ctx context.Context, kind, prompt, ref st switch kind { case "confirm": - outcome, err := t.humanStep.Confirm(ctx, prompt) + outcome, err := t.humanStep.Confirm(ctx, prompt, t.ScreenshotCapturer()) if err != nil { return errResult("request_human_step failed: %v", err) } @@ -387,7 +400,7 @@ func (t *browserTool) requestHumanStep(ctx context.Context, kind, prompt, ref st case "", "captcha": // No actionable kind: a CAPTCHA, or any step the agent cannot do. Surface a // notice and tell the model a person must step in. - if err := t.humanStep.Captcha(ctx, prompt); err != nil { + if err := t.humanStep.Captcha(ctx, prompt, t.ScreenshotCapturer()); err != nil { return errResult("request_human_step: could not notify anyone that human intervention is needed: %v", err) } return errResult("request_human_step: this step needs a person and cannot be completed automatically; a notice has been sent asking someone to take over in the browser") @@ -405,7 +418,7 @@ func (t *browserTool) requestHumanStepCode(ctx context.Context, prompt, ref stri if ref == "" { return errResult("request_human_step: kind=\"code\" requires a ref naming the input to type the code into — read the page first and pass the code field's ref") } - outcome, err := t.humanStep.RequestCode(ctx, prompt) + outcome, err := t.humanStep.RequestCode(ctx, prompt, t.ScreenshotCapturer()) if err != nil { return errResult("request_human_step failed: %v", err) } diff --git a/pkg/bronzethread/bundle.go b/pkg/bronzethread/bundle.go index dd9b7adf..9bf98827 100644 --- a/pkg/bronzethread/bundle.go +++ b/pkg/bronzethread/bundle.go @@ -795,6 +795,17 @@ type Assertions struct { // pre-runner park never writes. ApprovalPrompts *int `json:"approvalPrompts,omitempty"` + // ApprovalScreenshotAttached asserts that at least one published + // interaction approval prompt carries a non-empty + // InteractionRequestPayload.Screenshot.RenderName — the redacted page + // screenshot a browser acting-approval or human-step card attaches + // (Phase D). False (the default) means "do not check"; this is the one + // place a bundle can prove the screenshot reached the published card + // rather than merely that the capture/store code compiles — a scripted + // reply says the same words whether or not the attachment rode along, so + // this has to read the recorded payload, not the transcript. + ApprovalScreenshotAttached bool `json:"approvalScreenshotAttached,omitempty"` + // AgentReplyNotContains is the negative companion, checked against the SAME // per-turn reply, in order: the reply to turn i must NOT contain // AgentReplyNotContains[i]. The only way a bundle can claim a person did not diff --git a/pkg/channels/channelevents/interaction.go b/pkg/channels/channelevents/interaction.go index d2d8e0da..e3905cc9 100644 --- a/pkg/channels/channelevents/interaction.go +++ b/pkg/channels/channelevents/interaction.go @@ -299,6 +299,16 @@ type InteractionRequestPayload struct { // Details is the category's opaque side-band payload (tool_approval puts // ToolApprovalDetails here). Never rendered as copy by a surface. Details json.RawMessage `json:"details,omitempty"` + // Screenshot, when set, is an internal artifact reference (an ArtifactRender + // CR, resolved by AssetFetcher exactly like an outbound attachment) to a + // REDACTED page screenshot the surface renders with the card. Set ONLY for + // browser-originated interactions (acting-approval, request_human_step, + // captcha); nil for every other interaction, so non-browser payloads are + // unchanged on the wire. It is human-facing only and is NEVER delivered to + // the model. The image is redacted (credential-filled fields painted over) + // in the runner before it is stored — see pkg/agent/runner/browser_screenshot.go. + // +optional + Screenshot *AttachmentRef `json:"screenshot,omitempty"` // Resources names the source resources whose #owner may decide a // DecideResourceOwners interaction (tool_call → resource#owner, info_leakage // → data#owner). Empty ⇒ the session approve-set decides. Persisted onto the diff --git a/pkg/channels/channelevents/interaction_test.go b/pkg/channels/channelevents/interaction_test.go index f1faf567..64b7cf1c 100644 --- a/pkg/channels/channelevents/interaction_test.go +++ b/pkg/channels/channelevents/interaction_test.go @@ -314,6 +314,26 @@ func TestInteractionRequestValidate_ApproversMustBeAddressable(t *testing.T) { } } +func TestInteractionRequestPayload_ScreenshotOmitEmpty(t *testing.T) { + // A payload with no screenshot must NOT serialize a "screenshot" key. + b, err := json.Marshal(InteractionRequestPayload{Category: "tool_call", RequestRef: "r1", Lead: "x"}) + require.NoError(t, err) + assert.NotContains(t, string(b), "screenshot", "non-browser interaction must not carry a screenshot key") + + // A payload WITH a screenshot round-trips the AttachmentRef. + in := InteractionRequestPayload{ + Category: "login_human_step", RequestRef: "r2", Lead: "act on aa.com", + Screenshot: &AttachmentRef{RenderName: "shot-abc", MIME: "image/png", Filename: "page.png", AltText: "page screenshot"}, + } + raw, err := json.Marshal(in) + require.NoError(t, err) + var out InteractionRequestPayload + require.NoError(t, json.Unmarshal(raw, &out)) + require.NotNil(t, out.Screenshot) + assert.Equal(t, "shot-abc", out.Screenshot.RenderName) + assert.Equal(t, "image/png", out.Screenshot.MIME) +} + // InteractionItem's three tier tones and its Hint field must survive a JSON // round trip intact, the same wire-shape guarantee every other field on this // envelope already has — a surface reads Hint from the wire, so a marshaling diff --git a/pkg/channels/channelevents/screenshot.go b/pkg/channels/channelevents/screenshot.go new file mode 100644 index 00000000..7a3368fc --- /dev/null +++ b/pkg/channels/channelevents/screenshot.go @@ -0,0 +1,25 @@ +package channelevents + +// ScreenshotRendererKind and ScreenshotMIME name the ArtifactRenderSpec.Kind +// and MIME type every redacted browser-screenshot AttachmentRef (see the +// Screenshot field on InteractionRequestPayload) is built from — +// pkg/agent/runner/browser_screenshot.go is the sole producer. +// +// Named here rather than spelled as literals in pkg/agent/runner: that +// package is under a structural guard +// (pkg/agent/runner/mime_opinion_guard_test.go) against hardcoded +// image/MIME string literals, because such a literal is normally a SECOND +// OPINION about what a model accepts that pkg/agent/llm/models/models.go +// should own exclusively. This is not that — a screenshot attachment's MIME +// is a channel-delivery concern, never carried to the model — so the fix is +// not a models.go row, it is exactly the guard's own documented escape valve: +// a named constant in the package that owns the concern, referenced by name. +// channelevents owns it because it already defines AttachmentRef and the +// Screenshot field that carries these values. +const ( + // ScreenshotRendererKind matches the registered Kind() of the raster-image + // renderer (pkg/channels/channelassets/image). + ScreenshotRendererKind = "image" + // ScreenshotMIME is the MIME type of the redacted page screenshot PNG. + ScreenshotMIME = "image/png" +) diff --git a/pkg/channels/channelinteractions/textrender.go b/pkg/channels/channelinteractions/textrender.go index bd9d9b04..aae1839a 100644 --- a/pkg/channels/channelinteractions/textrender.go +++ b/pkg/channels/channelinteractions/textrender.go @@ -19,7 +19,13 @@ import ( // decisionURL maps a decision/link_mint action ID to its hosted decision-page // URL. Nil renders those actions as plain labels plus a pointer to respond // from a connected surface. -func RenderText(p channelevents.InteractionRequestPayload, decisionURL func(actionID string) string) string { +// +// screenshotURL resolves p.Screenshot (when set) to a human-facing reference +// — a signed view link, a CLI hint, whatever the calling surface can cleanly +// produce — appended as its own line. Best-effort: a nil callback, or one +// that returns "", renders no line at all; p.Screenshot == nil never calls +// the callback. +func RenderText(p channelevents.InteractionRequestPayload, decisionURL func(actionID string) string, screenshotURL func(*channelevents.AttachmentRef) string) string { var b strings.Builder // Tone marker, for notice categories only. The floor has no colour and no @@ -59,6 +65,12 @@ func RenderText(p channelevents.InteractionRequestPayload, decisionURL func(acti fmt.Fprintf(&b, "%s\n%s\n%s\n", fence, innerText, fence) } + if p.Screenshot != nil && screenshotURL != nil { + if url := screenshotURL(p.Screenshot); url != "" { + fmt.Fprintf(&b, "\n📷 Page screenshot: %s\n", url) + } + } + if len(p.Actions) > 0 { b.WriteString("\n") unlinkable := false diff --git a/pkg/channels/channelinteractions/textrender_notice_test.go b/pkg/channels/channelinteractions/textrender_notice_test.go index 78f29b7f..e7c8690c 100644 --- a/pkg/channels/channelinteractions/textrender_notice_test.go +++ b/pkg/channels/channelinteractions/textrender_notice_test.go @@ -45,7 +45,7 @@ func TestRenderText_NoticeCarriesToneMarkerAndNextStep(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { registerNoticeFixture(t, "demo_notice", tc.tone) - out := RenderText(noticePayload("demo_notice"), nil) + out := RenderText(noticePayload("demo_notice"), nil, nil) assert.Contains(t, out, "**Agent stopped**") assert.Contains(t, out, "**Start a new thread to try again.**", @@ -70,7 +70,7 @@ func TestRenderText_NoticeCarriesToneMarkerAndNextStep(t *testing.T) { // respond to. func TestRenderText_NoticeHasNoRespondTrailer(t *testing.T) { registerNoticeFixture(t, "demo_notice", ToneRoutine) - out := RenderText(noticePayload("demo_notice"), nil) + out := RenderText(noticePayload("demo_notice"), nil, nil) assert.NotContains(t, out, "respond from a connected app") } @@ -81,7 +81,7 @@ func TestRenderText_PromptRenderingUnchanged(t *testing.T) { t.Cleanup(Reset) Register(Category{Name: "tool_approval", Deciders: DecideOwner, Tone: ToneRoutine, Resurface: ResurfaceNone}) - out := RenderText(renderableRequest(), nil) + out := RenderText(renderableRequest(), nil, nil) assert.True(t, len(out) > 0) assert.NotContains(t, out, "[!]") assert.NotContains(t, out, "[degraded]") diff --git a/pkg/channels/channelinteractions/textrender_screenshot_test.go b/pkg/channels/channelinteractions/textrender_screenshot_test.go new file mode 100644 index 00000000..0be0bafd --- /dev/null +++ b/pkg/channels/channelinteractions/textrender_screenshot_test.go @@ -0,0 +1,81 @@ +package channelinteractions + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/authzed/openagentprimitives/pkg/channels/channelevents" +) + +// TestRenderText_Screenshot covers the four cases the browser-approval +// screenshot line must handle on the plain-text floor: a wired callback that +// resolves a URL, a wired callback that returns "" (e.g. minting failed or +// isn't configured), no Screenshot on the payload at all, and a nil callback. +// In every case but the first, no "Page screenshot:" line may appear — this +// is a best-effort addition to the card, never something that can block or +// alter the rest of it. +func TestRenderText_Screenshot(t *testing.T) { + withScreenshot := renderableRequest() + withScreenshot.Screenshot = &channelevents.AttachmentRef{ + RenderName: "shot-abc", + ArtifactID: "artifact-xyz", + MIME: channelevents.ScreenshotMIME, + Filename: "page.png", + AltText: "page screenshot", + } + withoutScreenshot := renderableRequest() + + cases := []struct { + name string + payload channelevents.InteractionRequestPayload + screenshotURL func(*channelevents.AttachmentRef) string + wantLine bool + wantURL string + }{ + { + name: "screenshot present, callback resolves a URL: line appears with the URL", + payload: withScreenshot, + screenshotURL: func(ref *channelevents.AttachmentRef) string { + return "https://example/view?d=x" + }, + wantLine: true, + wantURL: "https://example/view?d=x", + }, + { + name: "screenshot present, callback returns empty string: no line", + payload: withScreenshot, + screenshotURL: func(ref *channelevents.AttachmentRef) string { + return "" + }, + wantLine: false, + }, + { + name: "no screenshot on the payload: callback is never consulted, no line", + payload: withoutScreenshot, + screenshotURL: func(ref *channelevents.AttachmentRef) string { + return "should-not-appear" + }, + wantLine: false, + }, + { + name: "screenshot present, nil callback: no panic, no line", + payload: withScreenshot, + screenshotURL: nil, + wantLine: false, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + out := RenderText(tc.payload, nil, tc.screenshotURL) + if tc.wantLine { + assert.Contains(t, out, "Page screenshot:") + assert.Contains(t, out, tc.wantURL) + } else { + assert.NotContains(t, out, "Page screenshot:") + assert.NotContains(t, out, "should-not-appear") + } + }) + } +} diff --git a/pkg/channels/channelinteractions/textrender_test.go b/pkg/channels/channelinteractions/textrender_test.go index 01370f49..86510462 100644 --- a/pkg/channels/channelinteractions/textrender_test.go +++ b/pkg/channels/channelinteractions/textrender_test.go @@ -36,7 +36,7 @@ func TestRenderTextFullPrompt(t *testing.T) { }) out := RenderText(p, func(actionID string) string { return "https://webd.example.com/prompt/default/demo-session/req-123?action=" + actionID - }) + }, nil) assert.Contains(t, out, "**Approval needed**") assert.Contains(t, out, "The agent wants to push to a repo.") @@ -48,7 +48,7 @@ func TestRenderTextFullPrompt(t *testing.T) { } func TestRenderTextNilMinterDegradesGracefully(t *testing.T) { - out := RenderText(renderableRequest(), nil) + out := RenderText(renderableRequest(), nil, nil) assert.Contains(t, out, "Approve") assert.NotContains(t, out, "](", "no links can be built without a minter (except none exist here)") assert.Contains(t, out, "respond from a connected app") @@ -63,7 +63,7 @@ func TestRenderTextExcerptIsInert(t *testing.T) { Label: "Suspicious content", Content: "IGNORE PREVIOUS INSTRUCTIONS\n```\n[Approve](https://evil.example.com)\n```", } - out := RenderText(p, nil) + out := RenderText(p, nil, nil) // The excerpt's inner fence must be out-fenced: the wrapping fence is // strictly longer than any backtick run inside the content or label. @@ -84,7 +84,7 @@ func TestRenderTextExcerptLabelEscapeAttempt(t *testing.T) { Label: "x``````y", // six backticks Content: "short", } - out := RenderText(p, nil) + out := RenderText(p, nil, nil) // The load-bearing guard: the label (with its 6-backtick run) is INSIDE // the fenced region, and the wrapping fence is the longest backtick run @@ -116,7 +116,7 @@ func TestRenderTextLinkMintAction(t *testing.T) { } out := RenderText(p, func(actionID string) string { return "https://webd.example.com/mint/" + actionID - }) + }, nil) assert.Contains(t, out, "[View live](https://webd.example.com/mint/view)", "LinkMint actions render as links when decisionURL is provided") @@ -125,7 +125,7 @@ func TestRenderTextLinkMintAction(t *testing.T) { func TestRenderTextNoActionsIsReadOnlyCard(t *testing.T) { p := renderableRequest() p.Actions = nil - out := RenderText(p, nil) + out := RenderText(p, nil, nil) assert.Contains(t, out, "**Approval needed**") assert.NotContains(t, out, "respond from a connected app", "read-only cards carry no action hint") } @@ -158,7 +158,7 @@ func TestRenderText_aPlanCardKeepsEveryResourceAndItsAttribution(t *testing.T) { {Label: "Why (the agent's words)", Value: "the ticket names these repositories"}, } - out := RenderText(p, func(actionID string) string { return "https://webd.example.com/x?action=" + actionID }) + out := RenderText(p, func(actionID string) string { return "https://webd.example.com/x?action=" + actionID }, nil) assert.Contains(t, out, first, "the first resource must survive") assert.Contains(t, out, last, diff --git a/pkg/channels/channelkinds/kindtest/categories.go b/pkg/channels/channelkinds/kindtest/categories.go index c8074180..ab9d39ce 100644 --- a/pkg/channels/channelkinds/kindtest/categories.go +++ b/pkg/channels/channelkinds/kindtest/categories.go @@ -54,7 +54,7 @@ func AssertCategoryConformance(t *testing.T, c channelinteractions.Category) { require.NoError(t, c.Validate(), "category %q must validate", c.Name) req := SampleRequest(c) require.NoError(t, req.Validate(), "category %q sample payload must be wire-valid", c.Name) - out := channelinteractions.RenderText(req, nil) + out := channelinteractions.RenderText(req, nil, nil) assert.NotEmpty(t, out, "category %q must render on the text floor", c.Name) assert.Contains(t, out, req.Lead, "text floor must carry the lead") } diff --git a/pkg/channels/channelkinds/kindtest/excerpt_inertness_test.go b/pkg/channels/channelkinds/kindtest/excerpt_inertness_test.go index d39333f9..0f5b6320 100644 --- a/pkg/channels/channelkinds/kindtest/excerpt_inertness_test.go +++ b/pkg/channels/channelkinds/kindtest/excerpt_inertness_test.go @@ -33,7 +33,7 @@ func TestExcerptInertness_TextFloor(t *testing.T) { if req.Excerpt == nil { t.Skip("category has no excerpt") } - out := channelinteractions.RenderText(req, nil) + out := channelinteractions.RenderText(req, nil, nil) // Find the longest backtick run in the whole rendered output — that // run is the wrapping fence. It must exist, and its opening/closing diff --git a/pkg/channels/channelkinds/local/host.go b/pkg/channels/channelkinds/local/host.go index 5e694268..a0d640eb 100644 --- a/pkg/channels/channelkinds/local/host.go +++ b/pkg/channels/channelkinds/local/host.go @@ -188,7 +188,11 @@ func (h *Host) SubChannelSenderFor(_ context.Context, sess *spiceboxv1alpha1.Age minter: h.deps.AgentUIMinter, }, nil case "interaction": - return &interactionSender{sink: h.sink}, nil + return &interactionSender{ + sink: h.sink, + minter: h.deps.ArtifactViewMinter, + principal: h.principal, + }, nil default: return nil, nil } diff --git a/pkg/channels/channelkinds/local/interaction.go b/pkg/channels/channelkinds/local/interaction.go index 21512e2e..e779a620 100644 --- a/pkg/channels/channelkinds/local/interaction.go +++ b/pkg/channels/channelkinds/local/interaction.go @@ -5,9 +5,12 @@ import ( "encoding/json" "fmt" + "sigs.k8s.io/controller-runtime/pkg/log" + "github.com/authzed/openagentprimitives/pkg/channels/channelevents" "github.com/authzed/openagentprimitives/pkg/channels/channelinteractions" "github.com/authzed/openagentprimitives/pkg/channels/channelkinds" + "github.com/authzed/openagentprimitives/pkg/platform/identity" ) // interactionSender is the host-side "interaction" sub-channel sender: @@ -18,6 +21,15 @@ import ( // floor is used for the read-only (link / notice) path where no modal opens. type interactionSender struct { sink *inertSink + // minter and principal mint the screenshot reference link RenderText + // appends for a browser-originated interaction (see screenshotURL below). + // Both are the same ArtifactViewMinter/Principal pair host.go already wires + // into liveViewOfferSender for the sibling "live_view_offer" sub-channel — + // nil minter (no webd base URL configured) degrades to no line, never an + // error: unlike a live-view offer, the screenshot line is a side note on a + // card whose Lead/Actions must still be delivered. + minter channelkinds.ArtifactViewMinter + principal identity.Principal } func (s *interactionSender) emitErr(sess channelkinds.SessionInfo, k channelevents.Kind, err error) (channelkinds.SubChannelSendResult, error) { @@ -25,7 +37,7 @@ func (s *interactionSender) emitErr(sess channelkinds.SessionInfo, k channeleven return channelkinds.SubChannelSendResult{}, fmt.Errorf("local interaction sender (%s): %w", k, err) } -func (s *interactionSender) Send(_ context.Context, sess channelkinds.SessionInfo, env channelevents.Envelope) (channelkinds.SubChannelSendResult, error) { +func (s *interactionSender) Send(ctx context.Context, sess channelkinds.SessionInfo, env channelevents.Envelope) (channelkinds.SubChannelSendResult, error) { switch env.Kind { case channelevents.KindInteractionRequest: var pl channelevents.InteractionRequestPayload @@ -42,7 +54,7 @@ func (s *interactionSender) Send(_ context.Context, sess channelkinds.SessionInf // render as plain labels plus a note to respond from a connected app. // The rendered Text is the read-only timeline floor; Payload carries the // structured actions the TUI's decision modal binds keys to. - text := channelinteractions.RenderText(pl, nil) + text := channelinteractions.RenderText(pl, nil, s.screenshotURL(ctx, sess)) s.sink.Emit(MsgInteractionRequest{Session: refOf(sess), Payload: pl, Text: text}) return channelkinds.SubChannelSendResult{}, nil @@ -77,3 +89,25 @@ func (s *interactionSender) Send(_ context.Context, sess channelkinds.SessionInf return s.emitErr(sess, env.Kind, fmt.Errorf("unsupported envelope kind for the interaction sender")) } } + +// screenshotURL returns the per-call closure RenderText consults to resolve a +// browser-originated interaction's redacted screenshot to a human-facing +// link. Best-effort, matching the AGENTS.md "never silently drop errors" +// rule with a log line rather than a failure: the TUI's decision modal and +// the rest of the card must still render if minting fails or is unconfigured, +// so a mint error here is a skipped line, never a dropped card. +func (s *interactionSender) screenshotURL(ctx context.Context, sess channelkinds.SessionInfo) func(*channelevents.AttachmentRef) string { + return func(ref *channelevents.AttachmentRef) string { + if s.minter == nil || ref == nil || ref.ArtifactID == "" { + return "" + } + sessionRef := sess.Namespace + "/" + sess.Name + url, err := s.minter.MintArtifactViewLink(ref.ArtifactID, sessionRef, s.principal, "") + if err != nil { + log.FromContext(ctx).Info("local interaction sender: mint screenshot view link failed (best-effort, card still delivered)", + "session", sessionRef, "artifactID", ref.ArtifactID, "err", err.Error()) + return "" + } + return url + } +} diff --git a/pkg/channels/channelkinds/local/interaction_screenshot_test.go b/pkg/channels/channelkinds/local/interaction_screenshot_test.go new file mode 100644 index 00000000..55d92a6d --- /dev/null +++ b/pkg/channels/channelkinds/local/interaction_screenshot_test.go @@ -0,0 +1,91 @@ +package local + +import ( + "context" + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/authzed/openagentprimitives/pkg/channels/channelevents" + "github.com/authzed/openagentprimitives/pkg/platform/identity" +) + +func screenshotRequestEnv(t *testing.T) channelevents.Envelope { + t.Helper() + return mustEnv(t, channelevents.KindInteractionRequest, + channelevents.InteractionRequestPayload{ + AgentSessionRef: channelevents.SessionRef{Namespace: "default", Name: "s1"}, + Category: "acting_approval", + RequestRef: "act-1", + Lead: "Approve this browser action?", + Screenshot: &channelevents.AttachmentRef{ + RenderName: "shot-abc", + ArtifactID: "artifact-xyz", + MIME: channelevents.ScreenshotMIME, + Filename: "page.png", + }, + Audience: channelevents.InteractionAudience{ + Scope: channelevents.AudienceRequester, + Requester: &channelevents.ExternalIdentity{Kind: "user", ExternalID: "user-1"}, + }, + }) +} + +// TestInteractionSender_Screenshot_WiredMinter_EmitsLinkLine verifies that +// when a browser-originated interaction carries a Screenshot and the sender +// has a working ArtifactViewMinter + principal wired (as host.go wires for +// the "interaction" sub-channel, mirroring live_view_offer's), the rendered +// Text includes the screenshot reference line with the minted URL. +func TestInteractionSender_Screenshot_WiredMinter_EmitsLinkLine(t *testing.T) { + sink := &RecordingSink{} + s := &interactionSender{ + sink: newInertSink(sink), + minter: &fakeMinter{url: "https://webd.example.com/artifact-view?d=abc&sig=def"}, + principal: identity.RawSubject("user:alice"), + } + + _, err := s.Send(context.Background(), sessInfo(), screenshotRequestEnv(t)) + require.NoError(t, err) + require.Len(t, sink.Events(), 1) + m, ok := sink.Events()[0].(MsgInteractionRequest) + require.True(t, ok, "want MsgInteractionRequest, got %T", sink.Events()[0]) + assert.Contains(t, m.Text, "Page screenshot:") + assert.Contains(t, m.Text, "https://webd.example.com/artifact-view?d=abc&sig=def") +} + +// TestInteractionSender_Screenshot_NilMinter_NoLineNoError verifies the +// best-effort contract: an unwired minter must never block or error the +// card — it only means no screenshot line, exactly like a text payload with +// no Screenshot at all. +func TestInteractionSender_Screenshot_NilMinter_NoLineNoError(t *testing.T) { + sink := &RecordingSink{} + s := &interactionSender{sink: newInertSink(sink)} + + _, err := s.Send(context.Background(), sessInfo(), screenshotRequestEnv(t)) + require.NoError(t, err, "a nil minter must never fail the card") + require.Len(t, sink.Events(), 1) + m, ok := sink.Events()[0].(MsgInteractionRequest) + require.True(t, ok, "want MsgInteractionRequest, got %T", sink.Events()[0]) + assert.NotContains(t, m.Text, "Page screenshot:") +} + +// TestInteractionSender_Screenshot_MinterErrors_NoLineNoError verifies that a +// minting error is swallowed into "no line" rather than propagated as a +// Send error — the card (Lead/Body/Actions) must still be delivered. +func TestInteractionSender_Screenshot_MinterErrors_NoLineNoError(t *testing.T) { + sink := &RecordingSink{} + s := &interactionSender{ + sink: newInertSink(sink), + minter: &fakeMinter{err: errors.New("mint failed")}, + principal: identity.RawSubject("user:alice"), + } + + _, err := s.Send(context.Background(), sessInfo(), screenshotRequestEnv(t)) + require.NoError(t, err, "a minter error must never fail the card") + require.Len(t, sink.Events(), 1) + m, ok := sink.Events()[0].(MsgInteractionRequest) + require.True(t, ok, "want MsgInteractionRequest, got %T", sink.Events()[0]) + assert.NotContains(t, m.Text, "Page screenshot:") +} diff --git a/pkg/channels/channelkinds/slack/interaction.go b/pkg/channels/channelkinds/slack/interaction.go index 1334e1a4..c35e7fb1 100644 --- a/pkg/channels/channelkinds/slack/interaction.go +++ b/pkg/channels/channelkinds/slack/interaction.go @@ -14,7 +14,10 @@ // ephemeral-with-DM-fallback) to every identity in Audience.Requester or // Audience.Approvers depending on Scope. The delivery lands in s.delivery, // keyed by RequestRef, whenever deliveryEditable(p) says a later -// interaction_applied will need to edit it in place. +// interaction_applied will need to edit it in place. When the payload +// carries a Screenshot (browser-originated interactions only), the +// resolved PNG is uploaded as a threaded file under each recipient's +// card right after it posts — best-effort, see uploadScreenshot. // - KindInteractionApplied{Category: credential_link, Outcome: resolved}: // posts a "connected" notice at the resolved tone. credential_link resolves // out-of-band (an OAuth callback, not a click), so there is no response_url @@ -40,6 +43,7 @@ package slack import ( + "bytes" "context" "encoding/json" "fmt" @@ -344,6 +348,13 @@ func (s *interactionSender) sendRequest(ctx context.Context, sess channelkinds.S } lastRef = ref delivered = true + // Screenshot upload is threaded under THIS recipient's card, posted + // immediately after it so a fan-out to multiple approvers gives each + // their own in-thread picture rather than racing to attach to + // whichever card landed last. Best-effort: the card already carries + // the full approval text, so a fetch/upload failure must not fail the + // interaction send (see uploadScreenshot). + s.uploadScreenshot(ctx, sess, p, ref, logger) } // Nothing reached anybody. This is a FAILURE, not a benign skip: the // session parks awaiting a decision no one was asked for, and without a @@ -423,6 +434,58 @@ func (s *interactionSender) sendRequest(ctx context.Context, sess channelkinds.S return nil } +// uploadScreenshot resolves p.Screenshot's bytes via the AssetFetcher and +// uploads the redacted page image as a PNG file threaded under ref — the +// card's own (channelID, ts) that deliverPrompt / postBroadcast just +// returned. Mirrors sendWithAttachments' fetch+upload shape (sender.go), not +// its fallback-text behavior: there is nothing to fall back to here, since +// the card already landed carrying the full approval text — a missing +// picture is a cosmetic loss, not a delivery failure. +// +// Every exit here is a silent-but-logged skip, never an error returned to +// the caller (sendRequest): a nil Screenshot is the common case (every +// non-browser interaction) and logs nothing; a nil AssetFetcher (a surface +// that never configured one) or ref.TS == "" (nothing was actually posted to +// thread under, e.g. a delivery that errored before reaching here — though +// callers only reach this after a successful deliverPrompt/postBroadcast) +// logs once and returns; a Fetch or UploadFileContext failure logs the +// session + renderName + error per the repo's no-silent-errors rule and +// returns, leaving the card as the complete approval surface. +func (s *interactionSender) uploadScreenshot(ctx context.Context, sess channelkinds.SessionInfo, p channelevents.InteractionRequestPayload, ref deliveryRef, logger logr.Logger) { + if p.Screenshot == nil || ref.TS == "" { + return + } + if s.deps.AssetFetcher == nil { + logger.Info("interaction sender: no AssetFetcher configured; screenshot upload skipped", + "session", sess.Namespace+"/"+sess.Name, "requestRef", p.RequestRef, "render", p.Screenshot.RenderName) + return + } + ab, err := s.deps.AssetFetcher.Fetch(ctx, sess.Namespace, sess.Name, p.Screenshot.RenderName) + if err != nil { + logger.Info("interaction sender: fetch screenshot failed (best-effort, card already delivered)", + "session", sess.Namespace+"/"+sess.Name, "requestRef", p.RequestRef, "render", p.Screenshot.RenderName, "err", err.Error()) + return + } + filename := p.Screenshot.Filename + if filename == "" { + filename = ab.Filename + } + if filename == "" { + filename = "page.png" + } + if _, err := s.client.UploadFileContext(ctx, slackapi.UploadFileParameters{ + Reader: bytes.NewReader(ab.Bytes), + FileSize: len(ab.Bytes), + Filename: filename, + Channel: ref.ChannelID, + ThreadTimestamp: ref.TS, + AltTxt: p.Screenshot.AltText, + }); err != nil { + logger.Info("interaction sender: upload screenshot failed (best-effort, card already delivered)", + "session", sess.Namespace+"/"+sess.Name, "requestRef", p.RequestRef, "render", p.Screenshot.RenderName, "err", err.Error()) + } +} + // sendApplied renders a resolved interaction outcome. credential_link is a // special case (out-of-band "✅ connected" notice, see the file-top comment // for why the recipient comes from sess.SessionInitiator rather than the diff --git a/pkg/channels/channelkinds/slack/interaction_screenshot_test.go b/pkg/channels/channelkinds/slack/interaction_screenshot_test.go new file mode 100644 index 00000000..ff5c9948 --- /dev/null +++ b/pkg/channels/channelkinds/slack/interaction_screenshot_test.go @@ -0,0 +1,174 @@ +// pkg/channels/channelkinds/slack/interaction_screenshot_test.go +// +// Task 5 (browser approval screenshots): after an interaction_request card +// posts, a payload carrying Screenshot must have the resolved PNG uploaded +// as a file threaded under that SAME card — see interaction.go's +// uploadScreenshot. These tests pin the three contract points: the happy +// path threads the upload under the card's own ts, a nil Screenshot is a +// complete no-op (no Fetch, no upload — the common case for every +// non-browser interaction), and a Fetch failure must not fail the card send +// (best-effort; the card already carries the full approval). +package slack + +import ( + "context" + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/authzed/openagentprimitives/pkg/channels/channelevents" + "github.com/authzed/openagentprimitives/pkg/channels/channelkinds" + "github.com/authzed/openagentprimitives/pkg/platform/identity" +) + +// screenshotRequestPayload returns a wire-valid AudienceRequester +// InteractionRequestPayload carrying the given Screenshot ref — the shape a +// browser-originated interaction (acting-approval, login_human_step, +// captcha) publishes. nil is legal (every non-browser category's zero +// value). +func screenshotRequestPayload(rawExternalID string, screenshot *channelevents.AttachmentRef) channelevents.InteractionRequestPayload { + return channelevents.InteractionRequestPayload{ + Category: "login_human_step", + RequestRef: "req-shot-1", + Lead: "Finish signing in", + Screenshot: screenshot, + Actions: []channelevents.InteractionAction{ + {ID: "done", Label: "Done", Kind: channelevents.ActionKindDecision, Style: channelevents.ActionStylePrimary}, + }, + Audience: channelevents.InteractionAudience{ + Scope: channelevents.AudienceRequester, + Requester: &channelevents.ExternalIdentity{ + Kind: "slack", + ExternalID: identity.RawExternalID(rawExternalID), + }, + }, + } +} + +// TestInteractionSender_Request_Screenshot_UploadsThreadedUnderCard verifies +// the happy path end to end: the card posts first (via PostEphemeralContext, +// since the session carries channel context), and the Screenshot is then +// resolved via AssetFetcher.Fetch(ns, sess, RenderName) and uploaded via +// UploadFileContext with ThreadTimestamp equal to the card's own ts — the +// card establishes the thread root; the upload never can (see sender.go's +// sendWithAttachments note on files.uploadV2 returning no ts). +func TestInteractionSender_Request_Screenshot_UploadsThreadedUnderCard(t *testing.T) { + const ( + slackID = "U_SHOT" + chanID = "C01CHAN" + threadTS = "1700000000.000001" + ) + pngBytes := []byte("\x89PNG-fake-bytes") + af := &stubAssetFetcher{bytes: pngBytes, mime: "image/png", filename: "page.png"} + c := &fakeSlackClient{} + s := &interactionSender{client: c, deps: channelkinds.Deps{AssetFetcher: af}} + + screenshot := &channelevents.AttachmentRef{ + RenderName: "shot-1", + MIME: "image/png", + Filename: "page.png", + AltText: "redacted page screenshot", + } + pl := screenshotRequestPayload(slackID, screenshot) + env := interactionEnvelope(t, channelevents.KindInteractionRequest, pl) + sess := sessionWithChannel(chanID, threadTS) + + _, err := s.Send(context.Background(), sess, env) + require.NoError(t, err, "Send") + + // The card itself landed via PostEphemeral (channel context present). + require.Len(t, c.postEphemeralCalls, 1, "expected 1 card post") + cardTS := "1614191050.013301" // fakeSlackClient's fixed PostEphemeralContext ts + + require.Len(t, af.calls, 1, "expected exactly one asset fetch") + assert.Equal(t, stubFetchCall{ns: "default", sess: "sess-1", render: "shot-1"}, af.calls[0], + "AssetFetcher.Fetch must be called with the ref's RenderName") + + require.Len(t, c.uploadFileCalls, 1, "expected exactly one file upload") + up := c.uploadFileCalls[0] + assert.Equal(t, chanID, up.channelID, "upload must target the card's channel") + assert.Equal(t, cardTS, up.threadTS, "upload must be threaded under the card's own ts") + assert.Equal(t, pngBytes, up.contents, "uploaded bytes must be the fetched PNG bytes") + assert.Equal(t, "page.png", up.filename) + assert.Equal(t, "redacted page screenshot", up.altTxt) +} + +// TestInteractionSender_Request_NilScreenshot_NoUploadAttempt verifies the +// common case (every non-browser interaction): a nil Screenshot means no +// Fetch call and no UploadFileContext call, while the card itself still +// sends normally. +func TestInteractionSender_Request_NilScreenshot_NoUploadAttempt(t *testing.T) { + const ( + slackID = "U_NOSHOT" + chanID = "C01CHAN" + threadTS = "1700000000.000002" + ) + af := &stubAssetFetcher{bytes: []byte("should never be fetched"), mime: "image/png", filename: "page.png"} + c := &fakeSlackClient{} + s := &interactionSender{client: c, deps: channelkinds.Deps{AssetFetcher: af}} + + pl := screenshotRequestPayload(slackID, nil) + env := interactionEnvelope(t, channelevents.KindInteractionRequest, pl) + sess := sessionWithChannel(chanID, threadTS) + + _, err := s.Send(context.Background(), sess, env) + require.NoError(t, err, "Send") + + require.Len(t, c.postEphemeralCalls, 1, "the card must still send") + assert.Empty(t, af.calls, "no Screenshot ⇒ AssetFetcher.Fetch must never be called") + assert.Empty(t, c.uploadFileCalls, "no Screenshot ⇒ UploadFileContext must never be called") +} + +// TestInteractionSender_Request_ScreenshotFetchError_CardStillSucceeds +// verifies the best-effort contract: an AssetFetcher.Fetch error must not +// fail sendRequest (the card already carries the full approval text) and +// must not attempt an upload with no bytes. +func TestInteractionSender_Request_ScreenshotFetchError_CardStillSucceeds(t *testing.T) { + const ( + slackID = "U_FETCHFAIL" + chanID = "C01CHAN" + threadTS = "1700000000.000003" + ) + af := &stubAssetFetcher{err: errors.New("operator says no")} + c := &fakeSlackClient{} + s := &interactionSender{client: c, deps: channelkinds.Deps{AssetFetcher: af}} + + screenshot := &channelevents.AttachmentRef{RenderName: "shot-err", MIME: "image/png", Filename: "page.png"} + pl := screenshotRequestPayload(slackID, screenshot) + env := interactionEnvelope(t, channelevents.KindInteractionRequest, pl) + sess := sessionWithChannel(chanID, threadTS) + + _, err := s.Send(context.Background(), sess, env) + require.NoError(t, err, "a screenshot fetch failure must not fail the card send") + + require.Len(t, c.postEphemeralCalls, 1, "the card must still have landed") + require.Len(t, af.calls, 1, "the fetch must still have been attempted") + assert.Empty(t, c.uploadFileCalls, "no bytes to upload after a fetch failure") +} + +// TestInteractionSender_Request_ScreenshotNoAssetFetcher_SkipsUpload verifies +// the AssetFetcher-nil guard: a surface that never configured an +// AssetFetcher (some surfaces don't) must skip the upload entirely — +// no panic, no error, the card still sends. +func TestInteractionSender_Request_ScreenshotNoAssetFetcher_SkipsUpload(t *testing.T) { + const ( + slackID = "U_NOFETCHER" + chanID = "C01CHAN" + threadTS = "1700000000.000004" + ) + c := &fakeSlackClient{} + s := &interactionSender{client: c} // deps.AssetFetcher left nil + + screenshot := &channelevents.AttachmentRef{RenderName: "shot-nofetcher", MIME: "image/png", Filename: "page.png"} + pl := screenshotRequestPayload(slackID, screenshot) + env := interactionEnvelope(t, channelevents.KindInteractionRequest, pl) + sess := sessionWithChannel(chanID, threadTS) + + _, err := s.Send(context.Background(), sess, env) + require.NoError(t, err, "a nil AssetFetcher must not fail the card send") + + require.Len(t, c.postEphemeralCalls, 1, "the card must still have landed") + assert.Empty(t, c.uploadFileCalls, "no AssetFetcher ⇒ no upload attempted") +} diff --git a/pkg/web/webui/chat/sink_screenshot_test.go b/pkg/web/webui/chat/sink_screenshot_test.go new file mode 100644 index 00000000..1d436206 --- /dev/null +++ b/pkg/web/webui/chat/sink_screenshot_test.go @@ -0,0 +1,88 @@ +package chat + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/authzed/openagentprimitives/pkg/channels/channelevents" + "github.com/authzed/openagentprimitives/pkg/channels/channelkinds/browser" +) + +// TestToFrame_InteractionRequest_CarriesScreenshotRef pins the claim Task 6 +// of the browser-approval-screenshots feature depends on: toFrame's +// MsgInteractionRequest branch maps the WHOLE channelevents. +// InteractionRequestPayload (Payload: m) onto the wire frame, so +// InteractionRequestPayload.Screenshot — Task 1's redacted-screenshot +// AttachmentRef — rides along with no change to toFrame itself. This test +// exists to catch a future refactor that narrows the frame's payload to a +// hand-picked field set (the same mistake would silently drop Screenshot +// again) rather than to prove any code was written here. +func TestToFrame_InteractionRequest_CarriesScreenshotRef(t *testing.T) { + ref := browser.SessionRef{Namespace: "default", Name: "s1"} + shot := &channelevents.AttachmentRef{ + RenderName: "shot-1", + ArtifactID: "art-1", + MIME: "image/png", + Filename: "page.png", + AltText: "the page", + } + msg := browser.MsgInteractionRequest{ + Session: ref, + Payload: channelevents.InteractionRequestPayload{ + Category: "tool_approval", + RequestRef: "req-1", + Lead: "Approve this tool call?", + Screenshot: shot, + }, + } + + frame, ok := toFrame(msg) + require.True(t, ok, "toFrame must map browser.MsgInteractionRequest") + assert.Equal(t, "interaction_request", frame.Type) + + // Round-trip through JSON the same way the wire frame actually travels to + // the browser, rather than reaching into frame.Payload's concrete type — + // that is the only way to catch a regression in the json tag itself. + raw, err := json.Marshal(frame) + require.NoError(t, err) + var decoded struct { + Payload struct { + Payload struct { + Screenshot *channelevents.AttachmentRef `json:"screenshot"` + } `json:"payload"` + } `json:"payload"` + } + require.NoError(t, json.Unmarshal(raw, &decoded)) + require.NotNil(t, decoded.Payload.Payload.Screenshot, "screenshot ref must survive the frame round-trip") + assert.Equal(t, "shot-1", decoded.Payload.Payload.Screenshot.RenderName) + assert.Equal(t, "art-1", decoded.Payload.Payload.Screenshot.ArtifactID) + assert.Equal(t, "page.png", decoded.Payload.Payload.Screenshot.Filename) + assert.Equal(t, "the page", decoded.Payload.Payload.Screenshot.AltText) +} + +// TestToFrame_InteractionRequest_NilScreenshotOmitted covers the inverse: a +// non-browser (or best-effort-failed) interaction request carries no +// screenshot, and omitempty keeps it absent from the wire frame entirely +// rather than serializing an explicit null the TS side would have to special +// case. +func TestToFrame_InteractionRequest_NilScreenshotOmitted(t *testing.T) { + ref := browser.SessionRef{Namespace: "default", Name: "s1"} + msg := browser.MsgInteractionRequest{ + Session: ref, + Payload: channelevents.InteractionRequestPayload{ + Category: "credential_link", + RequestRef: "req-2", + Lead: "Connect your account", + }, + } + + frame, ok := toFrame(msg) + require.True(t, ok) + + raw, err := json.Marshal(frame) + require.NoError(t, err) + assert.NotContains(t, string(raw), `"screenshot"`, "a nil Screenshot must be omitted, not sent as null") +} diff --git a/pkg/web/webui/chat/ui/InteractionCard.test.tsx b/pkg/web/webui/chat/ui/InteractionCard.test.tsx index 731d25a0..4590a35d 100644 --- a/pkg/web/webui/chat/ui/InteractionCard.test.tsx +++ b/pkg/web/webui/chat/ui/InteractionCard.test.tsx @@ -612,4 +612,35 @@ describe("InteractionCard resolved-state collapsing", () => { expect(container.querySelector("details")).toBeNull(); expect(screen.getByText("Phase 1")).toBeTruthy(); }); + + it("renders the screenshot as an inline img built from the same-origin artifact-download URL", () => { + const { container } = render( +