Skip to content

docs: define custom provider security boundary#1807

Merged
steipete merged 3 commits into
mainfrom
codex/custom-provider-design-1735
Jul 3, 2026
Merged

docs: define custom provider security boundary#1807
steipete merged 3 commits into
mainfrom
codex/custom-provider-design-1735

Conversation

@steipete

@steipete steipete commented Jul 1, 2026

Copy link
Copy Markdown
Owner

Summary

  • define a bounded config-only custom HTTP JSON provider MVP
  • document the required runtime ProviderInstanceID seam before arbitrary provider instances can coexist safely
  • specify endpoint, redirect, full-URL approval, secret, transport-isolation, response-size, mapping, redaction, and persistence boundaries
  • list exact non-goals, threat model, staged implementation slices, required proof, and owner decisions

Recommendation

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 .custom slot or a parallel UI path.

Validation

  • make docs-list
  • swift test --filter 'StatuspageSummaryTests|GoogleWorkspaceStatusTests|UsageStoreCoverageTests' (35 tests)
  • make check
  • make 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.

@clawsweeper

clawsweeper Bot commented Jul 1, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed July 3, 2026, 3:28 PM ET / 19:28 UTC.

Summary
Adds docs/custom-provider-design.md, defining a config-only custom HTTP JSON provider boundary, runtime provider-instance identity seam, security constraints, staged implementation slices, proof requirements, and accepted owner decisions.

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.

  • Docs-only surface: 1 added docs file, 0 runtime files changed. Maintainers can review this as product/security design acceptance rather than a runtime behavior change.

Root-cause cluster
Relationship: partial_overlap
Canonical: #1735
Summary: This PR defines the accepted design/security boundary for the broader custom-provider feature request, while the feature issue remains open for implementation follow-up.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦞 diamond lobster
Proof: 🌊 off-meta tidepool
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Risk before merge

  • [P1] Because the document is now marked accepted, merging it should be treated as accepting this custom-provider security and architecture boundary; implementation still needs to keep networking disabled until the staged identity, evaluator, transport, approval, and proof work lands.

Maintainer options:

  1. Decide the mitigation before merge
    Merge the doc only as the accepted boundary, then keep implementation under [Feature] Declarative "Custom Provider": bring-your-own endpoint + JSONPath field mapping #1735 with separate identity-seam, evaluator, transport, approval, and app-integration PRs before enabling networking.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • No automated repair is needed; this owner-authored docs-only PR is ready for ordinary maintainer merge review.

Security
Cleared: The diff is docs-only and introduces no runtime code, dependency, secret-handling, network, or supply-chain change.

Review details

Best 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 changes

Label justifications:

  • P3: This is low-risk design documentation for a speculative provider extensibility feature, not a broken current workflow.
  • rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🌊 off-meta tidepool and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: Real behavior proof is not required because this PR only changes files under docs/.
Evidence reviewed

What I checked:

  • Repository policy read: AGENTS.md was present and read fully; its guidance on docs, provider auth/privacy, avoiding live Keychain/provider probes, and keeping provider data siloed affected this review. (AGENTS.md:1, 289ae204fa6b)
  • PR metadata and discussion: GitHub reports this PR as open, owner-authored by steipete, mergeable, and changing one added docs file with 257 additions and no runtime files; the prior ClawSweeper comment was reviewed and the latest commit updates the document from proposed to accepted owner decisions. (docs/custom-provider-design.md:1, e20a1bbeda8d)
  • Docs-only boundary: The added document says the status is an accepted design boundary and explicitly states that it does not authorize runtime networking or implement the feature. (docs/custom-provider-design.md:10, e20a1bbeda8d)
  • Security boundary content: The document specifies HTTPS/auth rules, full URL approval, no inline secrets, isolated ephemeral transport, redirect rejection for the custom provider MVP, response-size bounds, redaction, and local-config-only definitions. (docs/custom-provider-design.md:124, e20a1bbeda8d)
  • Current main still uses fixed provider IDs: Current main models provider identity as a fixed UsageProvider enum, so the document's runtime ProviderInstanceID migration premise is still grounded in source. (Sources/CodexBarCore/Providers/Providers.swift:5, 289ae204fa6b)
  • Current config is still first-party shaped: Current main config remains version 1 and ProviderConfig.id is a UsageProvider, with fixed provider fields rather than arbitrary request and mapping definitions. (Sources/CodexBarCore/Config/CodexBarConfig.swift:3, 289ae204fa6b)

Likely related people:

  • steipete: Current-main blame for UsageProvider, ProviderConfig, provider registries, endpoint validation, and settings selection points to Peter Steinberger in this checkout, and he authored both design-doc commits for this PR. (role: central provider/config owner and current design author; confidence: high; commits: 913b1a6412b5, 10fd48654028, e20a1bbeda8d; files: Sources/CodexBarCore/Providers/Providers.swift, Sources/CodexBarCore/Config/CodexBarConfig.swift, Sources/CodexBarCore/Providers/ProviderDescriptor.swift)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. labels Jul 1, 2026
@steipete

steipete commented Jul 3, 2026

Copy link
Copy Markdown
Owner Author

Accepted and refreshed on exact head e20a1bbeda8d384b99ed476ca641893b886b6950.

Maintainer improvements:

  • converted the proposal into an accepted design boundary without authorizing runtime networking
  • resolved all five owner decisions: identity seam first; approval-gated unauthenticated loopback only; derived environment secret only; one primary window; CLI-first integration
  • rechecked current main, including the new settings sidebar, and documented why its provider-keyed selection strengthens the need for a shared runtime identity seam
  • retained the fail-closed implementation gate and prohibition on a shared .custom slot, parallel UI, or compatibility fallback

Proof:

  • make docs-list
  • make check
  • exact-head CI: 6 successful, macOS test job correctly skipped for docs-only scope, no failures

This lands architecture only. Custom-provider networking stays disabled until the independent identity, evaluator, transport, approval, and proof slices land.

@steipete steipete merged commit c500fc2 into main Jul 3, 2026
7 checks passed
@steipete steipete deleted the codex/custom-provider-design-1735 branch July 3, 2026 19:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature] Declarative "Custom Provider": bring-your-own endpoint + JSONPath field mapping

1 participant