Skip to content
Merged
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
25 changes: 15 additions & 10 deletions guards/github-guard/docs/SECRECY_TAG_SPEC.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,11 @@ For public repository objects:
This ensures owner-level and repo-level confidentiality are both represented.

> **Sensitive resources in public repos**: Some resources are assigned `private:owner/repo` secrecy
> even when the repository is **public**: job logs, secret scanning alerts, sensitive credential
> files, workflow definition files, and artifact downloads. These resources may contain actual
> even when the repository is **public**: secret scanning alerts, sensitive credential
> files, and workflow definition files. These resources may contain actual
> credential values or sensitive operational data regardless of repository visibility.
> Job logs (`get_job_logs`) and artifact downloads (`actions_get` with `download_workflow_run_artifact`)
> are visibility-aware and return `[]` for public repositories to support integrity audit workflows.
> Using `private:owner/repo` (rather than a special `secret` tag) keeps the secrecy model
> consistent and scoped — an agent needs clearance for the *specific repository* to access
> those resources, not a global catch-all clearance.
Expand Down Expand Up @@ -80,10 +82,9 @@ Resource labels are coarse pre-check labels by tool call.
| Tool / Resource Type | Private Repo | Public Repo |
|---|---|---|
| Repo-scoped read tools (`get_issue`, `list_issues`, `get_pull_request`, `list_pull_requests`, `get_commit`, `list_commits`, `get_file_contents`, `list_branches`, `list_tags`, `get_tag`, `list_releases`, `get_latest_release`, `get_release_by_tag`, `get_label`, `list_label`, `actions_get`, `actions_list`, `search_code`, `get_repository`, `get_repository_tree`, `list_discussions`, `get_discussion`, `get_discussion_comments`, `list_discussion_categories`) | `private:<owner>`, `private:<owner>/<repo>` | `[]` |
| Job logs (`get_job_logs`) | `private:<owner>/<repo>` | `private:<owner>/<repo>` |
| Sensitive file content (`get_file_contents` with sensitive paths) | `private:<owner>/<repo>` | `private:<owner>/<repo>` |
| Job logs (`get_job_logs`) | `private:<owner>/<repo>` | `[]` |
| Secret scanning alerts (`list_secret_scanning_alerts`, `get_secret_scanning_alert`) | `private:<owner>/<repo>` | `private:<owner>/<repo>` |
| Artifact downloads (`actions_get` with method `download_workflow_run_artifact`) | `private:<owner>/<repo>` | `private:<owner>/<repo>` |
| Artifact downloads (`actions_get` with method `download_workflow_run_artifact`) | `private:<owner>/<repo>` | `[]` |
| Code scanning & Dependabot alerts (`list_code_scanning_alerts`, `get_code_scanning_alert`, `list_dependabot_alerts`, `get_dependabot_alert`) | `private:<owner>`, `private:<owner>/<repo>` | `private:<owner>`, `private:<owner>/<repo>` |
| Repo/org security advisories (`list_repository_security_advisories`, `list_org_repository_security_advisories`) | `private:<owner>`, `private:<owner>/<repo>` | `private:<owner>`, `private:<owner>/<repo>` |
| User-scoped tools (`get_me`, `get_teams`, `get_team_members`, `list_starred_repositories`) | `private:user` | `private:user` |
Expand All @@ -99,7 +100,10 @@ Notes:
- Response labeling is authoritative when item-level visibility is available.
- Resources marked with the same `private:<owner>/<repo>` for both private *and* public repos are
those that may embed actual credential values or sensitive operational data regardless of the
repository's public/private setting (job logs, secret scanning alerts, workflow files, artifacts).
repository's public/private setting (secret scanning alerts, workflow files).
- Job logs (`get_job_logs`) and artifact downloads (`actions_get` with `download_workflow_run_artifact`)
are visibility-aware: private repositories are labeled `private:<owner>/<repo>`, while public
repositories return `[]` to support integrity audit access to public Actions evidence.

---

Expand All @@ -116,7 +120,7 @@ Response labels are fine-grained per item and should be treated as authoritative
| File content item (`get_file_contents`) | `private:<owner>`, `private:<owner>/<repo>` | `[]` |
| Branch/tag/release metadata item | `private:<owner>`, `private:<owner>/<repo>` | `[]` |
| GitHub Actions workflow/artifact metadata | `private:<owner>`, `private:<owner>/<repo>` | `[]` |
| Job logs (`get_job_logs`) | `private:<owner>/<repo>` | `private:<owner>/<repo>` |
| Job logs (`get_job_logs`) | `private:<owner>/<repo>` | `[]` |
| Security alert item | `private:<owner>`, `private:<owner>/<repo>` (or stricter tool-specific secrecy where configured) | `[]` (or stricter tool-specific secrecy where configured) |
| Global security advisory | `[]` (public CVE data) | `[]` (public CVE data) |
| Repo/org security advisory | `private:<owner>`, `private:<owner>/<repo>` | `private:<owner>`, `private:<owner>/<repo>` |
Expand Down Expand Up @@ -154,9 +158,10 @@ The secrecy model uses only these tags:
For private repository objects, emit both owner and repo secrecy tags together.

