Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 71 additions & 5 deletions crates/openshell-providers/src/profiles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1763,15 +1763,81 @@ mod tests {
"github profile should include read-only GraphQL endpoint"
);
assert!(
proto
.endpoints
.iter()
.all(|endpoint| endpoint.access == "read-only"),
"github profile endpoints should all be read-only"
proto.endpoints.iter().all(|endpoint| {
// The REST/GraphQL API endpoints stay read-only. The git
// transport endpoint (github.com) carries explicit rules
// instead so it can allow clone/fetch while blocking push.
if endpoint.host == "github.com" {
endpoint.access.is_empty()
} else {
endpoint.access == "read-only"
}
}),
"github API endpoints should be read-only; git transport uses explicit rules"
);
assert_eq!(proto.binaries.len(), 4);
}

#[test]
fn github_git_transport_allows_clone_but_not_push() {
let profile = builtin_profile("github");
let proto = profile.to_proto();

let git_transport = proto
.endpoints
.iter()
.find(|endpoint| endpoint.host == "github.com" && endpoint.port == 443)
.expect("github.com git transport endpoint");

// The git transport carries explicit rules rather than an access preset
// (an empty preset would otherwise expand to GET/HEAD/OPTIONS).
assert!(
git_transport.access.is_empty(),
"git transport must use explicit rules, not an access preset"
);

// Assert the EXACT allowed rule set. Clone/fetch over git smart HTTP
// performs GET */info/refs (ref discovery) followed by POST
// */git-upload-pack. A substring check alone is not enough: a broader or
// additional POST rule (e.g. POST **) would also permit push via
// git-receive-pack while still passing a "some rule allows upload-pack"
// check. Pinning the whole set fails on any such regression. See #1769.
let mut allowed: Vec<(&str, &str)> = git_transport
.rules
.iter()
.map(|rule| {
let allow = rule
.allow
.as_ref()
.expect("git transport rules must be allow rules");
(allow.method.as_str(), allow.path.as_str())
})
.collect();
allowed.sort_unstable();

let mut expected = vec![
("GET", "**"),
("HEAD", "**"),
("OPTIONS", "**"),
("POST", "/**/git-upload-pack"),
];
expected.sort_unstable();

assert_eq!(
allowed, expected,
"git transport allow rules must be exactly the read-only methods \
plus POST */git-upload-pack (clone/fetch); a broader or extra POST \
rule would enable push (git-receive-pack)"
);

// Blocking push must not depend on a deny rule, which could mask an
// over-broad allow and hide a regression.
assert!(
git_transport.deny_rules.is_empty(),
"git transport should block push via its narrow allow set, not deny rules"
);
}

#[test]
fn credential_env_vars_are_deduplicated_in_profile_order() {
let profile = builtin_profile("claude-code");
Expand Down
55 changes: 49 additions & 6 deletions crates/openshell-server/src/grpc/policy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5116,12 +5116,55 @@ mod tests {
"github provider policy should include read-only GraphQL endpoint"
);
assert!(
layers[0]
.rule
.endpoints
.iter()
.all(|endpoint| endpoint.access == "read-only"),
"github provider policy should be read-only by default"
layers[0].rule.endpoints.iter().all(|endpoint| {
// API endpoints stay read-only; the github.com git transport
// carries explicit rules so clone/fetch works (see #1769).
if endpoint.host == "github.com" {
endpoint.access.is_empty()
} else {
endpoint.access == "read-only"
}
}),
"github API endpoints should be read-only; git transport uses explicit rules"
);
// Pin the exact composed rule set for the git transport. Clone/fetch
// needs GET */info/refs then POST */git-upload-pack; a broader or extra
// POST rule (e.g. POST **) would also permit push (git-receive-pack), so
// matching the whole set fails on any such regression (see #1769).
let git_transport = layers[0]
.rule
.endpoints
.iter()
.find(|endpoint| endpoint.host == "github.com")
.expect("composed policy should include the github.com git transport");
let mut allowed: Vec<(&str, &str)> = git_transport
.rules
.iter()
.map(|rule| {
let allow = rule
.allow
.as_ref()
.expect("git transport rules must be allow rules");
(allow.method.as_str(), allow.path.as_str())
})
.collect();
allowed.sort_unstable();
let mut expected = vec![
("GET", "**"),
("HEAD", "**"),
("OPTIONS", "**"),
("POST", "/**/git-upload-pack"),
];
expected.sort_unstable();
assert_eq!(
allowed, expected,
"composed git transport allow rules must be exactly the read-only \
methods plus POST */git-upload-pack; a broader POST rule would \
enable push (git-receive-pack)"
);
assert!(
git_transport.deny_rules.is_empty(),
"composed git transport should block push via its narrow allow set, not deny rules"
);
}

Expand Down
55 changes: 55 additions & 0 deletions crates/openshell-supervisor-network/src/opa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3443,6 +3443,61 @@ network_policies:
assert!(!eval_l7(&engine, &get_with_method));
}

