Skip to content

fix(repository): accept GitHub's spelling of the repository in metadata reads - #3282

Merged
freeqaz merged 2 commits into
mainfrom
fix-2239
Oct 23, 2026
Merged

freeqaz merged 2 commits into
mainfrom
fix-2239

Conversation

@freeqaz

@freeqaz freeqaz commented Oct 23, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

Finding 2239: repository metadata reads compare GitHub's full_name exactly with the registry's repository name. The registry lowercases owner/repository, but GitHub answers GET /repos/<name> with the repository's own spelling. So for a repository with capitals in its name, both the metadata-read response policy (metadata-response.ts) and the OpenShell metadata provider (egress-proxy/github-metadata.ts) reject the reply, and every metadata read fails. This is the same root cause as 2232/2236 (#3278, #3280).

Change

  • Both checks now use the shared isSameGitHubRepository helper from drivers/repo/credentials/client-contracts.ts (added by fix(repository): accept GitHub's spelling of the repository in native draft PR receipts #3280). There is no new copy. The egress proxy already imports from drivers/repo/credentials/, and the repository-credentials boundary check scans only credential sources, so it passes unchanged.
  • Only the ASCII letter case may differ. The owner/repository shape, ASCII-only names and the numeric repository ID check stay exact.
  • Both paths still release the admitted (registry) name, not GitHub's spelling. The egress provider used to echo the upstream full_name, which was always equal to the admitted name; it now returns operation.repository explicitly, so the released body is unchanged.
  • Test fixture: the GitHub protocol fixture gains a metadataResponse body hook, like the existing draftResponse.

Verification

  • tests/integration/protected-repository-metadata-read.test.mjs: a reply in GitHub's spelling is accepted through the real route and sender, and the recipient gets the configured lowercase identity. This holds for both mixed and all-caps spellings. A .git suffix and a non-string full_name are refused.
  • tests/integration/egress-prepared-github-metadata.test.mjs: the prepared OpenShell metadata exchange accepts Fixture/Repository and FIXTURE/REPOSITORY and releases fixture/repository. It blocks fixture/repository-other, fixture/repository.git, a non-ASCII lookalike (ſ) and a null name.
  • Internal review: approved. I applied its optional test nits.
  • Both new cases fail on main and pass here. Both files pinned 20/20 at 2 CPUs.
  • The repository-credentials conformance and HTTP suites pass, as does the workspace and credential boundary check. tsc -b, eslint and prettier are clean on the touched files.

Risks

Low. The comparison widens only by ASCII letter case on a reply that also has to carry the admitted numeric repository ID. The released body is unchanged.

…ta reads

The metadata-read response policy and the OpenShell metadata provider compared
GitHub's full_name exactly with the lowercased registry name, so a repository
with capitals in its name failed every metadata read (finding 2239). Both now
use the shared isSameGitHubRepository helper and still release the admitted
registry name; the ID check and the ASCII owner/repository shape stay exact.
@freeqaz
freeqaz requested a review from a team as a code owner October 23, 2026 07:10
@clawsweeper

clawsweeper Bot commented Oct 23, 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.

…ames

Review nits: the OpenShell metadata provider also refuses a non-ASCII
lookalike and a null full_name, and the metadata-read route accepts the
all-caps spelling.
@clawsweeper clawsweeper Bot added clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-environment Preliminary submission classification; full source review pending. clawsweeper:intake:needs-repro Preliminary submission classification; full source review pending. 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. clawsweeper:intake:needs-environment Preliminary submission classification; full source review pending. clawsweeper:intake:needs-repro Preliminary submission classification; full source review pending. labels Oct 23, 2026
@clawsweeper

clawsweeper Bot commented Oct 23, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge.

What this changes

This PR accepts ASCII case differences in GitHub repository metadata while returning the registered repository name.

Example: The registered identity is fixture/repository with ID 73, and GitHub returns Fixture/Repository with ID 73.

  • Before: Both metadata response validators reject the reply.
  • After: Both accept the reply and release {"id":73,"full_name":"fixture/repository"}.

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) A useful, focused repair with meaningful integration scenarios and no actionable defect found; behavior evidence remains limited to reported harness results.
Proof confidence 🦐 gold shrimp (3/6) This collaborator-authored PR is outside the ordinary contributor proof gate; inspected coverage exercises production validators through local route/sender sockets and a protected-exchange harness with simulated GitHub, rather than an installed deployment.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Product

Kind: Bug fix · Worth it: Yes · Fix scope: Complete
User problem: An Agent cannot read valid repository metadata when GitHub capitalizes the owner or repository name.
Reason: This collaborator-authored repair restores valid metadata reads within the existing identity contract and follows the direction established by the merged draft-response repairs.

Merge readiness

✅ Ready for maintainer review

Keep open: current main still rejects valid metadata replies with GitHub's capitalization, and this PR provides a focused repair with no concrete introduced defect found.

Priority: P2
Reviewed head: 0570380d7aba9d781eda30e3ebcb5c26711030db

Before merge

None.

Findings

None.

Tests

  • Missing end-to-end proof: The PR reports base-fail/head-pass integration results but supplies no live installed metadata-read trace against GitHub.
Agent review details

How this fits together

Repository metadata passes through the repository credential sender or protected egress provider, which validates GitHub's response and releases only the admitted repository identity.

flowchart TD
  A[Agent metadata request] --> B[Repository route authorization]
  B --> C[GitHub metadata GET]
  C --> D[Response identity validation]
  D --> E[Registered identity response]
  D --> F[Blocked invalid response]
Loading

Technical review

Best possible solution:

Land the shared comparison repair through both existing response owners while preserving exact repository IDs and admitted-name output.

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

Current-main source deterministically rejects Fixture/Repository for registered fixture/repository despite a matching numeric ID; the added scenarios cover that input, but this read-only review ran no target code.

Is this the best way to solve the issue?

The patch fixes both affected response owners with the already approved shared comparison, without introducing another identity policy or changing the released response shape.

AGENTS.md: found and applied where relevant.

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

Provenance checked

  • Repository credential metadata response policy changes intended behavior with a stated reason (3115679: Release only the configured repository identity after validating the provider response.)
  • Protected egress metadata validation and projection changes intended behavior with a stated reason (3115679: Constrain protected metadata responses to the admitted repository; exact name comparison previously made upstream spelling equal to the released name.)
  • Metadata integration scenarios and GitHub fixture keeps the original intent (3115679: Exercise exact metadata delivery and reject responses outside the admitted identity and response envelope.)

Testing

Proof path: in-process harness.

Security

None.

Evidence

What I checked:

Review metrics

Metric Value Why it matters
Production and test scope Production +13/-4 lines; tests and fixture +50/-4 lines Small production changes reuse an existing helper, and the additional coverage protects successful normalization and invalid-response rejection.

Labels

Label changes:

  • add P2: Valid metadata reads fail for repositories with capital letters, with impact limited to the repository metadata workflow.
  • 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: Valid metadata reads fail for repositories with capital letters, with impact limited to the repository metadata workflow.
  • 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.

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 23, 2026, 3:35 AM ET / 07:35 UTC.

@freeqaz
freeqaz merged commit eea5257 into main Oct 23, 2026
78 of 79 checks passed
@freeqaz
freeqaz deleted the fix-2239 branch October 23, 2026 07:37
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