Repo-scope the environment GitHub App token via metadata.github_repo (#25)

Thread the run's target repo (the subject's git_repo custom field, the same
value the harness renders as {{git_repo}}) into the EnvironmentSpec metadata as
github_repo so the ephemeral-node env plugin resolves the GitHub App
installation for that repo and scopes the minted token to it, instead of
falling back to the first of possibly several org installations (a push-403
class bug).

Covers both environment paths: the runner-owned prepare
(PreparedEnvironment) and the daemon broker acquire (BrokeredEnvironment),
sourced once per run in workflow_execute and once per phase in the brokered
plugin entry. Existing metadata is preserved; an absent git_repo leaves
metadata untouched.

Bump 0.4.36 -> 0.4.37.

Author: Shooksie

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "animus-workflow-runner-default"
-version = "0.4.36"
+version = "0.4.37"
 edition = "2021"
 description = "Reference workflow_runner plugin for Animus v0.5 (stdio JSON-RPC + direct-execute CLI; replaces in-tree workflow-runner-v2)"
 license = "Elastic-2.0"

src/phase_command.rs

@@ -473,6 +473,48 @@ async fn fetch_subject_record(project_root: &str, kind: &str, native_id: &str) -
     None
 }
 
+/// Best-effort fetch of the bound subject's `git_repo` custom field — the SAME
+/// value a command phase renders as `{{git_repo}}` (case-insensitive, mirroring
+/// [`build_command_template_vars`]'s lowercased custom-field keys). Used to
+/// repo-scope the per-run environment node's GitHub App installation token (see
+/// [`crate::phase_environment`]). Reuses the same bounded `subject/get` fetch the
+/// command path uses; a fetch failure / missing field degrades to `None` (a bare
+/// node, no repo scoping).
+pub(crate) async fn subject_git_repo(project_root: &str, kind: &str, subject_id: &str) -> Option<String> {
+    // The brokered per-phase entry supplies no explicit kind and a possibly
+    // kind-QUALIFIED id (`task:TASK-1`); the owned path supplies the real kind
+    // and a BARE native id. Derive `(kind, native_id)` for the `subject/get`
+    // fetch from whichever form arrived.
+    let (kind, native_id) = resolve_kind_and_native(kind, subject_id);
+    let record = tokio::time::timeout(
+        std::time::Duration::from_secs(SUBJECT_FETCH_TIMEOUT_SECS),
+        fetch_subject_record(project_root, kind, native_id),
+    )
+    .await
+    .ok()
+    .flatten();
+    subject_custom_fields(record.as_ref())
+        .into_iter()
+        .find(|(key, value)| key.eq_ignore_ascii_case("git_repo") && !value.trim().is_empty())
+        .map(|(_, value)| value)
+}
+
+/// Derive the `(kind, native_id)` pair the `subject/get` fetch needs from a
+/// caller-supplied kind (possibly empty) plus a subject id that may be BARE
+/// (`TASK-1`) or kind-QUALIFIED (`task:TASK-1`). An explicit kind wins; when it
+/// is empty the prefix of a qualified id supplies it. A bare id with no kind
+/// yields an empty kind (the fetch then degrades to `None`).
+fn resolve_kind_and_native<'a>(kind: &'a str, subject_id: &'a str) -> (&'a str, &'a str) {
+    let kind = kind.trim();
+    let id = subject_id.trim();
+    match id.split_once(':') {
+        Some((prefix, native)) if !prefix.is_empty() && !native.is_empty() => {
+            (if kind.is_empty() { prefix } else { kind }, native)
+        }
+        _ => (kind, id),
+    }
+}
+
 /// Extract the bound subject's CUSTOM fields from a raw `subject/get` record as
 /// string values, preferring the explicit `data` object then the backend
 /// `attributes` bag (the same precedence [`build_command_context_json`] uses).

src/phase_environment.rs

