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
58 changes: 58 additions & 0 deletions crates/github/src/repo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ pub struct PullRequestRefs {
pub head_sha: String,
pub base_ref: String,
pub base_sha: String,
/// True when the PR head lives in a different repository than the base — a
/// cross-repo (fork) PR carrying untrusted content. Drives the memory
/// trust decision (issue #6): fork content can never write durable memory.
pub head_is_fork: bool,
}

#[derive(Debug, Deserialize)]
Expand All @@ -39,11 +43,31 @@ struct PullRequestMetaResponse {
base: PrRef,
}

impl PullRequestMetaResponse {
/// A PR is a fork PR when its head repository differs from its base. A
/// missing head repo means the fork was deleted — treated as a fork
/// (untrusted) so memory stays fail-closed.
Comment on lines +47 to +49
fn head_is_fork(&self) -> bool {
match (self.head.repo.as_ref(), self.base.repo.as_ref()) {
(Some(head), Some(base)) => head.id != base.id,
_ => true,
}
}
}

#[derive(Debug, Deserialize)]
struct PrRef {
#[serde(rename = "ref")]
ref_name: String,
sha: String,
/// The repository the ref lives in; absent when a fork has been deleted.
#[serde(default)]
repo: Option<PrRepoRef>,
}

#[derive(Debug, Deserialize)]
struct PrRepoRef {
id: u64,
}

/// Fetches repository metadata (default branch, etc.).
Expand Down Expand Up @@ -137,11 +161,13 @@ pub async fn get_pull_request_refs_with_base_url(
)
.await?;
let body: PullRequestMetaResponse = response.json().await?;
let head_is_fork = body.head_is_fork();
Ok(PullRequestRefs {
head_ref: body.head.ref_name,
head_sha: body.head.sha,
base_ref: body.base.ref_name,
base_sha: body.base.sha,
head_is_fork,
})
}

Expand Down Expand Up @@ -275,6 +301,38 @@ mod tests {
assert_eq!(body.base.ref_name, "develop");
assert_eq!(body.base.sha, "basesha");
}

#[test]
fn same_repo_pr_is_not_a_fork() {
let body: PullRequestMetaResponse = serde_json::from_value(json!({
"head": { "ref": "feature", "sha": "h", "repo": { "id": 1 } },
"base": { "ref": "main", "sha": "b", "repo": { "id": 1 } }
}))
.unwrap();
assert!(!body.head_is_fork());
}

#[test]
fn cross_repo_pr_is_a_fork() {
let body: PullRequestMetaResponse = serde_json::from_value(json!({
"head": { "ref": "feature", "sha": "h", "repo": { "id": 2 } },
"base": { "ref": "main", "sha": "b", "repo": { "id": 1 } }
}))
.unwrap();
assert!(body.head_is_fork());
}

#[test]
fn deleted_or_absent_head_repo_is_treated_as_a_fork() {
// A fork whose repo was deleted (head.repo null) is untrusted — memory
// must stay fail-closed rather than assume same-repo.
let body: PullRequestMetaResponse = serde_json::from_value(json!({
"head": { "ref": "feature", "sha": "h" },
"base": { "ref": "main", "sha": "b", "repo": { "id": 1 } }
}))
.unwrap();
assert!(body.head_is_fork());
}
}