The previously-used `secret` tag has been fully retired. Resources that were previously tagged
`secret` (job logs, secret scanning alerts, workflow files, sensitive credential files, artifact
downloads) now use `private:<owner>/<repo>`, with the guarantee that the scope is always at least
the containing repository regardless of its visibility setting.
`secret` (secret scanning alerts, workflow files, sensitive credential files) now use
`private:<owner>/<repo>`, with the guarantee that the scope is always at least the containing
repository regardless of its visibility setting. Job logs and artifact downloads use
`private:<owner>/<repo>` for private repositories and `[]` for public repositories.

**Rationale**: The bare `secret` tag was an unscoped global catch-all that made it impossible for
an agent to receive targeted clearance (e.g. via an `allow-only` policy scoped to a specific repo)
Expand Down
15 changes: 7 additions & 8 deletions guards/github-guard/rust-guard/src/labels/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2414,12 +2414,12 @@ mod tests {
&ctx,
);

// Job logs always get private:owner/repo scope — CI logs may contain accidentally-printed secrets
// even in public repos, so visibility-inherited secrecy is not safe here.
// Job logs inherit repository visibility: public repos are untainted, private repos
// remain private-scoped.
assert_eq!(
secrecy,
vec!["private:github/copilot".to_string()],
"get_job_logs must always have private scope (CI logs may contain secrets)"
Vec::<String>::new(),
"get_job_logs for a public repo should have empty secrecy"
);
assert_eq!(
integrity,
Expand Down Expand Up @@ -2515,12 +2515,11 @@ mod tests {
&ctx,
);

// Artifact downloads always get private:owner/repo scope — artifacts may contain
// sensitive build outputs or accidentally-included secrets.
// Artifact downloads inherit repository visibility.
assert_eq!(
secrecy,
vec!["private:github/copilot".to_string()],
"artifact downloads must always have private scope"
Vec::<String>::new(),
"artifact downloads for a public repo should have empty secrecy"
);
assert_eq!(integrity, writer_integrity("github/copilot", &ctx));
}
Expand Down
106 changes: 86 additions & 20 deletions guards/github-guard/rust-guard/src/labels/tool_rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -398,19 +398,25 @@ pub fn apply_tool_labels(

// === Security-sensitive data: always private regardless of repo visibility ===
// Covers: secret scanning alerts (may contain actual secret values), code scanning
// and Dependabot alerts (security findings), and Actions job logs (may contain
// accidentally-printed CI tokens). All are private:repo + writer integrity.
// and Dependabot alerts (security findings). All are private:repo + writer integrity.
"list_secret_scanning_alerts"
| "get_secret_scanning_alert"
| "list_code_scanning_alerts"
| "get_code_scanning_alert"
| "list_dependabot_alerts"
| "get_dependabot_alert"
| "get_job_logs" => {
| "get_dependabot_alert" => {
secrecy = policy_private_scope_label(&owner, &repo, repo_id, ctx);
integrity = writer_integrity(repo_id, ctx);
}

// === Actions log and artifact reads (repo-scoped) ===
// S = S(repo) — inherits from repository visibility
// I = writer
Comment on lines +412 to +414
"get_job_logs" => {
secrecy = apply_repo_visibility_secrecy(&owner, &repo, repo_id, secrecy, ctx);
integrity = writer_integrity(repo_id, ctx);
}

// === Code quality findings (repo-scoped) ===
// S = S(repo) — inherits from repository visibility
// I = writer (requires repo write access to post/view code quality findings)
Expand All @@ -421,15 +427,7 @@ pub fn apply_tool_labels(

// === Actions: Workflow/Artifact Metadata and Artifact Downloads ===
"actions_get" => {
let method = tool_args.get("method").and_then(|v| v.as_str()).unwrap_or("");
if method == "download_workflow_run_artifact" {
// Artifact downloads may contain sensitive data or accidentally-included secrets.
// Always treat as private regardless of repository visibility.
// S(artifact) = private:owner/repo; I(artifact) = approved
secrecy = policy_private_scope_label(&owner, &repo, repo_id, ctx);
} else {
secrecy = apply_repo_visibility_secrecy(&owner, &repo, repo_id, secrecy, ctx);
}
secrecy = apply_repo_visibility_secrecy(&owner, &repo, repo_id, secrecy, ctx);
integrity = writer_integrity(repo_id, ctx);
}

Expand Down Expand Up @@ -1603,10 +1601,39 @@ mod tests {
}

#[test]
fn apply_tool_labels_get_job_logs_is_always_private() {
fn apply_tool_labels_get_job_logs_inherits_repo_visibility() {
let ctx = default_ctx();
let args = serde_json::json!({"owner": "octocat", "repo": "hello-world"});
let repo_id = "octocat/hello-world";
Comment thread
Copilot marked this conversation as resolved.
Resolved
let _guard = crate::labels::backend::cache_repo_visibility_for_tests(repo_id, false);

let (secrecy, integrity, _) = super::apply_tool_labels(
"get_job_logs",
&args,
repo_id,
vec![],
vec![],
String::new(),
&ctx,
);
assert_eq!(
secrecy,
Vec::<String>::new(),
"get_job_logs: expected public repo secrecy to be empty",
);
assert_eq!(
integrity,
writer_integrity(repo_id, &ctx),
"get_job_logs: expected writer-level integrity",
);
}

#[test]
fn apply_tool_labels_get_job_logs_private_repo_stays_private() {
let ctx = default_ctx();
let args = serde_json::json!({"owner": "octocat", "repo": "private-repo"});
let repo_id = "octocat/private-repo";
let _guard = crate::labels::backend::cache_repo_visibility_for_tests(repo_id, true);

let (secrecy, integrity, _) = super::apply_tool_labels(
"get_job_logs",
Expand All @@ -1619,8 +1646,8 @@ mod tests {
);
assert_eq!(
secrecy,
private_label("octocat", "hello-world", repo_id, &ctx),
"get_job_logs: expected private secrecy label (CI logs may contain tokens)",
private_label("octocat", "private-repo", repo_id, &ctx),
"get_job_logs: expected private repo secrecy label",
);
assert_eq!(
integrity,
Expand All @@ -1630,14 +1657,15 @@ mod tests {
}

#[test]
fn apply_tool_labels_actions_get_artifact_download_is_always_private() {
fn apply_tool_labels_actions_get_artifact_download_inherits_repo_visibility() {
let ctx = default_ctx();
let args = serde_json::json!({
"owner": "octocat",
"repo": "hello-world",
"method": "download_workflow_run_artifact",
});
let repo_id = "octocat/hello-world";
let _guard = crate::labels::backend::cache_repo_visibility_for_tests(repo_id, false);

let (secrecy, integrity, _) = super::apply_tool_labels(
"actions_get",
Expand All @@ -1650,8 +1678,8 @@ mod tests {
);
assert_eq!(
secrecy,
private_label("octocat", "hello-world", repo_id, &ctx),
"actions_get download_workflow_run_artifact must always be private",
Vec::<String>::new(),
"actions_get download_workflow_run_artifact should inherit public repo visibility",
Comment on lines +1681 to +1682
);
assert_eq!(
integrity,
Expand All @@ -1660,6 +1688,38 @@ mod tests {
);
}

#[test]
fn apply_tool_labels_actions_get_artifact_download_private_repo_stays_private() {
let ctx = default_ctx();
let args = serde_json::json!({
"owner": "octocat",
"repo": "private-repo",
"method": "download_workflow_run_artifact",
});
let repo_id = "octocat/private-repo";
let _guard = crate::labels::backend::cache_repo_visibility_for_tests(repo_id, true);

let (secrecy, integrity, _) = super::apply_tool_labels(
"actions_get",
&args,
repo_id,
vec![],
vec![],
String::new(),
&ctx,
);
assert_eq!(
secrecy,
private_label("octocat", "private-repo", repo_id, &ctx),
"actions_get download_workflow_run_artifact: expected private repo secrecy label",
);
assert_eq!(
integrity,
writer_integrity(repo_id, &ctx),
"actions_get download_workflow_run_artifact: expected writer-level integrity",
);
}

#[test]
fn apply_tool_labels_actions_get_non_artifact_inherits_repo_visibility() {
let ctx = default_ctx();
Expand All @@ -1669,8 +1729,9 @@ mod tests {
"method": "list_workflow_runs",
});
let repo_id = "octocat/hello-world";
let _guard = crate::labels::backend::cache_repo_visibility_for_tests(repo_id, false);

let (_, integrity, _) = super::apply_tool_labels(
let (secrecy, integrity, _) = super::apply_tool_labels(
"actions_get",
&args,
repo_id,
Expand All @@ -1679,6 +1740,11 @@ mod tests {
String::new(),
&ctx,
);
assert_eq!(
secrecy,
Vec::<String>::new(),
"actions_get non-artifact method: expected public repo secrecy to be empty",
);
assert_eq!(
integrity,
writer_integrity(repo_id, &ctx),
Expand Down
3 changes: 2 additions & 1 deletion internal/mcp/connection_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1081,7 +1081,8 @@ func TestNewConnection_ErrorPaths(t *testing.T) {
assertError: func(t *testing.T, err error) {
assert.ErrorContains(t, err, "failed to connect")
assert.ErrorContains(t, err, "initialize")
assert.ErrorContains(t, err, "EOF")
errText := err.Error()
assert.True(t, strings.Contains(errText, "EOF") || strings.Contains(errText, "broken pipe"), "error should reflect process exit during handshake: %v", err)
assert.False(t, errors.Is(err, context.DeadlineExceeded), "error should reflect process exit rather than deadline expiry")
},
assertLogs: func(t *testing.T, logDir string) {
Expand Down
Loading