Skip to content

Commit b2c0b27

Browse files
committed
feat(provider): add a refresh strategy for github app installations
Signed-off-by: Gordon Sim <gsim@redhat.com>
1 parent cfcc373 commit b2c0b27

36 files changed

Lines changed: 2832 additions & 406 deletions

File tree

‎CI.md‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,26 @@ This runs the `mechanistic-proposal`, `new-hostname-proposal`, and
5555
gateway binaries and runtime images. This manual run does not replace the
5656
required PR E2E gate.
5757

58+
### Run only the GitHub App provider regression
59+
60+
The GitHub App sandbox regression runs in the curated `e2e:podman:ci` suite
61+
used by the branch Podman E2E job. It is also independently selectable:
62+
63+
```shell
64+
OPENSHELL_E2E_PODMAN_TEST=provider_github_app mise run e2e:podman
65+
```
66+
67+
It builds a tool image with `gh` and Git, then uses a local installation-token
68+
issuer and authenticated HTTPS API/Git fixture. It checks both clients across
69+
12 rotations, revocation after reconfiguration, and a fresh process using the
70+
replacement handle. No live GitHub app or installation is required. It needs
71+
the wrapper-managed gateway so it can temporarily trust the fixture CA; the
72+
original gateway configuration is restored afterward. The OAuth counterpart
73+
remains separately selectable as `provider_refresh_handles`. It uses an HTTP
74+
fixture and the standard workload image, and supports both wrapper-managed
75+
gateways and existing gateways selected with `OPENSHELL_GATEWAY_ENDPOINT`.
76+
It does not install CA trust or restart the gateway.
77+
5878
## Informational security reports
5979

6080
Security workflow compute runs directly on GitHub-hosted runners instead of

‎architecture/build.md‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,16 @@ test identity and tools, with Python aligned to the host test runner for
113113
serialized callable compatibility. Default-image coverage retains the product
114114
image. Other compute-driver test lanes retain their existing workload fixtures.
115115

116+
The Podman E2E gateway pins both the supervisor and sandbox runtime images
117+
selected by its wrapper, so a local test cannot mix a checkout's supervisor
118+
with a registry-default sandbox runtime. GitHub App credential tests run as a
119+
separate target with a local HTTPS API and Git backend, ephemeral CA trust, and
120+
real workload clients; they require no live GitHub credentials. The target is
121+
included in the curated Podman suite used by branch E2E CI.
122+
Only the GitHub App variant requires a wrapper-managed gateway to install CA
123+
trust. OAuth refresh-handle coverage uses HTTP and the standard workload image,
124+
so it also runs against an existing gateway without changing its configuration.
125+
116126
The Docker image pipeline is a two-step flow: build the Rust binary natively
117127
for the target architecture, then assemble the container image from the
118128
prebuilt binary. The gateway, sandbox, and supervisor images use distinct

‎architecture/gateway.md‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,13 @@ their profile payloads.
275275

276276
The CLI exposes reusable profile definitions through `openshell profile`, with `list` and `describe` reading the same effective catalog used by provider creation. Export, import, update, lint, and delete share that top-level command group. Import and lint accept local files, local directories, or a single HTTP or HTTPS URL; the CLI fetches and parses remote content before submitting it to the gateway. Workspace selection and explicit platform scope apply at the existing profile API boundary; `openshell provider` manages credential-bearing instances.
277277

278+
CLI warnings for credentials passed through `--env` derive setup suggestions from
279+
that catalog. Static credentials retain existing-token setup; gateway-mintable
280+
credentials recommend runtime creation and explicit refresh configuration when
281+
the profile supports runtime creation. Other runtime setups use generic provider
282+
guidance. Shared environment aliases do not identify the authentication method:
283+
GitHub tokens and GitHub App installations remain separate choices.
284+
278285
Each logical gateway request captures the selected sources into one validated,
279286
immutable effective catalog before deriving provider behavior. Policy layers,
280287
credential scope, injected environment material, dynamic token grants, and
@@ -790,6 +797,28 @@ credential storage for defense in depth. Multi-replica deployments can use that
790797
default with a shared database and shared key-encryption key, or opt into an
791798
external backend such as Vault or Kubernetes Secrets.
792799

