docs: define custom provider security boundary#1807
Conversation
|
Codex review: needs maintainer review before merge. Reviewed July 3, 2026, 3:28 PM ET / 19:28 UTC. Summary Reproducibility: not applicable. This PR adds design documentation and no runtime behavior. Source inspection confirms current main still uses fixed provider IDs, fixed config shape, and exhaustive provider registries. Review metrics: 1 noteworthy metric.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Merge the doc only as the accepted boundary, then keep implementation under #1735 with separate identity-seam, evaluator, transport, approval, and app-integration PRs before enabling networking. Do we have a high-confidence way to reproduce the issue? Not applicable: this PR adds design documentation and no runtime behavior. Source inspection confirms current main still uses fixed provider IDs, fixed config shape, and exhaustive provider registries. Is this the best way to solve the issue? Yes for a docs-only boundary: the latest patch explicitly records accepted owner decisions while keeping runtime networking disabled until separate identity, evaluator, transport, approval, and proof slices land. The implementation itself still belongs in narrower follow-up PRs under the linked feature issue. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 289ae204fa6b. Label changesLabel justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
|
Accepted and refreshed on exact head Maintainer improvements:
Proof:
This lands architecture only. Custom-provider networking stays disabled until the independent identity, evaluator, transport, approval, and proof slices land. |
Summary
ProviderInstanceIDseam before arbitrary provider instances can coexist safelyRecommendation
Approve the security boundary and a separate identity-seam spike. Keep custom-provider networking disabled until the identity migration and pure transport/evaluator tests land independently. If the identity migration is too broad, close #1735 as unsupported rather than adding one shared
.customslot or a parallel UI path.Validation
make docs-listswift test --filter 'StatuspageSummaryTests|GoogleWorkspaceStatusTests|UsageStoreCoverageTests'(35 tests)make checkmake test/Users/steipete/Projects/agent-skills/skills/autoreview/scripts/autoreview --mode local ...No runtime behavior, provider auth, config version, or release metadata changes in this PR.
Issue #1735 intentionally remains open for owner acceptance and implementation follow-up.