feat(worker,github): fork-PR trust hardening for memory writes (#6) (#52)

Detect cross-repo (fork) PRs from the PR metadata (head repo id != base, or a
deleted head repo) and thread head_is_fork through ResolvedTargets. The memory
trust derivation now maps a fork PR to ForkPr — which grants no write scope —
overriding even a maintainer trigger, since the risk is the untrusted content,
not the actor. Closes the follow-up flagged in #50: auto and command reviews of
fork content can no longer write durable memory even with approval.

Signed-off-by: Val Alexander <bunsthedev@gmail.com>

Author: BunsDev

crates/github/src/repo.rs

@@ -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)]
@@ -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.
+    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.).
@@ -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,
     })
 }
 
@@ -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:

crates/worker/src/lib.rs

@@ -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,
     });
@@ -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
@@ -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 { .. }
@@ -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,
             })
         }
     }
@@ -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");
 

crates/worker/src/memory.rs

@@ -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.
@@ -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();