800+
GitHub App installation refresh signs an app JWT at the gateway and exchanges it
801+
for an installation token using a profile-owned endpoint and its default REST API
802+
version, allowing GitHub Enterprise Server endpoints to use their supported version.
803+
Each provider pins one installation and explicit repository IDs and permissions
804+
in its refresh material; there is no workload-selected scope or installation-wide
805+
default. The token's logical profile name resolves to the configured environment
806+
alias, or the first declared alias before configuration. Refresh configuration
807+
reserves one alias per logical GitHub App credential under the provider mutation
808+
lock, including pending and failed grants. Management operations use the same
809+
resolution; exact-key deletion remains available to clean up legacy conflicts.
810+
The app key is secret material, and only the installation token enters the
811+
existing credential distribution path. Refresh honors the issuer expiry, caps
812+
local use at one hour, and preserves the existing authorization epoch during
813+
routine rotation. Explicit
814+
reconfiguration revokes the old workload handles. Network policy remains an
815+
independent constraint on token use. GitHub mint failures use gateway-owned
816+
recovery classifications without storing issuer-controlled error prose.
817+
Transport diagnostics classify typed timeout, TLS, and connection failures
818+
without exposing request material or raw error chains.
819+
The Go SDK exposes this strategy as `v1.RefreshStrategyGitHubAppInstallation`
820+
through its curated `openshell/v1` package for use in `v1.RefreshConfig`.
821+
793822
The Vault credential driver requires HTTPS for every non-loopback backend,
794823
never follows HTTP redirects, and keeps standard certificate hostname
795824
verification enabled. Operators can add private Vault trust roots with a PEM

‎crates/openshell-cli/src/commands/common.rs‎

