Skip to content

fix(credential-proxy): name the git-only proxy refusal instead of a 503 or a bare 404 - #3531

Merged
freeqaz merged 2 commits into
mainfrom
fix-2360
Oct 24, 2026
Merged

freeqaz merged 2 commits into
mainfrom
fix-2360

Conversation

@freeqaz

@freeqaz freeqaz commented Oct 24, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

With the documented protected OpenShell Git setup (repository.openShell, Credential Proxy selected from the Sandbox's registration, Helm openshellGit.mode: protected), no Agent could get a model credential, and the API did not say why. Seen on main 3154820cf on a k3d install on a remote dogfood host:

  • Creating an openai credential source answered 503 DEPENDENCY_UNAVAILABLE with the generic message. The real cause was only in the API log: "The selected OpenShell counterpart has no negotiated protected egress authority."
  • Deploying an Agent without a repository binding whose Gateway-owned openai source predates the switch answered 404 NOT_FOUND, with nothing logged.

Root cause: openShellProxyConfiguration makes repository.registrationName both the Git and the egress registration, and the request-phase Git service advertises only the base contract, never the execution profile. So every non-Git check of the Proxy (static source admission, any deploy without Git) can never pass. That is a permanent configuration state, not an outage. The 404 came from admitProxyPlan, which refused a binding without live reads (here, a Gateway source made before the Proxy was selected) as a ScopeViolationError.

This matches the existing design (docs/guides/credential-proxy.md, "Enroll prepared GitHub access": bundled v5/v6 Git uses an existing ordinary Gateway Harness source; fresh static registration under the Proxy is an egress source). What was missing was a refusal that says so, and the upgrade table read as if any retained Gateway openai source keeps working once a Proxy is selected.

Change

  • OpenShellCredentialProxyDriver.sourceControl.assertAvailable: inside the existing refusal branch (its condition is unchanged), when a non-Git profile is checked against the repository.openShell registration and that registration is present but advertises no execution capability at all, it throws the new CredentialProxyRepositoryOnlyError (409 RESOURCE_CONFLICT, naming repository.openShell and drivers.credential_proxy). A missing registration, or a partial or malformed execution advertisement, stays a 503.
  • admitStaticEgressSource checks the Proxy before reading the Secret, so source creation gets that refusal. Prepared and credential-free metadata sources hit the same refusal through their own checks.
  • admitProxyPlan: a binding without live reads (other than the Harness source beside Git) now raises the new CredentialSourceUnprotectedError (409, says to bind a source created or adopted for live reads) instead of a 404. For an Agent without Git, the Git-only refusal comes first, because replacing the source would not help there.
  • Provisioning status passes both fixed messages through (the classes are ResourceStateConflictErrors, so provisioning records them as PROVISIONING_REJECTED).
  • Docs: protected-openshell-git.md says this composition serves only Git and where a Git Agent's model source comes from. The upgrade table's static openai row adds the limit, and the guide qualifies "fresh static registration produces an egress source". The egress flow names the refusals; to stay under the word cap (2496 words), it drops the "in the accompanying …" ending from five older changelog entries.

There are no new error codes, no API contract change and no change to what is admitted: each refusal that used to be a 503 or a 404 is now a 409 with a message.

Verification

  • New integration test A Git-only Credential Proxy names repository.openShell instead of a 503 or a bare 404 (tests/integration/repository-credentials-admission.test.mjs) uses the bundled OpenShell Proxy and Gateway Drivers through the HTTP API, with stock 0.1.5 gateway info. On the unfixed code it fails with the reported 503 (source create) and 404 (deploy). It passes with the fix, including a control where the registration advertises the execution profile, a deploy with no source (previously 503), and a registration missing from gateway info (still 503). It also fails if the Proxy check in admitStaticEgressSource is moved back after the Secret checks.
  • repository-credentials-admission.test.mjs: 36/36 locally. The new test pinned 20/20 at 2 CPUs on a remote test VM. tsc --build, eslint and prettier are clean on the touched files. Docs word counts were checked with countMarkdownWords (the flow doc is at 2494).
  • tests/helpers/postgres-worker-revision-fixture.mjs matches the new binding refusal text.
  • Postgres application lane (it holds the fixture above) passed on a remote test VM.
  • Internal review (high effort): approve with nits; all nits addressed in the second commit.
  • feat(occdev): wire the OpenShell Git route into occ dev up with an installed acceptance #3045 and fix(repo): refuse installation-wide access next to another binding at agent save #3524: git merge-tree --name-only gives no new conflicted file. Run Codex workspace requests and approved GitHub fetches in one protected Sandbox #3349 (draft, already conflicting with main) gains a conflict only in docs/flows/credential-source-egress.md, which it rewrites throughout.

Risks

  • Precedence: an Agent without Git whose binding count mismatches now makes one gateway read before its refusal, but only on that error path.
  • Static source admission now reads gateway info before the Secret checks rather than after them. Callers still need create on the source first.
  • A non-Git revision prepared on a Git-only Proxy (only possible after changing the Installation) now raises the 409 class in the worker. It keeps the existing retry outcome (code DEPENDENCY_UNAVAILABLE); as a ScopeViolationError subclass it also counts as an unrecoverable read for the refusal re-read. No new worker status code is added; a permanent code would be an API addition for a follow-up.
  • The open question of whether a Git-only Proxy should instead fall back to Gateway delivery for non-Git Agents is a design decision and is not changed here.

…03 or a bare 404

With repository.openShell, the bundled OpenShell Credential Proxy's only
registration is the request-phase Git service, which never advertises the
execution profile. Creating a model credential source answered a generic
503, deploying an Agent without Git answered 503, and deploying one whose
Gateway source predates the Proxy answered 404 with nothing logged.

The Driver now refuses a non-Git profile on a registered Git-only service
with CredentialProxyRepositoryOnlyError (409 naming repository.openShell).
Static source admission checks the Proxy before reading the Secret. Deploy
refuses a binding without live reads with CredentialSourceUnprotectedError
(409) instead of a scope 404, and lets the Git-only refusal come first.
Docs: protected OpenShell Git, the upgrade table and the egress flow.
@freeqaz
freeqaz requested a review from a team as a code owner October 24, 2026 22:00
@clawsweeper

clawsweeper Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

…fusal

- Refuse with CredentialProxyRepositoryOnlyError only when the Git
  registration advertises no execution capability at all; a partial or
  malformed advertisement stays a 503 as before. The check now sits inside
  the existing refusal branch, leaving its condition unchanged.
- Word both messages for what the code checks: every Proxy-owned source
  (static, prepared, credential-free metadata), and sources created or
  adopted under the Proxy plus the ready Gateway Harness source beside Git.
- Pass both fixed messages through provisioning status.
- Test: create only on the stock gateway (the refusal precedes the Secret
  read), and a missing registration still answers 503.
- Docs: guide qualifier for fresh static registration; flow paragraph
  rewritten to fit the word cap.
@clawsweeper clawsweeper Bot added the clawsweeper:intake:bug Preliminary submission classification; full source review pending. label Oct 24, 2026
@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed clawsweeper:intake:bug Preliminary submission classification; full source review pending. labels Oct 24, 2026
@clawsweeper

clawsweeper Bot commented Oct 24, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge.

What this changes

This PR returns descriptive 409 conflicts for Credential Proxy refusals previously reported as generic 503 or 404 errors.

Example: Create an openai credential source with repository.openShell selecting the registered Git-only proxy.

  • Before: 503 DEPENDENCY_UNAVAILABLE with a generic message.
  • After: 409 RESOURCE_CONFLICT explaining that repository.openShell serves only repository Git and naming the settings an administrator can change.

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) A focused, source-supported repair with meaningful HTTP regression coverage and no identified defect; supplied behavioral evidence remains limited to the integration harness.
Proof confidence 🦐 gold shrimp (3/6) This collaborator-authored diagnostic repair is outside the ordinary contributor proof gate; supplied verification exercises Fastify admission and bundled Drivers with simulated Gateway discovery, rather than an installed after-fix run.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Product

