diff --git a/docs/code-patterns.md b/docs/code-patterns.md
index 8a0f9de..1c21ad6 100644
--- a/docs/code-patterns.md
+++ b/docs/code-patterns.md
@@ -15,6 +15,8 @@
Worth knowing before touching related code:
+- The automation session's document CSP (`internal/browser/automation.go`, `restrictDocument`) only works because of two Chrome details. A document is paused again at the response stage only if its `ContinueRequest` sets `InterceptResponse`. And Chrome applies just the `sandbox` part of a CSP header edited with `Fetch.continueResponse`: `worker-src` and `connect-src` are ignored, so the response is fulfilled with its own body and the new header instead. Before this was found the policy never ran at all. `TestInjectedCSPApplies` runs a real page against it.
+- Screenshots are named by their actual format: Chrome returns JPEG below quality 100 and PNG at 100, and they used to be JPEG data in `.png` files.
- `history.Store.UpdatePipelineStatus` only moves a request forward (ranks in `internal/history/pipeline_rank.go`): an earlier or unknown stage is silently ignored, so a late "awaiting response" can't undo a confirmation. A person's explicit decision in the UI goes through `OverridePipelineStatus`, which always applies. Both record who made the change in `pipeline_actor`.
- `batch_approvals` timestamps are stored and compared in UTC. SQLite compares them as text, so a local UTC offset (DST) would break the ordering. `ConsumeBatchApproval` is the only way to spend an approval and it is atomic: one approval, one batch.
- `no_data_held` is a value in the export JSON (`response_type`, `pipeline_status`), so it can only be added to, never renamed. `MigrateNoData` only relabels saved replies; resolving a request is `ResolveNoData`'s job (and the reclassify action's).
diff --git a/internal/browser/automation.go b/internal/browser/automation.go
index b2f2adb..bcd86aa 100644
--- a/internal/browser/automation.go
+++ b/internal/browser/automation.go
@@ -4,6 +4,7 @@ import (
"context"
"crypto/rand"
"crypto/sha256"
+ "encoding/base64"
"encoding/json"
"fmt"
"net"
@@ -216,12 +217,7 @@ func (s *AutomationSession) Open(caller context.Context, raw string) (*Snapshot,
case *fetch.EventRequestPaused:
go s.checkRequest(e)
case *network.EventResponseReceived:
- // Our DNS lookup and Chrome's can differ, so check the address Chrome used.
- if e.Response != nil && s.policy.LocalOrigin == "" && !publicAddress(e.Response.RemoteIPAddress) {
- s.mu.Lock()
- s.networkBlocked = "The page connected to a private network address, so it was blocked. Review the broker's URLs."
- s.mu.Unlock()
- }
+ s.noteResponse(e)
}
})
ctx, cancel := s.operation(caller)
@@ -233,18 +229,63 @@ func (s *AutomationSession) Open(caller context.Context, raw string) (*Snapshot,
return s.Observe(caller)
}
+// csp is added to every document the session loads. It keeps popups, workers
+// and out-of-scope connections from escaping the tab's request interceptor;
+// policies stack, so the site's own stays in force.
+func (p URLPolicy) csp() string {
+ return "sandbox allow-scripts allow-same-origin allow-forms allow-modals; worker-src 'none'; connect-src " + strings.Join(p.connectSources(), " ")
+}
+
+// restrictDocument adds the CSP to a document response. Chrome only enforces the
+// sandbox part of a header changed through ContinueResponse and ignores
+// worker-src and connect-src from it, so the response is fulfilled with its own
+// body and the new header instead, which makes the whole policy count.
+func (s *AutomationSession) restrictDocument(ctx context.Context, e *fetch.EventRequestPaused) {
+ headers := append(e.ResponseHeaders, &fetch.HeaderEntry{Name: "Content-Security-Policy", Value: s.policy.csp()})
+ var err error
+ if e.ResponseStatusCode >= 300 && e.ResponseStatusCode < 400 {
+ // A redirect has no page; the next hop comes back through the interceptor.
+ if err := fetch.ContinueResponse(e.RequestID).Do(ctx); err != nil {
+ _ = fetch.FailRequest(e.RequestID, network.ErrorReasonBlockedByClient).Do(ctx)
+ }
+ return
+ }
+ body, bodyErr := fetch.GetResponseBody(e.RequestID).Do(ctx)
+ switch {
+ case bodyErr != nil:
+ // Nothing to read (an empty response): the sandbox still applies.
+ err = fetch.ContinueResponse(e.RequestID).WithResponseCode(e.ResponseStatusCode).WithResponseHeaders(headers).Do(ctx)
+ default:
+ // GetResponseBody returns the decoded body, so drop the encoding headers.
+ kept := make([]*fetch.HeaderEntry, 0, len(headers))
+ for _, h := range headers {
+ if n := strings.ToLower(h.Name); n != "content-encoding" && n != "content-length" {
+ kept = append(kept, h)
+ }
+ }
+ err = fetch.FulfillRequest(e.RequestID, e.ResponseStatusCode).WithResponseHeaders(kept).WithBody(base64.StdEncoding.EncodeToString(body)).Do(ctx)
+ }
+ if err != nil {
+ _ = fetch.FailRequest(e.RequestID, network.ErrorReasonBlockedByClient).Do(ctx)
+ }
+}
+
+// noteResponse stops the session when Chrome connected to a non-public address.
+// Our DNS lookup and Chrome's can differ, so check the address Chrome used.
+func (s *AutomationSession) noteResponse(e *network.EventResponseReceived) {
+ if e.Response != nil && s.policy.LocalOrigin == "" && !publicAddress(e.Response.RemoteIPAddress) {
+ s.mu.Lock()
+ s.networkBlocked = "The page connected to a private network address, so it was blocked. Review the broker's URLs."
+ s.mu.Unlock()
+ }
+}
+
func (s *AutomationSession) checkRequest(e *fetch.EventRequestPaused) {
ctx, cancel := context.WithTimeout(s.b.ctx, 10*time.Second)
defer cancel()
execCtx := cdp.WithExecutor(ctx, chromedp.FromContext(ctx).Target)
if e.ResponseStatusCode != 0 {
- // popups, workers and stray WebSockets could escape the request interceptor, so add
- // a CSP; policies stack, so the site's own stays in force
- csp := "sandbox allow-scripts allow-same-origin allow-forms allow-modals; worker-src 'none'; connect-src " + strings.Join(s.policy.connectSources(), " ")
- headers := append(e.ResponseHeaders, &fetch.HeaderEntry{Name: "Content-Security-Policy", Value: csp})
- if err := fetch.ContinueResponse(e.RequestID).WithResponseCode(e.ResponseStatusCode).WithResponseHeaders(headers).Do(execCtx); err != nil {
- _ = fetch.FailRequest(e.RequestID, network.ErrorReasonBlockedByClient).Do(execCtx)
- }
+ s.restrictDocument(execCtx, e)
return
}
// every request: redirects, frames, XHR, form actions. third-party portals must be in the catalog
@@ -259,7 +300,13 @@ func (s *AutomationSession) checkRequest(e *fetch.EventRequestPaused) {
_ = fetch.FailRequest(e.RequestID, network.ErrorReasonBlockedByClient).Do(execCtx)
return
}
- _ = fetch.ContinueRequest(e.RequestID).Do(execCtx)
+ // Chrome only pauses a document again at the response stage, where the CSP
+ // is added, when the request asks for it.
+ cont := fetch.ContinueRequest(e.RequestID)
+ if e.ResourceType == network.ResourceTypeDocument {
+ cont = cont.WithInterceptResponse(true)
+ }
+ _ = cont.Do(execCtx)
}
func (s *AutomationSession) evaluate(ctx context.Context, expression string, out any) error {
diff --git a/internal/browser/automation_extra_test.go b/internal/browser/automation_extra_test.go
new file mode 100644
index 0000000..187384c
--- /dev/null
+++ b/internal/browser/automation_extra_test.go
@@ -0,0 +1,249 @@
+package browser
+
+import (
+ "context"
+ "fmt"
+ "net/http"
+ "net/http/httptest"
+ "strings"
+ "testing"
+
+ "github.com/chromedp/cdproto/network"
+)
+
+// --- no Chrome needed ---------------------------------------------------
+
+func TestNewAutomationSessionNeedsApprovedDomains(t *testing.T) {
+ if _, err := NewAutomationSession(DefaultConfig(), testProfile(), URLPolicy{}); err == nil || !strings.Contains(err.Error(), "no approved domains") {
+ t.Fatalf("a broker with no domains: %v", err)
+ }
+ // Chrome starts lazily, so building and closing a session needs no browser.
+ s, err := NewAutomationSession(DefaultConfig(), testProfile(), NewURLPolicy("https://broker.example", ""))
+ if err != nil {
+ t.Fatal(err)
+ }
+ s.Close()
+}
+
+func TestSnapshotFingerprintIgnoresEphemeralFields(t *testing.T) {
+ a := Snapshot{ID: "one", Screenshot: "a.png", URL: "https://x.example/f", Text: "Remove me"}
+ b := a
+ b.ID, b.Screenshot = "two", "b.png"
+ if a.Fingerprint() != b.Fingerprint() {
+ t.Error("re-observing the same page changed its fingerprint")
+ }
+ b.Text = "Thank you"
+ if a.Fingerprint() == b.Fingerprint() {
+ t.Error("a changed page kept its fingerprint")
+ }
+}
+
+func TestPolicyEdgeCases(t *testing.T) {
+ if got := (URLPolicy{LocalOrigin: "http://127.0.0.1:9"}).connectSources(); got[len(got)-1] != "http://127.0.0.1:9" {
+ t.Errorf("connect sources = %v, want the local origin included", got)
+ }
+ err := NewURLPolicy("https://does-not-resolve.invalid", "").Validate(context.Background(), "https://does-not-resolve.invalid/form")
+ if err == nil || !strings.Contains(err.Error(), "cannot verify destination address") {
+ t.Errorf("unresolvable host: %v", err)
+ }
+}
+
+func TestOpenReportsChromeThatCannotStart(t *testing.T) {
+ cfg := DefaultConfig()
+ cfg.ExecutablePath = "/nonexistent/chrome"
+ s, err := NewAutomationSession(cfg, testProfile(), URLPolicy{LocalOrigin: "http://127.0.0.1:1"})
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer s.Close()
+ if _, err := s.Open(context.Background(), "http://127.0.0.1:1/form"); err == nil || !strings.Contains(err.Error(), "start Chrome") {
+ t.Fatalf("Open with no browser: %v", err)
+ }
+}
+
+func TestNoteResponseStopsTheSessionOnPrivateAddresses(t *testing.T) {
+ response := func(ip string) *network.EventResponseReceived {
+ return &network.EventResponseReceived{Response: &network.Response{RemoteIPAddress: ip}}
+ }
+ for ip, blocked := range map[string]bool{"93.184.216.34": false, "": false, "10.0.0.5": true, "127.0.0.1": true, "169.254.169.254": true} {
+ s := &AutomationSession{policy: NewURLPolicy("https://broker.example", "")}
+ s.noteResponse(response(ip))
+ if got := s.blockedReason() != ""; got != blocked {
+ t.Errorf("response from %q: blocked = %v, want %v", ip, got, blocked)
+ }
+ }
+ s := &AutomationSession{policy: NewURLPolicy("https://broker.example", "")}
+ s.noteResponse(&network.EventResponseReceived{}) // no response attached
+ if s.blockedReason() != "" {
+ t.Error("an event with no response blocked the session")
+ }
+ // The demo's own loopback server is allowed.
+ s = &AutomationSession{policy: URLPolicy{LocalOrigin: "http://127.0.0.1:9"}}
+ s.noteResponse(response("127.0.0.1"))
+ if s.blockedReason() != "" {
+ t.Error("the demo's local server was blocked")
+ }
+}
+
+// --- real Chrome --------------------------------------------------------
+
+const controlsPage = `Controls
+`
+
+// Each message comes from the page script or the Go wrapper in Act, and each is
+// what an agent sees when its plan no longer matches the page.
+func TestActReportsWhyAControlCannotBeUsed(t *testing.T) {
+ s, base := automationFixture(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ w.Header().Set("Content-Type", "text/html")
+ _, _ = fmt.Fprint(w, controlsPage)
+ }))
+ ctx := context.Background()
+ if _, err := s.Open(ctx, base); err != nil {
+ t.Fatal(err)
+ }
+ pageCtx, cancel := s.operation(ctx) // evaluate needs the browser's own context
+ defer cancel()
+ observe := func() *Snapshot {
+ t.Helper()
+ snap, err := s.Observe(ctx)
+ if err != nil {
+ t.Fatal(err)
+ }
+ return snap
+ }
+ mustFail := func(snap *Snapshot, label, action, value, want string) {
+ t.Helper()
+ _, err := s.Act(ctx, snap.ID, controlRef(t, snap, label), action, value)
+ if err == nil || !strings.Contains(err.Error(), want) {
+ t.Errorf("%s %s %q: error = %v, want it to mention %q", action, label, value, err, want)
+ }
+ }
+
+ snap := observe()
+ mustFail(snap, "Plan", "fill", "Enterprise", "Option not found")
+ snap = observe() // a failed action leaves the snapshot usable, but be explicit
+ mustFail(snap, "Agree", "fill", "maybe", "true or false")
+ snap = observe()
+ mustFail(snap, "Go", "fill", "text", "not a form field")
+ snap = observe()
+ mustFail(snap, "Go", "tap", "", "Unsupported action")
+
+ // A control removed after the snapshot, and one disabled after it.
+ snap = observe()
+ gone, goRef := controlRef(t, snap, "Vanishing"), controlRef(t, snap, "Go")
+ if err := s.evaluate(pageCtx, `document.getElementById('gone').remove(); document.getElementById('go').disabled = true; true`, nil); err != nil {
+ t.Fatal(err)
+ }
+ if _, err := s.Act(ctx, snap.ID, gone, "click", ""); err == nil || !strings.Contains(err.Error(), "Control changed") {
+ t.Errorf("removed control: %v", err)
+ }
+ if _, err := s.Act(ctx, snap.ID, goRef, "click", ""); err == nil || !strings.Contains(err.Error(), "Control unavailable") {
+ t.Errorf("disabled control: %v", err)
+ }
+}
+
+// The document CSP is what keeps a page from opening popups or starting
+// workers that the request interceptor cannot see. Chrome ignores most of it
+// when the header is only edited in place, so this checks it really applies.
+func TestInjectedCSPApplies(t *testing.T) {
+ mux := http.NewServeMux()
+ mux.HandleFunc("/worker.js", func(w http.ResponseWriter, r *http.Request) {
+ w.Header().Set("Content-Type", "text/javascript")
+ _, _ = fmt.Fprint(w, `postMessage('ran')`)
+ })
+ mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
+ w.Header().Set("Content-Type", "text/html")
+ _, _ = fmt.Fprint(w, `starting`)
+ })
+ s, base := automationFixture(t, mux)
+ ctx := context.Background()
+ if _, err := s.Open(ctx, base); err != nil {
+ t.Fatal(err)
+ }
+ // The page finishes asynchronously, whichever way the worker goes.
+ var title string
+ for range 40 {
+ snap, err := s.Observe(ctx)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if title = snap.Title; title != "starting" {
+ break
+ }
+ }
+ if title != "popup-blocked worker-blocked" {
+ t.Fatalf("title = %q, want popup-blocked worker-blocked: the injected CSP did not apply", title)
+ }
+}
+
+func TestOpenReportsAPageThatCannotBeLoaded(t *testing.T) {
+ b := requireChrome(t)
+ b.config.ScreenshotDir = t.TempDir()
+ web := httptest.NewServer(http.NotFoundHandler())
+ url := web.URL
+ web.Close() // nothing is listening any more
+ s := &AutomationSession{b: b, policy: URLPolicy{LocalOrigin: url}}
+ t.Cleanup(s.Close)
+ if _, err := s.Open(context.Background(), url); err == nil || !strings.Contains(err.Error(), "open task page") {
+ t.Fatalf("Open of an unreachable page: %v", err)
+ }
+}
+
+func TestInjectedPolicyStringKeepsPopupsAndWorkersOut(t *testing.T) {
+ csp := NewURLPolicy("https://broker.example", "").csp()
+ for _, want := range []string{"sandbox ", "allow-forms", "worker-src 'none'", "connect-src 'self' https://broker.example https://www.broker.example"} {
+ if !strings.Contains(csp, want) {
+ t.Errorf("policy %q is missing %q", csp, want)
+ }
+ }
+ if strings.Contains(csp, "allow-popups") || strings.Contains(csp, "allow-top-navigation") {
+ t.Errorf("policy %q allows what the sandbox is there to stop", csp)
+ }
+}
+
+// A redirect has no page of its own: it must go through untouched, and the page
+// it lands on must still get the policy.
+func TestRedirectedDocumentStillGetsThePolicy(t *testing.T) {
+ mux := http.NewServeMux()
+ mux.HandleFunc("/start", func(w http.ResponseWriter, r *http.Request) {
+ http.Redirect(w, r, "/landing", http.StatusFound)
+ })
+ mux.HandleFunc("/landing", func(w http.ResponseWriter, r *http.Request) {
+ w.Header().Set("Content-Type", "text/html")
+ _, _ = fmt.Fprint(w, `landed`)
+ })
+ s, base := automationFixture(t, mux)
+ snap, err := s.Open(context.Background(), base+"/start")
+ if err != nil {
+ t.Fatal(err)
+ }
+ if !strings.HasSuffix(snap.URL, "/landing") || snap.Title != "landed popup-blocked" {
+ t.Fatalf("after the redirect: url=%q title=%q", snap.URL, snap.Title)
+ }
+}
+
+// A response with nothing to read still loads, with the sandbox applied.
+func TestEmptyDocumentStillLoads(t *testing.T) {
+ s, base := automationFixture(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ w.Header().Set("Content-Type", "text/html")
+ }))
+ if _, err := s.Open(context.Background(), base); err != nil {
+ t.Fatalf("an empty page should still open: %v", err)
+ }
+}
diff --git a/internal/browser/browser.go b/internal/browser/browser.go
index 8dc4dbf..efc7746 100644
--- a/internal/browser/browser.go
+++ b/internal/browser/browser.go
@@ -379,7 +379,12 @@ func (b *Browser) takeScreenshotQuality(ctx context.Context, brokerID, suffix st
return "", err
}
- filename := fmt.Sprintf("%s_%s_%d.png", brokerID, suffix, time.Now().Unix())
+ // Chrome returns JPEG below quality 100, and these used to be named .png.
+ ext := "jpg"
+ if quality >= 100 {
+ ext = "png"
+ }
+ filename := fmt.Sprintf("%s_%s_%d.%s", brokerID, suffix, time.Now().Unix(), ext)
filepath := filepath.Join(b.config.ScreenshotDir, filename)
if err := os.WriteFile(filepath, buf, 0600); err != nil {
diff --git a/internal/browser/screenshot_format_test.go b/internal/browser/screenshot_format_test.go
new file mode 100644
index 0000000..d1ccd6e
--- /dev/null
+++ b/internal/browser/screenshot_format_test.go
@@ -0,0 +1,37 @@
+package browser
+
+import (
+ "bytes"
+ "os"
+ "path/filepath"
+ "strings"
+ "testing"
+)
+
+// The file name must say what is inside: Chrome returns JPEG below quality 100
+// and PNG at 100, and screenshots used to be JPEG data named .png.
+func TestScreenshotFilesAreNamedByTheirFormat(t *testing.T) {
+ b := requireChrome(t)
+ b.config.ScreenshotDir = t.TempDir()
+
+ for _, tc := range []struct {
+ quality int
+ ext string
+ magic []byte
+ }{
+ {90, ".jpg", []byte{0xFF, 0xD8, 0xFF}},
+ {100, ".png", []byte("\x89PNG")},
+ } {
+ name, err := b.takeScreenshotQuality(b.ctx, "acme", "shot", tc.quality)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if !strings.HasSuffix(name, tc.ext) {
+ t.Errorf("quality %d: file %q, want a %s extension", tc.quality, name, tc.ext)
+ }
+ data, err := os.ReadFile(filepath.Join(b.config.ScreenshotDir, name))
+ if err != nil || !bytes.HasPrefix(data, tc.magic) {
+ t.Errorf("quality %d: %q does not start with the %s signature (%v)", tc.quality, name, tc.ext, err)
+ }
+ }
+}
diff --git a/internal/inbox/imap_server_test.go b/internal/inbox/imap_server_test.go
index 016866c..53803e4 100644
--- a/internal/inbox/imap_server_test.go
+++ b/internal/inbox/imap_server_test.go
@@ -1,17 +1,20 @@
package inbox
import (
+ "bytes"
"context"
"fmt"
"io"
"log"
"net"
+ "os"
"path/filepath"
"strconv"
"strings"
"testing"
"time"
+ "github.com/emersion/go-imap"
"github.com/emersion/go-imap/backend/memory"
"github.com/emersion/go-imap/server"
@@ -221,3 +224,62 @@ func TestArchiveEmailsNothingToDo(t *testing.T) {
t.Error("archiving into a missing folder should fail")
}
}
+
+// Without UID EXPUNGE (go-imap v1.2.1), archiving by COPY then EXPUNGE removes
+// every \Deleted message in the mailbox, not just the ones being archived.
+// Mail another client already flagged would go too, so the code warns before
+// and after instead of doing it silently.
+func TestArchiveEmailsWarnsWhenOtherMailIsAlreadyFlaggedDeleted(t *testing.T) {
+ f := newIMAPFixture(t, false) // no MOVE: COPY + \Deleted + EXPUNGE
+ f.Deliver(t, "INBOX", "privacy@acme.example", "Re: first", "first")
+ f.Deliver(t, "INBOX", "privacy@acme.example", "Re: second", "second")
+ f.Mailbox(t, "Eraser") // the archive folder must exist for COPY
+
+ inbox := f.Mailbox(t, "INBOX")
+ n := len(inbox.Messages)
+ stray, ours := inbox.Messages[n-2], inbox.Messages[n-1]
+ stray.Flags = append(stray.Flags, imap.DeletedFlag)
+ before := f.Count(t, "INBOX")
+
+ var logs bytes.Buffer
+ log.SetOutput(&logs)
+ t.Cleanup(func() { log.SetOutput(os.Stderr) })
+
+ m := f.connect(t)
+ if err := m.ArchiveEmails([]uint32{ours.Uid}, "Eraser"); err != nil {
+ t.Fatal(err)
+ }
+ out := logs.String()
+ for _, want := range []string{
+ "1 message(s) besides the 1 just archived are already flagged \\Deleted",
+ "expunge removed 2 message(s) but this operation only intended to remove 1",
+ } {
+ if !strings.Contains(out, want) {
+ t.Errorf("log is missing %q:\n%s", want, out)
+ }
+ }
+ // This is the behaviour the warnings exist for: the stray message is gone too.
+ if got := f.Count(t, "INBOX"); got != before-2 {
+ t.Errorf("INBOX has %d messages, want %d", got, before-2)
+ }
+}
+
+// With nothing else flagged, archiving by COPY and EXPUNGE stays quiet.
+func TestArchiveEmailsWithoutStrayFlagsDoesNotWarn(t *testing.T) {
+ f := newIMAPFixture(t, false)
+ f.Deliver(t, "INBOX", "privacy@acme.example", "Re: only", "only")
+ f.Mailbox(t, "Eraser")
+ inbox := f.Mailbox(t, "INBOX")
+ ours := inbox.Messages[len(inbox.Messages)-1]
+
+ var logs bytes.Buffer
+ log.SetOutput(&logs)
+ t.Cleanup(func() { log.SetOutput(os.Stderr) })
+
+ if err := f.connect(t).ArchiveEmails([]uint32{ours.Uid}, "Eraser"); err != nil {
+ t.Fatal(err)
+ }
+ if strings.Contains(logs.String(), "WARNING") {
+ t.Errorf("unexpected warning:\n%s", logs.String())
+ }
+}