// Mirrors the default GitHub provider's github.com git-transport endpoint
// (providers/github.yaml). Git smart HTTP clone/fetch performs a GET on
// */info/refs followed by a POST to */git-upload-pack; push uses
// */git-receive-pack, which must stay blocked. Regression test for #1769.
#[test]
fn l7_github_git_transport_allows_clone_blocks_push() {
let data = r#"
network_policies:
github:
name: github
endpoints:
- host: github.com
port: 443
protocol: rest
enforcement: enforce
rules:
- allow: { method: GET, path: "**" }
- allow: { method: HEAD, path: "**" }
- allow: { method: OPTIONS, path: "**" }
- allow: { method: POST, path: "/**/git-upload-pack" }
binaries:
# l7_input() issues requests as /usr/bin/curl.
- { path: /usr/bin/curl }
"#;
let engine = OpaEngine::from_strings(TEST_POLICY, data).expect("engine from yaml");

// Reference discovery (GET) is allowed.
let refs = l7_input("github.com", 443, "GET", "/NVIDIA/OpenShell.git/info/refs");
assert!(eval_l7(&engine, &refs), "GET info/refs should be allowed");

// Clone/fetch (POST git-upload-pack) is allowed.
let upload_pack = l7_input(
"github.com",
443,
"POST",
"/NVIDIA/OpenShell.git/git-upload-pack",
);
assert!(
eval_l7(&engine, &upload_pack),
"POST git-upload-pack should be allowed for clone/fetch"
);

// Push (POST git-receive-pack) is denied.
let receive_pack = l7_input(
"github.com",
443,
"POST",
"/NVIDIA/OpenShell.git/git-receive-pack",
);
assert!(
!eval_l7(&engine, &receive_pack),
"POST git-receive-pack (push) must be denied"
);
}

#[test]
fn l7_jsonrpc_request_params_do_not_affect_method_policy() {
let data = r#"
Expand Down
14 changes: 12 additions & 2 deletions docs/sandboxes/providers-v2.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -605,11 +605,17 @@ endpoints:
- host: github.com
port: 443
protocol: rest
access: read-only
enforcement: enforce
rules:
- allow: { method: GET, path: "**" }
- allow: { method: HEAD, path: "**" }
- allow: { method: OPTIONS, path: "**" }
- allow: { method: POST, path: "/**/git-upload-pack" }
binaries: [/usr/bin/gh, /usr/local/bin/gh, /usr/bin/git, /usr/local/bin/git]
```

The `github.com` git-transport endpoint uses explicit rules instead of the `read-only` preset so HTTPS clone and fetch work out of the box. Git smart HTTP performs a `GET` on `*/info/refs` followed by a `POST` to `*/git-upload-pack`; the read-only preset (`GET`/`HEAD`/`OPTIONS`) blocks that `POST`. The rules permit the read-only methods plus `POST */git-upload-pack` only, so clone and fetch succeed while push (`git-receive-pack`) and other API mutations stay denied. Enabling push requires an explicit policy proposal.

If a sandbox attaches a provider named `work-github`, the effective policy includes a generated provider rule:

```yaml wordWrap showLineNumbers={false}
Expand Down Expand Up @@ -642,8 +648,12 @@ network_policies:
- host: github.com
port: 443
protocol: rest
access: read-only
enforcement: enforce
rules:
- allow: { method: GET, path: "**" }
- allow: { method: HEAD, path: "**" }
- allow: { method: OPTIONS, path: "**" }
- allow: { method: POST, path: "/**/git-upload-pack" }
binaries:
- path: /usr/bin/gh
- path: /usr/local/bin/gh
Expand Down
33 changes: 33 additions & 0 deletions e2e/python/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

from __future__ import annotations

import fcntl
import os
import time
from typing import TYPE_CHECKING
Expand All @@ -16,6 +17,38 @@
from collections.abc import Callable, Iterator


def pytest_configure(config: pytest.Config) -> None:
config.addinivalue_line(
"markers",
"exclusive_gateway_config: hold an exclusive lock on gateway-global "
"config so no other xdist worker observes a transient global setting",
)


@pytest.fixture(autouse=True)
def _gateway_config_guard(
request: pytest.FixtureRequest,
tmp_path_factory: pytest.TempPathFactory,
) -> Iterator[None]:
"""Readers-writer guard over gateway-global config mutations.

The e2e gateway is shared across xdist workers, so a test that flips a
gateway-global setting can leak that transient value into another worker's
sandbox creation. Every test holds a shared lock by default; a test marked
``exclusive_gateway_config`` holds an exclusive lock, so while it mutates and
restores the setting no other worker is mid-test and none can observe the
transient value. The lock file lives in the run's shared base temp dir
(``getbasetemp().parent``), which is common to all xdist workers.
"""
lock_path = tmp_path_factory.getbasetemp().parent / "gateway-config.lock"
exclusive = (
request.node.get_closest_marker("exclusive_gateway_config") is not None
)
with lock_path.open("w") as lock_file:
fcntl.flock(lock_file, fcntl.LOCK_EX if exclusive else fcntl.LOCK_SH)
yield


@pytest.fixture(scope="session")
def cluster_name() -> str | None:
return os.environ.get("OPENSHELL_GATEWAY")
Expand Down
Loading
Loading