Kind: Bug fix · Worth it: Yes · Fix scope: Complete
User problem: Operators receive generic outage or not-found errors when their selected credential-delivery composition cannot support the requested operation.
Reason: The collaborator-authored direction improves diagnosis within the existing contract without introducing fallback delivery, configuration options, or broader admission.

Merge readiness

✅ Ready for maintainer review

Keep open: this collaborator-authored PR provides a focused diagnostic repair that current main still lacks, with no actionable correctness or security findings identified.

Priority: P2
Reviewed head: 6c02a814d852a9ff46d2fc0812452552faa5fad3

Before merge

None.

Findings

None.

Tests

  • Missing end-to-end proof: No installed after-fix API trace is supplied for source creation and deployment under the Git-only configuration; base-fail/head-pass results are reported but were not rerun during this read-only review.
Agent review details

How this fits together

OCC authorizes credential-source creation and Agent deployment, then asks the selected Credential Proxy to validate delivery; incompatible compositions return errors before source or revision creation.

flowchart LR
  Caller[API caller] --> API[Fastify API]
  API --> OCC[OCC authorization and admission]
  OCC --> Proxy[Credential Proxy Driver]
  Proxy --> Gateway[OpenShell registration discovery]
  Gateway --> Proxy
  Proxy --> Refusal[Descriptive conflict response]
  Refusal --> Caller
