mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-10-06 02:48:25 +00:00
Merge pull request #679 from swerner/fix/remove-workflows-write-permission
Stop requesting GitHub workflow write permission
This commit is contained in:
commit
18f07f4d4e
4 changed files with 29 additions and 27 deletions
|
|
@ -55,7 +55,6 @@ When you choose the GitHub App strategy, the CLI opens GitHub with a pre-filled
|
|||
| Permission | Level | Purpose |
|
||||
|---|---|---|
|
||||
| Contents | Write | Clone repos, push run branches and checkpoints |
|
||||
| Workflows | Write | Push changes under `.github/workflows/` |
|
||||
| Metadata | Read | Look up repository installation status |
|
||||
| Pull requests | Write | Create and update PRs from workflows |
|
||||
| Checks | Write | Report workflow status on commits |
|
||||
|
|
@ -66,6 +65,8 @@ When you choose the GitHub App strategy, the CLI opens GitHub with a pre-filled
|
|||
|
||||
These permissions are included when Fabro registers a new app. For an existing GitHub App, add the missing permissions in the app's settings, then approve the permission update on each installation before workflows can use them.
|
||||
|
||||
Fabro's App manifest does not request the Workflows permission, so Apps registered through Fabro cannot publish changes under `.github/workflows/`.
|
||||
|
||||
The installer:
|
||||
|
||||
1. Lets you choose where to register the app. If you have the `gh` CLI installed, Fabro detects your GitHub username and any organizations you administer and lets you pick. If `gh` is not available, the installer prompts for a GitHub token and uses it to look up your identity and organizations.
|
||||
|
|
@ -220,7 +221,7 @@ When a workflow runs in a remote sandbox (Daytona or Docker), Fabro clones the c
|
|||
2. SSH URLs (e.g. `git@github.com:owner/repo.git`) are converted to HTTPS
|
||||
3. Fabro signs a short-lived JWT using the App ID and private key (RS256, 10-minute validity)
|
||||
4. Using the JWT, Fabro looks up the GitHub App installation for the repository (`GET /repos/\{owner\}/\{repo\}/installation`)
|
||||
5. Fabro requests a scoped Installation Access Token with `contents: write` and `workflows: write` permissions on the specific repository
|
||||
5. Fabro requests a scoped Installation Access Token with `contents: write` permission on the specific repository
|
||||
6. The sandbox clones via HTTPS using `x-access-token` as the username and the token as the password
|
||||
|
||||
For public repositories, the clone works without credentials. The token is still generated because it's needed for pushing checkpoints.
|
||||
|
|
|
|||
|
|
@ -2684,6 +2684,16 @@ client_id = "client-id"
|
|||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn manifest_excludes_workflows_permission() {
|
||||
let manifest = build_github_app_manifest("Fabro-test", 12345, "https://app.example.com");
|
||||
|
||||
assert!(
|
||||
manifest["default_permissions"].get("workflows").is_none(),
|
||||
"GitHub App must not be able to write workflow files"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn persist_install_outputs_persists_vault_secrets_via_server_when_autostarting() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
|
|
|
|||
|
|
@ -2053,7 +2053,6 @@ fn build_github_app_manifest(
|
|||
"public": false,
|
||||
"default_permissions": {
|
||||
"contents": "write",
|
||||
"workflows": "write",
|
||||
"metadata": "read",
|
||||
"pull_requests": "write",
|
||||
"checks": "write",
|
||||
|
|
@ -2362,7 +2361,7 @@ mod tests {
|
|||
};
|
||||
|
||||
#[test]
|
||||
fn github_app_manifest_allows_workflow_file_writes() {
|
||||
fn github_app_manifest_excludes_workflows_permission() {
|
||||
let manifest = build_github_app_manifest(
|
||||
"Fabro Test",
|
||||
"https://fabro.example/setup",
|
||||
|
|
@ -2370,9 +2369,9 @@ mod tests {
|
|||
"https://fabro.example/setup",
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
manifest["default_permissions"]["workflows"],
|
||||
serde_json::Value::String("write".to_string())
|
||||
assert!(
|
||||
manifest["default_permissions"].get("workflows").is_none(),
|
||||
"GitHub App must not be able to write workflow files"
|
||||
);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -596,10 +596,7 @@ async fn mint_installation_token_with_jwt(
|
|||
})
|
||||
}
|
||||
|
||||
/// Request a scoped Installation Access Token for git writes.
|
||||
///
|
||||
/// The `workflows` permission is required when a pushed commit creates or
|
||||
/// updates files under `.github/workflows/`.
|
||||
/// Request a scoped Installation Access Token with `contents: write`.
|
||||
pub async fn create_installation_access_token(
|
||||
client: &impl HttpClient,
|
||||
jwt: &str,
|
||||
|
|
@ -613,7 +610,7 @@ pub async fn create_installation_access_token(
|
|||
owner,
|
||||
repo,
|
||||
base_url,
|
||||
serde_json::json!({ "contents": "write", "workflows": "write" }),
|
||||
serde_json::json!({ "contents": "write" }),
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
|
@ -1060,8 +1057,7 @@ pub async fn update_app_webhook_config(
|
|||
///
|
||||
/// Returns `(username, password)` for authenticated cloning and pushing.
|
||||
/// Always generates a token regardless of repo visibility, since the token
|
||||
/// is needed for pushing from the sandbox. The token includes `workflows:
|
||||
/// write` so a run can publish workflow-file changes.
|
||||
/// is needed for pushing from the sandbox.
|
||||
pub async fn resolve_clone_credentials(
|
||||
ctx: &GitHubContext<'_>,
|
||||
owner: &str,
|
||||
|
|
@ -1072,14 +1068,14 @@ pub async fn resolve_clone_credentials(
|
|||
GitHubCredentials::Installation(token) => token.valid_token()?.to_string(),
|
||||
GitHubCredentials::App(_) => {
|
||||
let client = ctx.http_client()?;
|
||||
mint_git_write_token(&client, ctx, owner, repo).await?
|
||||
mint_git_contents_write_token(&client, ctx, owner, repo).await?
|
||||
}
|
||||
};
|
||||
Ok((Some("x-access-token".to_string()), Some(token)))
|
||||
}
|
||||
|
||||
/// Mint an installation token scoped for git writes, including workflow files.
|
||||
async fn mint_git_write_token(
|
||||
/// Mint an installation token scoped to repository contents writes.
|
||||
async fn mint_git_contents_write_token(
|
||||
client: &impl HttpClient,
|
||||
ctx: &GitHubContext<'_>,
|
||||
owner: &str,
|
||||
|
|
@ -1091,7 +1087,7 @@ async fn mint_git_write_token(
|
|||
owner,
|
||||
repo,
|
||||
ctx.base_url,
|
||||
serde_json::json!({ "contents": "write", "workflows": "write" }),
|
||||
serde_json::json!({ "contents": "write" }),
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
|
@ -1806,7 +1802,7 @@ mod tests {
|
|||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn create_iat_success() {
|
||||
async fn create_iat_requests_only_contents_write() {
|
||||
let mock = MockHttpClient::new()
|
||||
.on(
|
||||
HttpMethod::Get,
|
||||
|
|
@ -1822,9 +1818,7 @@ mod tests {
|
|||
r#"{"token": "ghs_xxx", "expires_at": "2099-01-01T00:00:00Z"}"#,
|
||||
)
|
||||
.with_req_header("Authorization", "Bearer test-jwt")
|
||||
.with_req_body(
|
||||
r#"{"permissions":{"contents":"write","workflows":"write"},"repositories":["repo"]}"#,
|
||||
);
|
||||
.with_req_body(r#"{"permissions":{"contents":"write"},"repositories":["repo"]}"#);
|
||||
|
||||
let token = create_installation_access_token(&mock, "test-jwt", "owner", "repo", "")
|
||||
.await
|
||||
|
|
@ -2321,7 +2315,7 @@ mod tests {
|
|||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn resolve_clone_credentials_requests_workflow_write_permission() {
|
||||
async fn clone_token_requests_only_contents_write() {
|
||||
let mock = MockHttpClient::new()
|
||||
.on(
|
||||
HttpMethod::Get,
|
||||
|
|
@ -2335,16 +2329,14 @@ mod tests {
|
|||
201,
|
||||
r#"{"token": "ghs_xxx", "expires_at": "2099-01-01T00:00:00Z"}"#,
|
||||
)
|
||||
.with_req_body(
|
||||
r#"{"permissions":{"contents":"write","workflows":"write"},"repositories":["repo"]}"#,
|
||||
);
|
||||
.with_req_body(r#"{"permissions":{"contents":"write"},"repositories":["repo"]}"#);
|
||||
let credentials = GitHubCredentials::App(GitHubAppCredentials {
|
||||
app_id: "test".to_string(),
|
||||
private_key_pem: test_rsa_key().to_string(),
|
||||
slug: None,
|
||||
});
|
||||
let context = GitHubContext::new(&credentials, "");
|
||||
let token = mint_git_write_token(&mock, &context, "owner", "repo")
|
||||
let token = mint_git_contents_write_token(&mock, &context, "owner", "repo")
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue