Skip to content

Guard DIFC: add explicit label rules for notification writes and repository create/fork operations #4300

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lpcox merged 4 commits into from
Apr 22, 2026
+197 −45
Merged

Guard DIFC: add explicit label rules for notification writes and repository create/fork operations #4300

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bbc6fbd
Initial plan
Copilot Apr 21, 2026
4460784
fix(guard): enforce explicit DIFC labels for notification management …
Copilot Apr 21, 2026
1d85709
fix: replace Docker-dependent integration tests with mock HTTP backends
lpcox Apr 22, 2026
c2b28dd
fix(guard): preserve github baseline scope for notification and repo-…
Copilot Apr 22, 2026
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
66 changes: 53 additions & 13 deletions guards/github-guard/rust-guard/src/labels/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4614,22 +4614,31 @@ mod tests {
}

#[test]
fn test_apply_tool_labels_dismiss_notification_private_user() {
fn test_apply_tool_labels_notification_management_public_project_github() {
let ctx = default_ctx();
let tool_args = json!({ "threadId": "123" });

let (secrecy, integrity, _desc) = apply_tool_labels(
for tool in &[
"dismiss_notification",
&tool_args,
"",
vec![],
vec![],
String::new(),
&ctx,
);
"mark_all_notifications_read",
"manage_notification_subscription",
"manage_repository_notification_subscription",
] {
let (secrecy, integrity, _desc) =
apply_tool_labels(tool, &tool_args, "", vec![], vec![], String::new(), &ctx);

assert_eq!(secrecy, private_user_label(), "dismiss_notification must carry private:user secrecy");
assert_eq!(integrity, none_integrity("", &ctx), "dismiss_notification should have none-level integrity (references unknown content)");
assert!(
secrecy.is_empty(),
"{} should have empty (public) secrecy",
tool
);
assert_eq!(
integrity,
project_github_label(&ctx),
"{} should have project:github integrity",
tool
);
}
Comment on lines +4617 to +4641
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests assert apply_tool_labels() output directly, but the production path in label_resource re-applies ensure_integrity_baseline() using infer_scope_for_baseline(). For tools with empty repo_id, that second pass can change the final integrity labels (including potentially downgrading github-scoped integrity to unscoped none). Add coverage that exercises label_resource (or at least infer_scope_for_baseline for these tool names) so the intended end-to-end DIFC behavior is locked in.

Copilot uses AI. Check for mistakes.
}

#[test]
Expand Down Expand Up @@ -4919,7 +4928,7 @@ mod tests {
"repo": "copilot"
});