Lines changed: 182 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -756,6 +756,9 @@ pub fn parse_duration_to_ms(s: &str) -> Result<i64> {
756756
pub struct ProfileSuggestion {
757757
pub provider_type: String,
758758
pub credential: String,
759+
credential_key: String,
760+
display_name: String,
761+
refresh: Option<openshell_providers::CredentialRefreshProfile>,
759762
}
760763

761764
fn credential_env_matches(
@@ -786,10 +789,23 @@ fn credential_env_matches(
786789
let mut suggestions = Vec::new();
787790
for profile in profiles {
788791
for cred in &profile.credentials {
789-
if cred.env_vars.iter().any(|v| v.eq_ignore_ascii_case(key)) {
792+
if let Some(credential_key) =
793+
cred.env_vars.iter().find(|v| v.eq_ignore_ascii_case(key))
794+
{
795+
let refresh = cred.refresh.as_ref().filter(|r| r.is_gateway_mintable());
796+
// Do not offer a static shortcut for runtime credentials.
797+
// Mixed profiles and token grants need their full setup guide.
798+
if cred.is_runtime_resolvable()
799+
&& (refresh.is_none() || !profile.allows_runtime_provider_credentials())
800+
{
801+
continue;
802+
}
790803
suggestions.push(ProfileSuggestion {
791804
provider_type: profile.id.clone(),
792805
credential: cred.name.clone(),
806+
credential_key: credential_key.clone(),
807+
display_name: profile.display_name.clone(),
808+
refresh: refresh.cloned(),
793809
});
794810
}
795811
}
@@ -801,7 +817,13 @@ fn credential_env_matches(
801817

802818
for key in env.keys() {
803819
let sug = profile_suggestions(key);
804-
if !sug.is_empty() || looks_like_credential(key) {
820+
let known_credential = profiles.iter().any(|profile| {
821+
profile
822+
.credentials
823+
.iter()
824+
.any(|cred| cred.env_vars.iter().any(|v| v.eq_ignore_ascii_case(key)))
825+
});
826+
if known_credential || looks_like_credential(key) {
805827
matches.push((key.clone(), sug));
806828
}
807829
}
@@ -825,34 +847,78 @@ pub fn warn_credential_env_vars(
825847
return;
826848
}
827849

828-
let matches = credential_env_matches(env, profiles);
829-
if matches.is_empty() {
830-
return;
850+
for warning in credential_env_warnings(env, profiles) {
851+
eprintln!("{warning}");
831852
}
853+
}
832854

833-
for (key, suggestions) in &matches {
834-
eprintln!(
835-
"{} {key} looks like a credential passed as a plain environment variable.",
836-
"⚠".yellow()
837-
);
838-
eprintln!(" The agent inside the sandbox can read this value directly.");
839-
eprintln!();
855+
fn credential_env_warnings(
856+
env: &HashMap<String, String>,
857+
profiles: &[openshell_providers::ProviderTypeProfile],
858+
) -> Vec<String> {
859+
let mut warnings = Vec::new();
860+
for (key, suggestions) in credential_env_matches(env, profiles) {
861+
let mut lines = vec![
862+
format!(
863+
"{} {key} looks like a credential passed as a plain environment variable.",
864+
"⚠".yellow()
865+
),
866+
" The agent inside the sandbox can read this value directly.".to_owned(),
867+
String::new(),
868+
];
840869

841870
if suggestions.is_empty() {
842-
eprintln!(" To hide it from the agent, use a provider instead of --env.");
871+
lines.push(" To hide it from the agent, use a provider instead of --env.".to_owned());
843872
} else {
844-
eprintln!(" To hide it from the agent, use a provider instead:");
845-
for s in suggestions {
846-
eprintln!(
847-
" openshell provider create --name my-{ty} --type {ty} --credential {key}",
848-
ty = s.provider_type
849-
);
873+
lines.push(" To hide it from the agent, choose a provider setup that matches your authentication:".to_owned());
874+
for s in &suggestions {
875+
if let Some(refresh) = &s.refresh {
876+
let strategy =
877+
super::provider::provider_refresh_strategy_name(refresh.strategy)
878+
.replace('_', "-");
879+
lines.push(format!(
880+
" {} ({strategy}, gateway-managed refresh):",
881+
s.display_name
882+
));
883+
lines.push(format!(
884+
" openshell provider create --name my-{ty} --type {ty} --runtime-credentials",
885+
ty = s.provider_type
886+
));
887+
lines.push(format!(
888+
" Then run openshell provider refresh configure my-{} --credential-key {} --strategy {strategy}",
889+
s.provider_type, s.credential_key
890+
));
891+
let material = refresh
892+
.material
893+
.iter()
894+
.filter(|m| m.required)
895+
.map(|m| m.name.as_str())
896+
.collect::<Vec<_>>();
897+
if !material.is_empty() {
898+
lines.push(format!(
899+
" Supply required refresh material: {}. Use --secret-material-env for secrets.",
900+
material.join(", ")
901+
));
902+
}
903+
lines.push(" Follow the refresh setup guide before attaching this provider to a sandbox.".to_owned());
904+
} else {
905+
lines.push(format!(
906+
" {} (existing token or credential):",
907+
s.display_name
908+
));
909+
lines.push(format!(
910+
" openshell provider create --name my-{ty} --type {ty} --credential {key}",
911+
ty = s.provider_type
912+
));
913+
}
850914
}
851-
eprintln!(" openshell sandbox create --provider my-<name> ...");
915+
lines.push(" openshell sandbox create --provider my-<name> ...".to_owned());
852916
}
853-
eprintln!(" See: {DOCS_PROVIDERS_URL}");
854-
eprintln!();
917+
lines.push(format!(" See: {DOCS_PROVIDERS_URL}"));
918+
lines.push(String::new());
919+
warnings.push(lines.join("\n"));
855920
}
921+
warnings
856922
}
857923

858924
pub fn parse_key_value_pairs(items: &[String], flag: &str) -> Result<HashMap<String, String>> {
@@ -1181,13 +1247,15 @@ mod tests {
11811247
assert_eq!(prof[0].0, "GITHUB_TOKEN");
11821248

11831249
let sug = &prof[0].1;
1184-
assert_eq!(sug.len(), 2_usize);
1250+
assert_eq!(sug.len(), 3_usize);
11851251

11861252
assert_eq!(sug[0].provider_type, "copilot");
11871253
assert_eq!(sug[0].credential, "api_token");
11881254

11891255
assert_eq!(sug[1].provider_type, "github");
11901256
assert_eq!(sug[1].credential, "api_token");
1257+
assert_eq!(sug[2].provider_type, "github-app");
1258+
assert_eq!(sug[2].credential, "api_token");
11911259
}
11921260

11931261
#[test]
@@ -1199,13 +1267,103 @@ mod tests {
11991267
assert_eq!(prof[0].0, "gh_token");
12001268

12011269
let sug = &prof[0].1;
1202-
assert_eq!(sug.len(), 2_usize);
1270+
assert_eq!(sug.len(), 3_usize);
12031271

12041272
assert_eq!(sug[0].provider_type, "copilot");
12051273
assert_eq!(sug[0].credential, "api_token");
12061274

12071275
assert_eq!(sug[1].provider_type, "github");
12081276
assert_eq!(sug[1].credential, "api_token");
1277+
assert_eq!(sug[2].provider_type, "github-app");
1278+
assert_eq!(sug[2].credential, "api_token");
1279+
}
1280+
1281+
#[test]
1282+
fn credential_warning_distinguishes_existing_github_tokens_from_app_refresh() {
1283+
for key in ["GITHUB_TOKEN", "GH_TOKEN", "gh_token"] {
1284+
let warnings = credential_env_warnings(&env(&[(key, "secretVALUE42")]), catalog());
1285+
let warning = &warnings[0];
1286+
assert!(warning.contains(&format!(
1287+
"openshell provider create --name my-github --type github --credential {key}"
1288+
)));
1289+
assert!(warning.contains("GitHub (existing token or credential)"));
1290+
assert!(
1291+
warning.contains("GitHub App (github-app-installation, gateway-managed refresh)")
1292+
);
1293+
assert!(warning.contains(
1294+
"openshell provider create --name my-github-app --type github-app --runtime-credentials"
1295+
));
1296+
assert!(warning.contains(&format!(
1297+
"openshell provider refresh configure my-github-app --credential-key {} --strategy github-app-installation",
1298+
key.to_ascii_uppercase()
1299+
)));
1300+
assert!(
1301+
warning.contains(
1302+
"client_id, installation_id, private_key, repository_ids, permissions"
1303+
)
1304+
);
1305+
assert!(warning.contains("--secret-material-env"));
1306+
assert!(warning.contains(DOCS_PROVIDERS_URL));
1307+
assert!(!warning.contains("--type github-app --credential"));
1308+
assert!(!warning.contains("secretVALUE42"));
1309+
}
1310+
}
1311+
1312+
#[test]
1313+
fn credential_warning_uses_profile_capabilities_not_its_name() {
1314+
let mut profile = catalog()
1315+
.iter()
1316+
.find(|p| p.id == "github-app")
1317+
.unwrap()
1318+
.clone();
1319+
profile.id = "enterprise-repos".to_owned();
1320+
let env = env(&[("GITHUB_TOKEN", "secretVALUE42")]);
1321+
let warning = credential_env_warnings(&env, &[profile.clone()]).join("\n");
1322+
assert!(warning.contains("--type enterprise-repos --runtime-credentials"));
1323+
assert!(warning.contains("--strategy github-app-installation"));
1324+
1325+
// A refresh declaration alone does not imply gateway minting.
1326+
for strategy in [
1327+
openshell_core::proto::ProviderCredentialRefreshStrategy::Static,
1328+
openshell_core::proto::ProviderCredentialRefreshStrategy::External,
1329+
] {
1330+
profile.credentials[0].refresh.as_mut().unwrap().strategy = strategy;
1331+
let warning = credential_env_warnings(&env, &[profile.clone()]).join("\n");
1332+
assert!(warning.contains("--type enterprise-repos --credential GITHUB_TOKEN"));
1333+
assert!(!warning.contains("--runtime-credentials"));
1334+
assert!(!warning.contains("refresh configure"));
1335+
}
1336+
}
1337+
1338+
#[test]
1339+
fn credential_warning_does_not_offer_invalid_runtime_creation_for_mixed_profile() {
1340+
let mut profile = catalog()
1341+
.iter()
1342+
.find(|p| p.id == "github-app")
1343+
.unwrap()
1344+
.clone();
1345+
let mut static_credential = profile.credentials[0].clone();
1346+
static_credential.name = "other_secret".to_owned();
1347+
static_credential.env_vars = vec!["OTHER_SECRET".to_owned()];
1348+
static_credential.refresh = None;
1349+
profile.credentials.push(static_credential);
1350+
// A known alias must still warn even without a credential keyword.
1351+
profile.credentials[0].env_vars = vec!["CUSTOM_AUTH".to_owned()];
1352+
let warning = credential_env_warnings(&env(&[("CUSTOM_AUTH", "x")]), &[profile]).join("\n");
1353+
assert!(warning.contains("CUSTOM_AUTH looks like a credential"));
1354+
assert!(warning.contains("use a provider instead of --env"));
1355+
assert!(!warning.contains("provider create"));
1356+
}
1357+
1358+
#[test]
1359+
fn credential_warning_without_catalog_is_generic_and_does_not_leak_values() {
1360+
let warnings = credential_env_warnings(&env(&[("APP_SECRET", "secretVALUE42")]), &[]);
1361+
assert_eq!(warnings.len(), 1);
1362+
assert!(warnings[0].contains("APP_SECRET looks like a credential"));
1363+
assert!(warnings[0].contains("use a provider instead of --env"));
1364+
assert!(!warnings[0].contains("provider create"));
1365+
assert!(!warnings[0].contains("secretVALUE42"));
1366+
assert!(credential_env_warnings(&env(&[("PATH", "/usr/bin")]), &[]).is_empty());
12091367
}
12101368

12111369
#[test]

‎crates/openshell-cli/src/commands/provider.rs‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2065,6 +2065,7 @@ fn provider_refresh_strategy(strategy: &str) -> Result<ProviderCredentialRefresh
20652065
Ok(ProviderCredentialRefreshStrategy::GoogleServiceAccountJwt)
20662066
}
20672067
"aws_sts_assume_role" => Ok(ProviderCredentialRefreshStrategy::AwsStsAssumeRole),
2068+
"github_app_installation" => Ok(ProviderCredentialRefreshStrategy::GithubAppInstallation),
20682069
_ => Err(miette!("unsupported provider refresh strategy: {strategy}")),
20692070
}
20702071
}
@@ -2108,14 +2109,17 @@ fn provider_refresh_recovery_action_name(
21082109
}
21092110
}
21102111

2111-
fn provider_refresh_strategy_name(strategy: ProviderCredentialRefreshStrategy) -> &'static str {
2112+
pub(super) fn provider_refresh_strategy_name(
2113+
strategy: ProviderCredentialRefreshStrategy,
2114+
) -> &'static str {
21122115
match strategy {
21132116
ProviderCredentialRefreshStrategy::Static => "static",
21142117
ProviderCredentialRefreshStrategy::External => "external",
21152118
ProviderCredentialRefreshStrategy::Oauth2RefreshToken => "oauth2_refresh_token",
21162119
ProviderCredentialRefreshStrategy::Oauth2ClientCredentials => "oauth2_client_credentials",
21172120
ProviderCredentialRefreshStrategy::GoogleServiceAccountJwt => "google_service_account_jwt",
21182121
ProviderCredentialRefreshStrategy::AwsStsAssumeRole => "aws_sts_assume_role",
2122+
ProviderCredentialRefreshStrategy::GithubAppInstallation => "github_app_installation",
21192123
ProviderCredentialRefreshStrategy::Unspecified => "unspecified",
21202124
}
21212125
}

0 commit comments

Comments
 (0)