Loading

Technical review

Best possible solution:

Preserve the existing credential-delivery boundary and return these specific, actionable conflicts through admission and provisioning status.

Do we have a high-confidence way to reproduce the issue?

Current-main composition selects the same registration for repository and ordinary egress discovery, explaining the generic 503; unmatched ordinary source bindings take the existing scope-error path that produces 404.

Is this the best way to solve the issue?

The repair uses the existing Driver discovery and OCC admission owners, preserves accepted configurations, and distinguishes a permanent composition limit from an unavailable counterpart.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against ec9d42e175aa.

Provenance checked

  • OpenShell capability refusal keeps the original intent (3115679: The Credential Proxy implementation requires negotiated execution capabilities for ordinary protected egress and distinct discovery for repository Git.)
  • Static source admission and unmatched source bindings keeps the original intent (3115679: Protected admission requires explicitly admitted sources, with a narrow ordinary Gateway Harness-source exception beside Git; the PR changes diagnosis and check ordering while retaining that rule.)
  • Provisioning message disclosure keeps the original intent (adb52c3: Provisioning exposes selected caller-safe refusal messages rather than arbitrary internal errors; the added fixed messages contain no resource values and fit the existing length limit.)

Testing

Proof path: in-process harness.

Security

None.

Evidence

What I checked:

Labels

Label changes:

  • add P2: Misleading credential admission errors create avoidable operator troubleshooting, while the existing refusal continues to protect unsupported configurations.
  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Label justifications:

  • P2: Misleading credential admission errors create avoidable operator troubleshooting, while the existing refusal continues to protect unsupported configurations.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Attach a redacted installed API trace showing the new conflicts for source creation and non-Git deployment under the Git-only composition.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

Reviewed October 24, 2026, 6:27 PM ET / 22:27 UTC.

@freeqaz
freeqaz merged commit 84a4767 into main Oct 24, 2026
60 checks passed
@freeqaz
freeqaz deleted the fix-2360 branch October 24, 2026 22:28
freeqaz added a commit that referenced this pull request Oct 25, 2026
… and named 409s (#3550)

#3514, #3516, #3517 and #3524 refuse at Agent save or admission what stored Agents may hold, and #3531 and #3534 turned 503 or bare 404 answers into named 409s, without breaking-change notices. This adds 2026-10-24 notices for them in the house format, and moves older 10-24 notices to a new archive page to keep the main page under its word cap. Docs only. Fixes board finding 2381.
freeqaz added a commit that referenced this pull request Oct 25, 2026
…without a driver and git-only proxy probes (#3552)

Mutant testing found repository refusal branches from #3524, #3531 and #3534 that no case could fail: installation-wide access beside another binding in any position, approval scopes with no repository Driver, and the Git-only Credential Proxy probes. repository-credentials-admission now pins them with table-driven cases. Test only.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. 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.

1 participant