/// Fetches the repository permission level GitHub reports for a user:
Expand Down
19 changes: 10 additions & 9 deletions crates/worker/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -630,20 +630,15 @@ async fn run_and_publish(
};
// Compute the memory governance policy (issue #6). Deny-by-default: unless
// the installation opts memory in for this repo, this is None and no policy
// is stamped, so the runtime does no memory work. Trust: a commander here
// already passed the #13 write-access gate (below-write is declined
// earlier), so a command carries maintainer trust; auto-triggered work gets
// the safe collaborator default. Fork-PR hardening is a follow-up.
// is stamped, so the runtime does no memory work. Trust is derived from the
// review target and actor: a fork PR is untrusted content and can never
// write durable memory, overriding even a maintainer trigger.
let repo_full = format!("{}/{}", task.repo_owner, task.repo_name);
let memory_policy = memory::compute_policy(memory::PolicyInputs {
enabled: config.memory.enabled_for(&repo_full),
installation_id: task.installation_id,
repo: &repo_full,
trust: if task.commander.is_some() {
memory::TrustScope::Maintainer
} else {
memory::TrustScope::Collaborator
},
trust: memory::derive_trust(targets.head_is_fork, task.commander.is_some()),
approval_required: config.memory.approval_required_for(&repo_full),
retention_days: config.memory.retention_days,
});
Expand Down Expand Up @@ -1279,6 +1274,9 @@ struct ResolvedTargets {
base_ref: String,
/// Immutable commit SHA the Check Run attaches to.
head_sha: String,
/// True when this task reviews a fork PR — untrusted content that can never
/// write durable memory (issue #6). Always false for non-PR tasks.
head_is_fork: bool,
}

/// Resolves the repository default branch and the immutable target refs a task
Expand All @@ -1303,6 +1301,7 @@ async fn resolve_targets(api_base_url: &str, token: &str, task: &Task) -> Result
default_branch: meta.default_branch,
base_ref: refs.base_ref,
head_sha: refs.head_sha,
head_is_fork: refs.head_is_fork,
})
}
TaskKind::FixIssue { .. }
Expand All @@ -1321,6 +1320,7 @@ async fn resolve_targets(api_base_url: &str, token: &str, task: &Task) -> Result
base_ref: meta.default_branch.clone(),
default_branch: meta.default_branch,
head_sha,
head_is_fork: false,
})
}
}
Expand Down Expand Up @@ -2128,6 +2128,7 @@ exit 0
default_branch: "main".to_string(),
base_ref: "main".to_string(),
head_sha: "abc123".to_string(),
head_is_fork: false,
};
let workspace = root.join("ws");

Expand Down
36 changes: 36 additions & 0 deletions crates/worker/src/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,24 @@ pub fn compute_policy(input: PolicyInputs) -> Option<MemoryPolicy> {
})
}

/// Derives the actor trust level for a memory decision (issue #6).
///
/// A fork PR is untrusted **content** — planted facts could be written into
/// durable memory to poison later reviews — so `head_is_fork` maps to
/// [`TrustScope::ForkPr`] and **overrides** the actor's own standing, even a
/// maintainer who triggered the review. Otherwise a commander (who already
/// passed the #13 write-access gate) is a maintainer, and auto-triggered work
/// gets the safe collaborator default.
pub fn derive_trust(head_is_fork: bool, has_commander: bool) -> TrustScope {
if head_is_fork {
TrustScope::ForkPr
} else if has_commander {
TrustScope::Maintainer
} else {
TrustScope::Collaborator
}
}

/// Default `(read, write)` namespace grants per trust level (contract's table).
/// The load-bearing rule: `ForkPr` and `External` get **no** write scope, so a
/// hostile fork PR can never poison durable memory.
Expand Down Expand Up @@ -272,6 +290,24 @@ mod tests {
assert!(compute_policy(inputs(TrustScope::Maintainer, false)).is_none());
}

#[test]
fn fork_review_overrides_actor_trust() {
// A fork PR is untrusted content: even a maintainer command reviewing it
// is ForkPr (never writes), while a same-repo command is Maintainer.
assert_eq!(derive_trust(true, true), TrustScope::ForkPr);
assert_eq!(derive_trust(true, false), TrustScope::ForkPr);
assert_eq!(derive_trust(false, true), TrustScope::Maintainer);
assert_eq!(derive_trust(false, false), TrustScope::Collaborator);

// And the fork grant carries through to no write scope.
let policy = compute_policy(PolicyInputs {
trust: derive_trust(true, true),
..inputs(TrustScope::Maintainer, true)
})
.unwrap();
assert!(policy.write_scopes.is_empty(), "fork review must never write");
}

#[test]
fn fork_pr_gets_read_but_never_write() {
let policy = compute_policy(inputs(TrustScope::ForkPr, true)).unwrap();
Expand Down
Loading