Skip to content

Commit e23c76d

Browse files
authored
fix(guard): cover 4 DIFC labeling gaps — get_code_quality_finding, ui_get, add_gpg_key, add_ssh_key (#7765)
Four operations from `github-mcp-server` and the GitHub CLI had no explicit DIFC rules in the guard, risking write operations bypassing classification or read operations missing secrecy/integrity labels. ## `tools.rs` — new write operations - `add_gpg_key` — pre-emptive synthetic entry for `gh gpg-key add` (`POST /user/gpg_keys`) - `add_ssh_key` — pre-emptive synthetic entry for `gh ssh-key add` (`POST /user/keys`, `/user/ssh_signing_keys`) Both inserted at sorted positions in `WRITE_OPERATIONS`. ## `tool_rules.rs` — new DIFC match arms **`get_code_quality_finding`** — repo-visibility secrecy + writer integrity, mirroring other repo-scoped read tools: ```rust "get_code_quality_finding" => { secrecy = apply_repo_visibility_secrecy(&owner, &repo, repo_id, secrecy, ctx); integrity = writer_integrity(repo_id, ctx); } ``` **`ui_get`** — method-dispatch mirroring existing standalone counterparts: ```rust "ui_get" => { let method = tool_args.get("method").and_then(|v| v.as_str()).unwrap_or(""); match method { "labels" | "milestones" | "branches" => { /* mirrors list_label / list_branches */ } "issue_types" | "issue_fields" => { /* mirrors list_issue_types / list_issue_fields */ } "assignees" | "reviewers" => { /* mirrors list_repository_collaborators */ } _ => {} } } ``` **`add_gpg_key | add_ssh_key`** — user-scoped private secrecy + writer integrity, matching the pattern used by `add_deploy_key` and other pre-emptive CLI-only operations: ```rust "add_gpg_key" | "add_ssh_key" => { secrecy = private_user_label(); baseline_scope = Cow::Borrowed(scope_names::USER); integrity = writer_integrity(scope_names::USER, ctx); } ``` Tests added for all four new rules.
2 parents da5dd40 + 9bde647 commit e23c76d

2 files changed

Lines changed: 187 additions & 0 deletions

File tree

guards/github-guard/rust-guard/src/labels/tool_rules.rs

Lines changed: 183 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -341,6 +341,14 @@ pub fn apply_tool_labels(
341341
integrity = writer_integrity(repo_id, ctx);
342342
}
343343

344+
// === Code quality findings (repo-scoped) ===
345+
// S = S(repo) — inherits from repository visibility
346+
// I = writer (requires repo write access to post/view code quality findings)
347+
"get_code_quality_finding" => {
348+
secrecy = apply_repo_visibility_secrecy(&owner, &repo, repo_id, secrecy, ctx);
349+
integrity = writer_integrity(repo_id, ctx);
350+
}
351+
344352
// === Actions: Workflow/Artifact Metadata and Artifact Downloads ===
345353
"actions_get" => {
346354
let method = tool_args.get("method").and_then(|v| v.as_str()).unwrap_or("");
@@ -355,6 +363,32 @@ pub fn apply_tool_labels(
355363
integrity = writer_integrity(repo_id, ctx);
356364
}
357365

366+
// === UI metadata dispatch (repo/org-scoped, method-dependent) ===
367+
// Mirrors existing rules for list_label, list_branches, list_issue_types,
368+
// list_issue_fields, and list_repository_collaborators.
369+
"ui_get" => {
370+
let method = tool_args.get("method").and_then(|v| v.as_str()).unwrap_or("");
371+
match method {
372+
// Repo-scoped metadata: labels, milestones, branches
373+
// S = S(repo); I = writer
374+
"labels" | "milestones" | "branches" => {
375+
secrecy = apply_repo_visibility_secrecy(&owner, &repo, repo_id, secrecy, ctx);
376+
integrity = writer_integrity(repo_id, ctx);
377+
}
378+
"issue_types" | "issue_fields" => {
379+
baseline_scope = Cow::Borrowed(scope_names::GITHUB);
380+
integrity = project_github_label(ctx);
381+
}
382+
// Access-sensitive membership/reviewer data
383+
// S = private policy scope; I = reader
384+
"assignees" | "reviewers" => {
385+
secrecy = policy_private_scope_label(&owner, &repo, repo_id, ctx);
386+
integrity = reader_integrity(repo_id, ctx);
387+
}
388+
_ => {}
389+
}
390+
}
391+
358392
// === Repo-scoped resources: visibility-inherited secrecy, approved integrity ===
359393
// S = inherits from repo visibility; I = approved (writer-level)
360394
"actions_list"
@@ -649,6 +683,19 @@ pub fn apply_tool_labels(
649683
integrity = writer_integrity(repo_id, ctx);
650684
}
651685

686+
// === User SSH/GPG key management (account-scoped writes) ===
687+
// Pre-emptive synthetic guard entries for CLI-only operations:
688+
// `gh ssh-key add` → POST /user/keys and /user/ssh_signing_keys
689+
// `gh gpg-key add` → POST /user/gpg_keys
690+
// Adding auth/signing keys is a high-risk account-level write operation.
691+
// S = private:user (user-account-scoped sensitive data)
692+
// I = writer(user) (requires authenticated account write access)
693+
"add_gpg_key" | "add_ssh_key" => {
694+
secrecy = private_user_label();
695+
baseline_scope = Cow::Borrowed(scope_names::USER);
696+
integrity = writer_integrity(scope_names::USER, ctx);
697+
}
698+
652699
// === Dynamic toolset enablement (capability expansion) ===
653700
"enable_toolset" => {
654701
// Enabling a toolset expands the agent's runtime capability set.
@@ -1297,4 +1344,140 @@ mod tests {
12971344
"delete_gist: destructive operation must require writer-level user integrity",
12981345
);
12991346
}
1347+
1348+
#[test]
1349+
fn apply_tool_labels_get_code_quality_finding_inherits_repo_visibility() {
1350+
let ctx = default_ctx();
1351+
let args = serde_json::json!({"owner": "octocat", "repo": "hello-world"});
1352+
let repo_id = "octocat/hello-world";
1353+
1354+
let (_, integrity, _) = super::apply_tool_labels(
1355+
"get_code_quality_finding",
1356+
&args,
1357+
repo_id,
1358+
vec![],
1359+
vec![],
1360+
String::new(),
1361+
&ctx,
1362+
);
1363+
assert_eq!(
1364+
integrity,
1365+
writer_integrity(repo_id, &ctx),
1366+
"get_code_quality_finding: expected writer-level integrity",
1367+
);
1368+
}
1369+
1370+
#[test]
1371+
fn apply_tool_labels_ui_get_labels_milestones_branches_are_repo_scoped() {
1372+
let ctx = default_ctx();
1373+
let repo_id = "octocat/hello-world";
1374+
let expected_integrity = writer_integrity(repo_id, &ctx);
1375+
1376+
for method in &["labels", "milestones", "branches"] {
1377+
let args = serde_json::json!({
1378+
"owner": "octocat",
1379+
"repo": "hello-world",
1380+
"method": method,
1381+
});
1382+
let (_, integrity, _) = super::apply_tool_labels(
1383+
"ui_get",
1384+
&args,
1385+
repo_id,
1386+
vec![],
1387+
vec![],
1388+
String::new(),
1389+
&ctx,
1390+
);
1391+
assert_eq!(
1392+
integrity, expected_integrity,
1393+
"ui_get method={method}: expected writer-level integrity",
1394+
);
1395+
}
1396+
}
1397+
1398+
#[test]
1399+
fn apply_tool_labels_ui_get_issue_types_and_fields_are_github_approved() {
1400+
let ctx = default_ctx();
1401+
let repo_id = "octocat/hello-world";
1402+
1403+
for (method, standalone) in &[("issue_types", "list_issue_types"), ("issue_fields", "list_issue_fields")] {
1404+
let args = serde_json::json!({
1405+
"owner": "octocat",
1406+
"repo": "hello-world",
1407+
"method": method,
1408+
});
1409+
let (_, integrity, _) = super::apply_tool_labels(
1410+
"ui_get",
1411+
&args,
1412+
repo_id,
1413+
vec![],
1414+
vec![],
1415+
String::new(),
1416+
&ctx,
1417+
);
1418+
// Org-level metadata should be treated as GitHub-controlled.
1419+
let expected_integrity = project_github_label(&ctx);
1420+
assert_eq!(
1421+
integrity, expected_integrity,
1422+
"ui_get method={method}: expected same integrity as {standalone}",
1423+
);
1424+
}
1425+
}
1426+
1427+
#[test]
1428+
fn apply_tool_labels_ui_get_assignees_and_reviewers_are_access_sensitive() {
1429+
let ctx = default_ctx();
1430+
let repo_id = "octocat/hello-world";
1431+
let expected_integrity = reader_integrity(repo_id, &ctx);
1432+
1433+
for method in &["assignees", "reviewers"] {
1434+
let args = serde_json::json!({
1435+
"owner": "octocat",
1436+
"repo": "hello-world",
1437+
"method": method,
1438+
});
1439+
let (secrecy, integrity, _) = super::apply_tool_labels(
1440+
"ui_get",
1441+
&args,
1442+
repo_id,
1443+
vec![],
1444+
vec![],
1445+
String::new(),
1446+
&ctx,
1447+
);
1448+
let _ = secrecy; // secrecy is policy_private_scope_label (backend unavailable in tests)
1449+
assert_eq!(
1450+
integrity, expected_integrity,
1451+
"ui_get method={method}: expected reader-level integrity",
1452+
);
1453+
}
1454+
}
1455+
1456+
#[test]
1457+
fn apply_tool_labels_add_gpg_key_and_add_ssh_key_are_user_private_writes() {
1458+
let ctx = default_ctx();
1459+
let args = serde_json::json!({});
1460+
let expected_secrecy = private_user_label();
1461+
let expected_integrity = writer_integrity(scope_names::USER, &ctx);
1462+
1463+
for tool in &["add_gpg_key", "add_ssh_key"] {
1464+
let (secrecy, integrity, _) = super::apply_tool_labels(
1465+
tool,
1466+
&args,
1467+
"",
1468+
vec![],
1469+
vec![],
1470+
String::new(),
1471+
&ctx,
1472+
);
1473+
assert_eq!(
1474+
secrecy, expected_secrecy,
1475+
"{tool}: must be user-private (secrecy = private:user)",
1476+
);
1477+
assert_eq!(
1478+
integrity, expected_integrity,
1479+
"{tool}: must require writer-level user integrity",
1480+
);
1481+
}
1482+
}
13001483
}

guards/github-guard/rust-guard/src/tools.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,11 @@ pub const WRITE_OPERATIONS: &[&str] = &[
99
"actions_run_trigger",
1010
"add_comment_to_pending_review",
1111
"add_deploy_key",
12+
"add_gpg_key", // gh gpg-key add — adds a user GPG signing key
1213
"add_issue_comment",
1314
"add_project_item", // deprecated alias for projects_write (addProjectV2ItemById)
1415
"add_reply_to_pull_request_comment",
16+
"add_ssh_key", // gh ssh-key add — adds a user SSH auth/signing key
1517
"archive_repository", // gh repo archive — blocked: repo settings change unsupported
1618
"assign_copilot_to_issue",
1719
"cancel_workflow_run", // gh run cancel — cancels an in-progress workflow run
@@ -296,6 +298,8 @@ mod tests {
296298
"revert_pull_request",
297299
"add_deploy_key",
298300
"delete_deploy_key",
301+
"add_gpg_key",
302+
"add_ssh_key",
299303
] {
300304
assert!(
301305
is_write_operation(op),

0 commit comments

Comments
 (0)