let (_secrecy, integrity, _desc) = apply_tool_labels(
let (secrecy, integrity, _desc) = apply_tool_labels(
"fork_repository",
&tool_args,
repo_id,
Expand All @@ -4929,7 +4938,38 @@ mod tests {
&ctx,
);

assert_eq!(integrity, writer_integrity(repo_id, &ctx), "fork_repository should have writer integrity");
assert!(secrecy.is_empty(), "fork_repository should have empty (public) secrecy");
assert_eq!(
integrity,
writer_integrity("github", &ctx),
"fork_repository should have writer integrity in github scope"
);
}

#[test]
fn test_apply_tool_labels_create_repository_writer_integrity() {
let ctx = default_ctx();
let repo_id = "github/copilot";
let tool_args = json!({
"name": "new-repo"
});

let (secrecy, integrity, _desc) = apply_tool_labels(
"create_repository",
&tool_args,
repo_id,
vec![],
vec![],
String::new(),
&ctx,
);

assert!(secrecy.is_empty(), "create_repository should have empty (public) secrecy");
assert_eq!(
integrity,
writer_integrity("github", &ctx),
"create_repository should have writer integrity in github scope"
);
}

#[test]
Expand Down
28 changes: 23 additions & 5 deletions guards/github-guard/rust-guard/src/labels/tool_rules.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -466,17 +466,26 @@ pub fn apply_tool_labels(
}

// === Notifications (user-scoped, private) ===
"list_notifications" | "get_notification_details"
| "dismiss_notification" | "mark_all_notifications_read"
| "manage_notification_subscription"
| "manage_repository_notification_subscription" => {
"list_notifications" | "get_notification_details" => {
// Notifications are private to the authenticated user.
// S = private:user
// I = none (notifications reference external content of unknown trust)
secrecy = private_user_label();
integrity = vec![];
}

// === Notification management (account-scoped writes) ===
"dismiss_notification"
| "mark_all_notifications_read"
| "manage_notification_subscription"
| "manage_repository_notification_subscription" => {
// These operations change notification/subscription state and return minimal metadata.
// S = public (empty); I = project:github
secrecy = vec![];
baseline_scope = "github".to_string();
integrity = project_github_label(ctx);
}
Comment on lines +477 to +487
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

apply_tool_labels sets baseline_scope = "github" and returns github-scoped writer integrity for these notification management tools, but label_resource later re-applies ensure_integrity_baseline() using infer_scope_for_baseline(). For these tools repo_id is typically empty and infer_scope_for_baseline() returns "", which causes max_integrity("", ...) to downgrade the returned github-scoped integrity tags to unscoped none. Fix by teaching infer_scope_for_baseline() to return "github" for these tool names (or by removing/avoiding the second baseline enforcement so it can’t clobber tool-specific baseline_scope).

This issue also appears on line 582 of the same file.

Copilot uses AI. Check for mistakes.

// === Private GitHub-controlled metadata (user-associated): PII/org-structure sensitive ===
"get_me"
| "get_teams"
Expand Down Expand Up @@ -562,14 +571,23 @@ pub fn apply_tool_labels(

// === Repo content and structure write operations ===
"create_or_update_file" | "push_files" | "delete_file" | "create_branch"
| "update_pull_request_branch" | "create_repository" | "fork_repository" => {
| "update_pull_request_branch" => {
// Write operations that modify repo content/structure.
// S = S(repo) — response references repo-scoped content
// I = writer (agent-authored content)
secrecy = apply_repo_visibility_secrecy(&owner, &repo, repo_id, secrecy, ctx);
integrity = writer_integrity(repo_id, ctx);
}

// === Repository creation/fork (user/org-scoped writes) ===
"create_repository" | "fork_repository" => {
// Creating/forking repositories is account-scoped and does not return repo content.
// S = public (empty); I = writer(github)
secrecy = vec![];
baseline_scope = "github".to_string();
integrity = writer_integrity("github", ctx);
}

// === Projects write operations (org-scoped) ===
"projects_write"
// Deprecated aliases that map to projects_write
Expand Down
74 changes: 74 additions & 0 deletions guards/github-guard/rust-guard/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,12 @@ fn infer_scope_for_baseline(tool_name: &str, tool_args: &Value, repo_id: &str) -
}

match tool_name {
"dismiss_notification"
| "mark_all_notifications_read"
| "manage_notification_subscription"
| "manage_repository_notification_subscription"
| "create_repository"
| "fork_repository" => "github".to_string(),
"search_code" | "search_issues" | "search_pull_requests" => {
let query = tool_args
.get("query")
Expand Down Expand Up @@ -1159,6 +1165,74 @@ mod tests {
assert_eq!(inferred, "github/gh-aw-mcpg");
}

#[test]
fn infer_scope_for_baseline_uses_github_scope_for_notification_management_tools() {
let tool_args = json!({ "threadId": "123" });
for tool in &[
"dismiss_notification",
"mark_all_notifications_read",
"manage_notification_subscription",
"manage_repository_notification_subscription",
] {
let inferred = infer_scope_for_baseline(tool, &tool_args, "");
assert_eq!(inferred, "github", "{} should infer github baseline scope", tool);
}
}

#[test]
fn infer_scope_for_baseline_uses_github_scope_for_repo_creation_tools() {
let tool_args = json!({ "name": "new-repo" });
for tool in &["create_repository", "fork_repository"] {
let inferred = infer_scope_for_baseline(tool, &tool_args, "");
assert_eq!(inferred, "github", "{} should infer github baseline scope", tool);
}
}

#[test]
fn notification_management_integrity_preserved_after_baseline() {
let ctx = PolicyContext::default();
let tool_args = json!({ "threadId": "123" });
for tool in &[
"dismiss_notification",
"mark_all_notifications_read",
"manage_notification_subscription",
"manage_repository_notification_subscription",
] {
let (_, integrity, _) =
labels::apply_tool_labels(tool, &tool_args, "", vec![], vec![], String::new(), &ctx);
let baseline_scope = infer_scope_for_baseline(tool, &tool_args, "");
let after_baseline =
labels::ensure_integrity_baseline(&baseline_scope, integrity, &ctx);

assert_eq!(
after_baseline,
labels::project_github_label(&ctx),
"{} integrity should remain github-scoped after baseline enforcement",
tool
);
}
}

#[test]
fn repo_creation_integrity_preserved_after_baseline() {
let ctx = PolicyContext::default();
let tool_args = json!({ "name": "new-repo" });
for tool in &["create_repository", "fork_repository"] {
let (_, integrity, _) =
labels::apply_tool_labels(tool, &tool_args, "", vec![], vec![], String::new(), &ctx);
let baseline_scope = infer_scope_for_baseline(tool, &tool_args, "");
let after_baseline =
labels::ensure_integrity_baseline(&baseline_scope, integrity, &ctx);

assert_eq!(
after_baseline,
labels::writer_integrity("github", &ctx),
"{} integrity should remain github writer-scoped after baseline enforcement",
tool
);
}
}

#[test]
fn transfer_repository_integrity_is_blocked_after_ensure_baseline() {
// Verify that the is_blocked_tool + blocked_integrity override logic produces
Expand Down
74 changes: 47 additions & 27 deletions test/integration/binary_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,47 +25,53 @@ func TestBinaryInvocation_RoutedMode(t *testing.T) {
t.Skip("Skipping binary integration test in short mode")
}

// Find the binary
binaryPath := findBinary(t)
t.Logf("Using binary: %s", binaryPath)

// Create a temporary config file
configFile := createTempConfig(t, map[string]interface{}{
"testserver": map[string]interface{}{
"command": "docker",
"args": []string{"run", "--rm", "-i", "alpine:latest", "echo"},
},
})
defer os.Remove(configFile)
// Use an in-process mock backend to avoid Docker dependency
mockBackend := createMinimalMockMCPBackend(t)
defer mockBackend.Close()

// Start the server process
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
defer cancel()

port := "13001" // Use a specific port for testing
port := "13001"
cmd := exec.CommandContext(ctx, binaryPath,
"--config", configFile,
"--config-stdin",
"--listen", "127.0.0.1:"+port,
"--routed",
)

// Capture output for debugging
configJSON := map[string]interface{}{
"mcpServers": map[string]interface{}{
"testserver": map[string]interface{}{
"type": "http",
"url": mockBackend.URL + "/mcp",
},
},
"gateway": map[string]interface{}{
"port": 13001,
"domain": "localhost",
"apiKey": "test-token",
},
}
configBytes, _ := json.Marshal(configJSON)

var stdout, stderr bytes.Buffer
cmd.Stdin = bytes.NewReader(configBytes)
cmd.Stdout = &stdout
cmd.Stderr = &stderr

if err := cmd.Start(); err != nil {
t.Fatalf("Failed to start server: %v", err)
}

// Ensure the process is killed at the end
defer func() {
if cmd.Process != nil {
cmd.Process.Kill()
}
}()

// Wait for server to start
serverURL := "http://127.0.0.1:" + port
if !waitForServer(t, serverURL+"/health", 10*time.Second) {
t.Logf("STDOUT: %s", stdout.String())
Expand Down Expand Up @@ -130,29 +136,43 @@ func TestBinaryInvocation_UnifiedMode(t *testing.T) {
binaryPath := findBinary(t)
t.Logf("Using binary: %s", binaryPath)

configFile := createTempConfig(t, map[string]interface{}{
"backend1": map[string]interface{}{
"command": "docker",
"args": []string{"run", "--rm", "-i", "alpine:latest", "echo"},
},
"backend2": map[string]interface{}{
"command": "docker",
"args": []string{"run", "--rm", "-i", "alpine:latest", "echo"},
},
})
defer os.Remove(configFile)
// Use in-process mock backends to avoid Docker dependency
mockBackend1 := createMinimalMockMCPBackend(t)
defer mockBackend1.Close()
mockBackend2 := createMinimalMockMCPBackend(t)
defer mockBackend2.Close()

ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
defer cancel()

port := "13002"
cmd := exec.CommandContext(ctx, binaryPath,
"--config", configFile,
"--config-stdin",
"--listen", "127.0.0.1:"+port,
"--unified",
)

configJSON := map[string]interface{}{
"mcpServers": map[string]interface{}{
"backend1": map[string]interface{}{
"type": "http",
"url": mockBackend1.URL + "/mcp",
},
"backend2": map[string]interface{}{
"type": "http",
"url": mockBackend2.URL + "/mcp",
},
},
"gateway": map[string]interface{}{
"port": 13002,
"domain": "localhost",
"apiKey": "test-token",
},
}
configBytes, _ := json.Marshal(configJSON)

var stdout, stderr bytes.Buffer
cmd.Stdin = bytes.NewReader(configBytes)
cmd.Stdout = &stdout
cmd.Stderr = &stderr

Expand Down
Loading