fix(#7): allow POST git-upload-pack in default GitHub provider · Pull Request #8 · ggallen-sandbox/OpenShell

ghost · 2026-06-17T21:05:09Z

The default GitHub provider profile used access: read-only on the github.com:443 endpoint, which expands to GET/HEAD/OPTIONS only. Git's smart HTTP protocol requires POST to **/git-upload-pack for clone/fetch and POST to **/info/refs for ref discovery, so those operations were denied by the L7 proxy.

Replace the access: read-only preset on the github.com endpoint with explicit L7 rules that permit the standard read-only methods plus POST on **/git-upload-pack and **/info/refs. Push operations (git-receive-pack) remain blocked.

Updated the github_profile_materializes_policy_metadata test to reflect that github.com now uses explicit rules instead of the read-only access preset.

Note: Rust toolchain unavailable in sandbox — unit tests could not be compiled. Manual verification of openshell-providers tests is required.

Closes #7

Post-script verification

Branch is not main/master (agent/7-github-provider-git-upload-pack)
Secret scan passed (gitleaks — 09bd32f35a3b539e6aec895d4dec87d4ec030369..HEAD)
Pre-commit hooks passed (authoritative run on runner)
Tests ran inside sandbox

The default GitHub provider profile used `access: read-only` on the `github.com:443` endpoint, which expands to GET/HEAD/OPTIONS only. Git's smart HTTP protocol requires POST to `**/git-upload-pack` for clone/fetch and POST to `**/info/refs` for ref discovery, so those operations were denied by the L7 proxy. Replace the `access: read-only` preset on the `github.com` endpoint with explicit L7 rules that permit the standard read-only methods plus POST on `**/git-upload-pack` and `**/info/refs`. Push operations (`git-receive-pack`) remain blocked. Updated the `github_profile_materializes_policy_metadata` test to reflect that `github.com` now uses explicit rules instead of the `read-only` access preset. Note: Rust toolchain unavailable in sandbox — unit tests could not be compiled. Manual verification of openshell-providers tests is required. Closes #7 Signed-off-by: fullsend-code <289857995+devtest-coder[bot]@users.noreply.github.com>

ghost · 2026-06-17T21:07:10Z

🤖 Finished Review · ✅ Success · Started 9:07 PM UTC · Completed 9:17 PM UTC
Commit: 19f115c · View workflow run →

ghost · 2026-06-17T21:17:53Z

Review

Findings

High

[stale-reference] crates/openshell-server/src/grpc/policy.rs:4616 — The test provider_policy_layers_include_known_provider_profiles asserts that ALL github provider endpoints have access == "read-only" via .all(|endpoint| endpoint.access == "read-only"). After this PR changes github.com to use explicit rules instead of access: read-only, the endpoint will have an empty access field, causing this assertion to fail. The parallel test in profiles.rs was updated with a .filter(|endpoint| endpoint.host == "api.github.com") but this test was missed.
Remediation: Update the assertion at line 4616 to filter to api.github.com endpoints before checking access == "read-only" (mirroring the fix applied to the profiles.rs test), or replace the blanket assertion with targeted checks for each endpoint type.

Low

[edge-case] providers/github.yaml:50 — The rule { method: POST, path: "**/info/refs" } allows POST requests to /info/refs. In Git's smart HTTP protocol, /info/refs is only accessed via GET (with ?service=git-upload-pack). POST is used only for /git-upload-pack and /git-receive-pack directly. Since the GET ** rule already covers GET /info/refs, this POST rule is unnecessary and slightly widens the allowed surface.
[test-inadequate] crates/openshell-providers/src/profiles.rs:1695 — The new test assertions check .access.is_empty() and !.rules.is_empty(), but do not validate the actual content of the rules array (specific methods, paths). Consider asserting at least one critical rule (e.g., the POST git-upload-pack rule) to catch regressions in rule content.
[yaml_structure] providers/github.yaml:40 — The rules array is the first use of explicit L7 rules in any provider YAML file in the providers/ directory. This introduces a new pattern — not a problem, but worth noting for future consistency.
[design-smell] providers/github.yaml:34 — The github.com endpoint uses explicit L7 rules while api.github.com endpoints use the read-only access preset. The inconsistency is inherent and well-documented by the inline comment, but may warrant a git-read-only preset if this pattern recurs.

Info

[permission-expansion] providers/github.yaml:43 — The github.com:443 endpoint is upgraded from access: read-only (GET/HEAD/OPTIONS only) to explicit rules that additionally allow POST on **/git-upload-pack and **/info/refs. The expansion is justified by issue bug: gh repo clone fails with default github provider — POST git-upload-pack denied #7 and follows least-privilege for the stated use case.
[authorization] providers/github.yaml:51 — POST to **/info/refs is not part of the standard Git smart HTTP protocol (which uses GET for ref discovery). The practical security impact is negligible given the default-deny policy.

ghost

See the review comment for full details.

ghost · 2026-06-17T21:17:54Z

+      - allow:
+          method: OPTIONS
+          path: "**"
+      - allow:


[low] edge-case

The rule { method: POST, path: **/info/refs } allows POST requests to /info/refs. In Git smart HTTP protocol, /info/refs is only accessed via GET. POST is used only for /git-upload-pack and /git-receive-pack. Since the GET ** rule already covers GET /info/refs, this POST rule is unnecessary.

Suggested fix: Change the method from POST to GET, or remove this rule entirely since GET ** already covers it.

ghost · 2026-06-17T21:17:54Z

+            git_endpoint.access.is_empty(),
+            "github.com should use explicit rules, not an access preset"
+        );
+        assert!(


[low] test-inadequate

The new test assertions check .access.is_empty() and !.rules.is_empty(), but do not validate the actual content of the rules array (specific methods, paths).

Suggested fix: Add assertions that validate at least one critical rule (e.g., the POST git-upload-pack rule) to ensure the rules contain expected values.

ghost · 2026-06-17T21:17:54Z

    protocol: rest
-    access: read-only
    enforcement: enforce
+    rules:


[low] yaml_structure

The rules array is the first use of explicit L7 rules in any provider YAML file in the providers/ directory. This introduces a new pattern without similar examples for consistency.

ghost · 2026-06-17T21:17:54Z

    enforcement: enforce
  # github.com is the git transport (clone / fetch by default).
+  # Explicit rules instead of `access: read-only` so that POST to
+  # git-upload-pack (clone/fetch) is allowed while git-receive-pack


[low] design-smell

The github.com endpoint uses explicit L7 rules while api.github.com endpoints use the read-only access preset, creating inconsistent policy expression patterns within a single provider profile. The inconsistency is inherent and documented by the inline comment.

ghost · 2026-06-17T21:17:54Z

+    rules:
+      - allow:
+          method: GET
+          path: "**"


[info] permission-expansion

The github.com:443 endpoint is upgraded from access: read-only to explicit rules that additionally allow POST on **/git-upload-pack and **/info/refs. The expansion is justified by issue #7 and follows least-privilege for the stated use case.

ghost · 2026-06-17T21:17:54Z

+          method: OPTIONS
+          path: "**"
+      - allow:
+          method: POST


[info] authorization

POST to **/info/refs is not part of the standard Git smart HTTP protocol (which uses GET for ref discovery). The practical security impact is negligible given the default-deny policy.

ghost suggested changes Jun 17, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(#7): allow POST git-upload-pack in default GitHub provider#8

fix(#7): allow POST git-upload-pack in default GitHub provider#8
ghost wants to merge 1 commit into
mainfrom
agent/7-github-provider-git-upload-pack

ghost commented Jun 17, 2026

Uh oh!

ghost commented Jun 17, 2026 •

edited by ghost

Loading

Uh oh!

ghost commented Jun 17, 2026

Uh oh!

ghost left a comment

Uh oh!

ghost Jun 17, 2026

Uh oh!

ghost Jun 17, 2026

Uh oh!

ghost Jun 17, 2026

Uh oh!

ghost Jun 17, 2026

Uh oh!

ghost Jun 17, 2026

Uh oh!

ghost Jun 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

0 participants

Conversation

ghost commented Jun 17, 2026

Post-script verification

Uh oh!

ghost commented Jun 17, 2026 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ghost commented Jun 17, 2026

Review

Findings

High

Low

Info

Uh oh!

ghost left a comment

Choose a reason for hiding this comment

Uh oh!

ghost Jun 17, 2026

Choose a reason for hiding this comment

Uh oh!

ghost Jun 17, 2026

Choose a reason for hiding this comment

Uh oh!

ghost Jun 17, 2026

Choose a reason for hiding this comment

Uh oh!

ghost Jun 17, 2026

Choose a reason for hiding this comment

Uh oh!

ghost Jun 17, 2026

Choose a reason for hiding this comment

Uh oh!

ghost Jun 17, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

0 participants

ghost commented Jun 17, 2026 •

edited by ghost

Loading