Follow-up from the #285 review (inline finding on guardrails_merge.go).
MergeGuardrails intersects the agent's and platform's urlFilter.allowlist (and skillConstraints.allowedSkills) — fewer entries = stricter. But when the two lists are disjoint, the intersection is an empty-but-non-nil slice. Whether the guardrails library reads that as:
- "deny everything" (the correct ratchet — nothing passes the allowlist), or
- "no allowlist filtering" (a catastrophic loosen — everything passes)
depends on how the library interprets an empty-but-non-nil allowlist under the active Mode. The current code special-cases "platform empty = no opinion" but NOT the disjoint case.
Ask
- Determine the library's actual behavior for
Allowlist: []string{} (non-nil, empty) under allowlist / both modes — write a test pinning it.
- If empty means "allow all", the intersect MUST special-case disjoint → an explicit block-all (e.g. a sentinel deny), so a disjoint overlay never loosens.
- Same treatment for
skillConstraints.allowedSkills.
A code comment marking the caveat is in place (guardrails_merge.go, urlFilter allowlist block); remove it once resolved.
Related: #284, PR #285.
Follow-up from the #285 review (inline finding on
guardrails_merge.go).MergeGuardrailsintersects the agent's and platform'surlFilter.allowlist(andskillConstraints.allowedSkills) — fewer entries = stricter. But when the two lists are disjoint, the intersection is an empty-but-non-nil slice. Whether the guardrails library reads that as:depends on how the library interprets an empty-but-non-nil allowlist under the active
Mode. The current code special-cases "platform empty = no opinion" but NOT the disjoint case.Ask
Allowlist: []string{}(non-nil, empty) underallowlist/bothmodes — write a test pinning it.skillConstraints.allowedSkills.A code comment marking the caveat is in place (
guardrails_merge.go, urlFilter allowlist block); remove it once resolved.Related: #284, PR #285.