@@ -305,7 +305,11 @@ impl PreparedEnvironment {
     /// Resolve the environment plugin and prepare a BARE node for the whole run.
     /// Blocking (the [`EnvironmentClient`] surface is blocking) — call via
     /// [`Self::prepare_off_runtime`] from an async context.
-    pub(crate) fn prepare(project_root: &Path, environment: &ResolvedEnvironment) -> Result<Self> {
+    pub(crate) fn prepare(
+        project_root: &Path,
+        environment: &ResolvedEnvironment,
+        github_repo: Option<&str>,
+    ) -> Result<Self> {
         let environment_id = environment.id.as_str();
         let client = EnvironmentClient::resolve(project_root, environment_id).map_err(|err| {
             anyhow!(
@@ -314,7 +318,7 @@ impl PreparedEnvironment {
             )
         })?;
         let backend_label = format!("environment:{}", client.plugin_name());
-        Self::prepare_with_backend(Arc::new(client), backend_label, environment)
+        Self::prepare_with_backend(Arc::new(client), backend_label, environment, github_repo)
     }
 
     /// Prepare against an already-resolved backend (production wraps an
@@ -325,6 +329,7 @@ impl PreparedEnvironment {
         backend: Arc<dyn EnvironmentExecBackend>,
         backend_label: String,
         environment: &ResolvedEnvironment,
+        github_repo: Option<&str>,
     ) -> Result<Self> {
         let mut spec = EnvironmentSpec {
             kind: environment.id.clone(),
@@ -337,6 +342,9 @@ impl PreparedEnvironment {
         if let Some(overrides) = environment.spec_overrides.clone() {
             apply_spec_overrides(&mut spec, overrides);
         }
+        // Repo-scope the node's GitHub App token: merged AFTER the routing-rule
+        // overrides so it lands on (and preserves) any metadata they set.
+        merge_github_repo_metadata(&mut spec, github_repo);
         let handle =
             backend.prepare(spec).map_err(|err| anyhow!("environment prepare failed for {backend_label}: {err:#}"))?;
         Ok(Self {
@@ -360,7 +368,11 @@ impl PreparedEnvironment {
     /// runtime cannot be created from within another runtime's worker), then the
     /// idle runtime — whose own worker threads keep the driver parked and live —
     /// is moved back to the async caller and stored on the struct.
-    pub(crate) async fn prepare_off_runtime(project_root: &Path, environment: &ResolvedEnvironment) -> Result<Self> {
+    pub(crate) async fn prepare_off_runtime(
+        project_root: &Path,
+        environment: &ResolvedEnvironment,
+        github_repo: Option<String>,
+    ) -> Result<Self> {
         let project_root = project_root.to_path_buf();
         let environment = environment.clone();
         let (tx, rx) = tokio::sync::oneshot::channel();
@@ -373,7 +385,8 @@ impl PreparedEnvironment {
                     .context("building dedicated runtime for the environment host")?;
                 // Lease + prepare INSIDE this runtime so the host's I/O driver
                 // binds here (not to a per-call throwaway that would die on return).
-                let mut prepared = runtime.block_on(async { Self::prepare(&project_root, &environment) })?;
+                let mut prepared =
+                    runtime.block_on(async { Self::prepare(&project_root, &environment, github_repo.as_deref()) })?;
                 prepared.runtime = Some(runtime);
                 Ok::<_, anyhow::Error>(prepared)
             })();
@@ -487,6 +500,27 @@ impl HeldEnvironment for PreparedEnvironment {
     }
 }
 
+/// Merge the run's target repo into the [`EnvironmentSpec`]'s `metadata` as
+/// `github_repo`, so the environment plugin resolves the GitHub App INSTALLATION
+/// for THAT repo (`GET /repos/{owner}/{repo}/installation`) and scopes the minted
+/// installation token to it — instead of falling back to the first of possibly
+/// several org installations (which mints a token for the WRONG org and 403s on
+/// push). The repo is the subject's `git_repo` custom field (the same value the
+/// harness renders as `{{git_repo}}`). Any existing `metadata` object is
+/// PRESERVED (`github_repo` is merged in, not clobbered); a `None`/empty repo
+/// leaves `metadata` untouched (a bare non-coding run must not invent a repo).
+fn merge_github_repo_metadata(spec: &mut EnvironmentSpec, github_repo: Option<&str>) {
+    let Some(repo) = github_repo.map(str::trim).filter(|repo| !repo.is_empty()) else {
+        return;
+    };
+    if !matches!(spec.metadata, Value::Object(_)) {
+        spec.metadata = Value::Object(serde_json::Map::new());
+    }
+    if let Value::Object(map) = &mut spec.metadata {
+        map.insert("github_repo".to_string(), Value::String(repo.to_string()));
+    }
+}
+
 /// Merge a routing rule's opaque `spec` overrides into the compiled
 /// [`EnvironmentSpec`]: the wire-typed keys (`image`, `resources`, `env`) land
 /// on their typed fields; everything else is carried opaquely on `metadata`. The
@@ -1074,24 +1108,27 @@ impl BrokeredEnvironment {
     ///
     /// The blocking socket dial runs on a blocking thread so it never stalls the
     /// async runtime's worker.
-    pub(crate) async fn acquire_from_env() -> Option<Result<Self>> {
+    pub(crate) async fn acquire_from_env(github_repo: Option<String>) -> Option<Result<Self>> {
         let env = broker_env()?;
-        let joined = tokio::task::spawn_blocking(move || Self::acquire(env)).await;
+        let joined = tokio::task::spawn_blocking(move || Self::acquire(env, github_repo.as_deref())).await;
         Some(joined.map_err(|err| anyhow!("environment broker acquire task panicked: {err}")).and_then(|res| res))
     }
 
     /// Dial the broker socket and perform the `acquire` handshake.
-    fn acquire(env: BrokerEnv) -> Result<Self> {
+    fn acquire(env: BrokerEnv, github_repo: Option<&str>) -> Result<Self> {
         // Bare spec (no repos), carrying `metadata.animus_run_id` so the plugin
-        // names the node deterministically per run.
-        let spec = EnvironmentSpec {
+        // names the node deterministically per run — plus `github_repo` (when the
+        // subject carries one) so the plugin repo-scopes the minted GitHub App
+        // installation token to the run's target repo.
+        let mut spec = EnvironmentSpec {
             kind: env.environment_id.clone(),
             repos: Vec::new(),
             image: None,
             resources: None,
             env: BTreeMap::new(),
             metadata: serde_json::json!({ "animus_run_id": env.run_id }),
         };
+        merge_github_repo_metadata(&mut spec, github_repo);
         let request = AcquireRequest {
             op: "acquire",
             token: &env.token,
@@ -1605,6 +1642,52 @@ environment_routing:
 
     /// The prepared node is BARE (no repos) but a routing rule's opaque `spec`
     /// overrides (image / env / metadata) still ride the prepared EnvironmentSpec.
+    #[test]
+    fn merge_github_repo_metadata_scopes_the_token_and_preserves_existing_metadata() {
+        let mut spec = EnvironmentSpec {
+            kind: "railway".to_string(),
+            repos: Vec::new(),
+            image: None,
+            resources: None,
+            env: BTreeMap::new(),
+            metadata: Value::Null,
+        };
+
+        // Null metadata -> a fresh object carrying only github_repo.
+        merge_github_repo_metadata(&mut spec, Some("launchapp-dev/animus-cli"));
+        assert_eq!(
+            spec.metadata.pointer("/github_repo").and_then(Value::as_str),
+            Some("launchapp-dev/animus-cli"),
+            "github_repo lands on metadata so the plugin repo-scopes the token"
+        );
+
+        // Existing metadata keys (e.g. the broker's animus_run_id) are PRESERVED.
+        spec.metadata = serde_json::json!({ "animus_run_id": "RUN-1", "network": "none" });
+        merge_github_repo_metadata(&mut spec, Some("  launchapp-dev/animus-cli  "));
+        assert_eq!(spec.metadata.pointer("/animus_run_id").and_then(Value::as_str), Some("RUN-1"));
+        assert_eq!(spec.metadata.pointer("/network").and_then(Value::as_str), Some("none"));
+        assert_eq!(
+            spec.metadata.pointer("/github_repo").and_then(Value::as_str),
+            Some("launchapp-dev/animus-cli"),
+            "the repo is trimmed and merged in without clobbering sibling keys"
+        );
+
+        // None / empty repo (a non-coding run) leaves metadata untouched — never
+        // invents a repo.
+        let mut bare = EnvironmentSpec {
+            kind: "railway".to_string(),
+            repos: Vec::new(),
+            image: None,
+            resources: None,
+            env: BTreeMap::new(),
+            metadata: Value::Null,
+        };
+        merge_github_repo_metadata(&mut bare, None);
+        assert!(bare.metadata.is_null(), "no repo -> metadata stays untouched");
+        merge_github_repo_metadata(&mut bare, Some("   "));
+        assert!(bare.metadata.is_null(), "blank repo -> metadata stays untouched");
+    }
+
     #[test]
     fn prepared_spec_is_bare_but_carries_rule_spec_overrides() {
         let _lock = crate::test_env::scoped_state_serializer();
@@ -1641,6 +1724,7 @@ environment_routing:
             backend.clone(),
             "environment:container-env".to_string(),
             &resolved,
+            None,
         )
         .expect("prepare succeeds");
         assert_eq!(prepared.id(), "container-env");
@@ -2076,6 +2160,7 @@ environment_routing:
             backend.clone(),
             "environment:container".to_string(),
             &bare_env("container"),
+            None,
         ) {
             Ok(_) => panic!("prepare failure must surface as an error"),
             Err(err) => err,
@@ -2094,6 +2179,7 @@ environment_routing:
             backend.clone(),
             "environment:container".to_string(),
             &bare_env("container"),
+            None,
         )
         .expect("prepare succeeds");
         assert_eq!(backend.prepare_calls.load(Ordering::SeqCst), 1, "prepared exactly once");
@@ -2292,6 +2378,9 @@ environment_routing:
             // on metadata for deterministic per-run node naming.
             assert!(v["spec"].get("repos").is_none(), "bare node carries no repos: {v}");
             assert_eq!(v["spec"]["metadata"]["animus_run_id"], "run-1");
+            // The target repo rides metadata so the plugin repo-scopes the token,
+            // alongside (not clobbering) the run id.
+            assert_eq!(v["spec"]["metadata"]["github_repo"], "launchapp-dev/animus-cli");
             writeln!(out, r#"{{"ok":true,"workspace_root":"/workspace","handle_id":"h-1"}}"#).unwrap();
             out.flush().unwrap();
         });
@@ -2301,7 +2390,7 @@ environment_routing:
             run_id: "run-1".to_string(),
             environment_id: "railway".to_string(),
         };
-        let brokered = BrokeredEnvironment::acquire(env).expect("acquire succeeds");
+        let brokered = BrokeredEnvironment::acquire(env, Some("launchapp-dev/animus-cli")).expect("acquire succeeds");
         assert_eq!(brokered.id(), "railway");
         assert_eq!(brokered.handle_id, "h-1");
         assert_eq!(brokered.workspace_root, "/workspace");
@@ -2321,7 +2410,7 @@ environment_routing:
             run_id: "run-1".to_string(),
             environment_id: "railway".to_string(),
         };
-        let err = BrokeredEnvironment::acquire(env).expect_err("ok:false must fail");
+        let err = BrokeredEnvironment::acquire(env, None).expect_err("ok:false must fail");
         assert!(format!("{err:#}").contains("no capacity"), "error surfaces the broker reason: {err:#}");
         server.join().unwrap();
     }

src/plugin.rs

@@ -454,8 +454,16 @@ pub async fn handle_workflow_run_phase(request: WorkflowPhaseRunRequest) -> Resu
     // teardown — the daemon owns the lifecycle). Absent the broker env vars this
     // stays `None` (the byte-for-byte local path). A broker acquire failure fails
     // the phase and NEVER falls back to an owned prepare.
+    // The run's target repo (the subject's `git_repo` custom field — the SAME
+    // value a command phase renders as `{{git_repo}}`) rides the acquire spec's
+    // `metadata.github_repo` so the environment plugin repo-scopes the minted
+    // GitHub App installation token to THAT repo. The broker single-flights the
+    // node per run, so this matters on the run's FIRST phase acquire; a bare run
+    // without `git_repo` leaves the metadata untouched.
+    let subject_git_repo =
+        crate::phase_command::subject_git_repo(&project_root, "", &request.subject_id).await;
     let brokered_environment: Option<std::sync::Arc<crate::phase_environment::BrokeredEnvironment>> =
-        match crate::phase_environment::BrokeredEnvironment::acquire_from_env().await {
+        match crate::phase_environment::BrokeredEnvironment::acquire_from_env(subject_git_repo).await {
             Some(result) => Some(std::sync::Arc::new(result.with_context(|| {
                 format!("failed to acquire brokered environment for phase '{}'", request.phase_id)
             })?)),

src/workflow_execute.rs

@@ -339,8 +339,16 @@ pub async fn execute_workflow_with_hub(
     // Preparation runs OFF the async runtime (the `EnvironmentClient` surface is
     // blocking); a prepare failure fails the run up front rather than executing
     // locally.
+    //
+    // The run's target repo (the subject's `git_repo` custom field — the SAME
+    // value a command phase renders as `{{git_repo}}`) rides the prepared spec's
+    // `metadata.github_repo` so the environment plugin repo-scopes the GitHub App
+    // installation token to THAT repo (correct installation, least privilege). A
+    // bare non-coding run without `git_repo` leaves the metadata untouched.
+    let subject_git_repo =
+        crate::phase_command::subject_git_repo(&params.project_root, &subject_kind_str, &subject_id_str).await;
     let brokered_environment: Option<std::sync::Arc<crate::phase_environment::BrokeredEnvironment>> =
-        match crate::phase_environment::BrokeredEnvironment::acquire_from_env().await {
+        match crate::phase_environment::BrokeredEnvironment::acquire_from_env(subject_git_repo.clone()).await {
             Some(result) => Some(std::sync::Arc::new(
                 result
                     .with_context(|| format!("failed to acquire brokered environment for workflow {}", workflow.id))?,
@@ -360,6 +368,7 @@ pub async fn execute_workflow_with_hub(
                     let prepared = crate::phase_environment::PreparedEnvironment::prepare_off_runtime(
                         Path::new(&params.project_root),
                         &environment,
+                        subject_git_repo.clone(),
                     )
                     .await
                     .with_context(|| {