feat: add cookie allowlist policy - #98
Conversation
Greptile SummaryThis PR adds an explicit
Confidence Score: 4/5Safe to merge. All sync boundaries remain fail-closed on policy load errors, backwards compatibility with existing blocklist.yaml files is preserved, and the allowlist path is covered by tests at every layer. The core policy logic is sound — internal/cli/sink.go — the HTTP error response body on line 199 still references "load blocklist" after the surrounding log messages were updated to "cookie policy". Important Files Changed
|
| if err != nil { | ||
| fmt.Fprintf(os.Stderr, "agentcookie sink: blocklist load failed: %v\n", err) | ||
| fmt.Fprintf(os.Stderr, "agentcookie sink: cookie policy load failed: %v\n", err) | ||
| recordSinkReject(sinkState, stateWriter, err) | ||
| http.Error(w, "load blocklist: "+err.Error(), http.StatusInternalServerError) |
There was a problem hiding this comment.
Stale "load blocklist" string in HTTP response body
The stderr log on the line above was updated to "cookie policy load failed", but the http.Error body still reads "load blocklist: ...". When a source operator is debugging a policy load failure, the HTTP response they receive will still say "load blocklist" while the sink log says "cookie policy" — the mismatch is especially confusing for allowlist policy errors where "blocklist" is semantically wrong. Consider changing this to "cookie policy load failed: " + err.Error().
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Summary
agentcookie can now run cookie sync in explicit allowlist mode for high-trust agent deployments. Existing users keep the same behavior: omitted
policy,policy: blocklist, missingblocklist.yaml, and empty blocklist domains all preserve the current sync-all/blocklist semantics.What Changed
blocklist.yamlacceptspolicy: blocklistorpolicy: allowlist; unknown policies fail closed with a field-specific error.cookies,cmux-sync, andagent-syncuse the same policy-aware matcher.status,doctor, and sink startup logs report sync-all, blocklist, or allowlist mode.accounts on/offremain blocklist-only and refuse to mutate allowlist policy files.Allowlist mode is defense in depth: source-side filtering avoids shipping non-allowed hosts, and sink-side filtering drops them again if they arrive anyway. An empty allowlist syncs no cookie hosts and is surfaced as a doctor warning.
Compatibility Notes
blocklist.yamlneeds to change.blocklist.yamlstill means sync-all.policystill drops matching patterns and passes everything else.allowlist.yamlmigration behavior is unchanged.Verified
go vet ./...go test ./...Post-Deploy Monitoring & Validation
cookie policy,load blocklist,non-allowlisted,blocklisted,agentcookie source: read,agentcookie sink: wrote.agentcookie statusshows the intendedcookie policy;agentcookie doctorreports sync-all/blocklist/allowlist clearly; allowlist-mode sinks report expected non-allowlisted drops without write failures.cookie policy: sync-all,load blocklistfailures, unexpected spike inTotalRejects, or agents losing access because a needed host was omitted from the allowlist.policy: allowlistor setpolicy: blocklistto return to legacy behavior, then rerunagentcookie doctorand one source/sink sync cycle.source --onceand one sink receipt before leaving the daemon unattended.Review Questions
blocklist.yamlacceptable for compatibility, or would you prefer a neutralcookie-policy.yamlin a follow-up?doctor, or should it be OK because it is fail-closed and sometimes intentional?