Skip to content
Merged
Show file tree
Hide file tree
Changes from 11 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
9 changes: 9 additions & 0 deletions docs/spec/runtime/repos.md
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,15 @@ commits, namespaced branches, operator approval — is a separate follow-up, and
the confinement above is arranged so that adding it is an explicit new capability
rather than a hole that was already open.

Its groundwork (issue #734) has landed without opening that path: a `repo.write`
grant now exists — distinct from and tighter than `repo`, so a bare `repo` (the
read tier every company sets) never confers it and the catch-all `*` never
confers it — and each binding records whether its bound credential can push, read
from the forge's `permissions.push` at bind time and healed on the next fetch
when it is still unknown. Granting `repo.write` over a read-only credential is
fail-closed: it warns and wires nothing. No tool consumes either yet; they are
what the push follow-up will gate on.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

Also absent: a distinct uid or read-only bind mount for the agent shell (the real
filesystem boundary, named under ["The honest limit"](#the-honest-limit)), and
forges other than GitHub.
3 changes: 2 additions & 1 deletion src/company/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,8 @@ pub use types::{
Inference, KNOWN_CHANNELS, MAX_DELEGATION_DEPTH_BOUNDS, McpServer, ORCHESTRATOR_TIER,
PLAN_NAMES, PLAN_PERIODS, POLICY_MODES, PROVISIONED_POLICY_MODE, Place, Plan, Policy, Schedule,
Skill, TIERS, TOOL_PROVIDERS, Tools, grants_composio_explicit, grants_media_explicit,
grants_repo_explicit, grants_search_explicit, grants_workspace_write_explicit, orchestrator_id,
grants_repo_explicit, grants_repo_write_explicit, grants_search_explicit,
grants_workspace_write_explicit, orchestrator_id,
};
pub use workflow_file::{
WORKFLOW_DESTINATION_KINDS, WORKFLOW_NODE_KINDS, WorkflowDestinationDef, WorkflowEdgeDef,
Expand Down
58 changes: 58 additions & 0 deletions src/company/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,33 @@ pub fn grants_repo_explicit(grants: &[String]) -> bool {
.any(|grant| grant == "repo" || grant.starts_with("repo."))
}

/// Whether a tool-grant list **explicitly** grants the repository *write* tier
/// (issue #734) — the tier under which an agent's work can be pushed to a real
/// remote and opened as a pull request.
///
/// This is the tightest predicate on this surface, and deliberately tighter than
/// **both** of its neighbours. Do not "harmonise" it back toward either shape:
///
/// * Unlike [`grants_repo_explicit`], a **bare `repo` grant confers nothing
/// here.** Every company adopting the read tier writes bare `repo`; if that
/// silently carried push, a company that asked for agents *reading* code would
/// get agents *pushing* it — exactly the outcome issue #247's write tier exists
/// to prevent. Read and write are separate decisions, so they are separate
/// grants. Widening this to the `repo` / `repo.*` shape reintroduces that
/// footgun.
/// * Unlike [`grants_workspace_write_explicit`], not even a *bare namespace*
/// token confers it: only the **exact** string `repo.write` does. `repo`,
/// `repo.read`, any other `repo.*` sub-grant, and the catch-all `*` all confer
/// nothing. Matching a `repo.write` *prefix* (`starts_with`) would let a stray
/// `repo.writer` slip through; the exact-string match is the point.
///
/// Lives here (always compiled) so the feature-gated harness wiring
/// (`build::build_agent`) and always-compiled tooling share one source of truth,
/// as with the read predicate above.
pub fn grants_repo_write_explicit(grants: &[String]) -> bool {
grants.iter().any(|grant| grant == "repo.write")
}

/// Whether a tool-grant list **explicitly** grants writes to the company
/// workspace (issue #237).
///
Expand Down Expand Up @@ -973,6 +1000,37 @@ mod test {
assert!(!grants_repo_explicit(&["repository".into()]));
}

/// The repository *write* tier (issue #734) is conferred ONLY by the exact
/// string `repo.write` — never a bare `repo`, never a `repo.write` prefix,
/// never the catch-all `*`. Read and write are separate decisions.
#[test]
fn repo_write_is_conferred_only_by_the_exact_grant() {
assert!(grants_repo_write_explicit(&["repo.write".into()]));
assert!(grants_repo_write_explicit(&[
"web.*".into(),
"repo.write".into()
]));
// The catch-all `*` must NOT grant write.
assert!(!grants_repo_write_explicit(&["*".into()]));
// A read grant is genuinely read-only.
assert!(!grants_repo_write_explicit(&["repo.read".into()]));
assert!(!grants_repo_write_explicit(&["repo.checkout".into()]));
assert!(!grants_repo_write_explicit(&[]));
// A prefix match must not count: `repo.writer` is not `repo.write`.
assert!(!grants_repo_write_explicit(&["repo.writer".into()]));
}

/// **The regression this predicate exists to prevent.** Every company
/// adopting the read tier writes bare `repo`; that must confer read tools
/// (`grants_repo_explicit`) and **not** push (`grants_repo_write_explicit`),
/// so a company reading code never silently gains agents pushing it. Named so
/// its purpose survives a refactor that "harmonises" the two predicates.
#[test]
fn bare_repo_confers_read_but_not_write() {
assert!(grants_repo_explicit(&["repo".into()]));
assert!(!grants_repo_write_explicit(&["repo".into()]));
}

/// `repo` is a budgetable namespace, so a `[plan].token_budgets` key of
/// that name is accepted rather than rejected as unknown.
#[test]
Expand Down
11 changes: 11 additions & 0 deletions src/harness/brain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,9 @@ impl HarnessBrain {
// over an empty ledger is a no-op, which is what every turn that touches
// no repository does.
let _checkout_janitor = CheckoutJanitor::claim(&self.deps.checkouts);
// Issue #735: a re-dispatched grant is not a task card, so clear any task
// a prior turn stamped — `repo_publish` requires a task and refuses here.
self.deps.checkouts.set_task(None);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
// Un-streamed, like a dispatched card: this turn is answered by the
// bubble returned below, and its transient frames would otherwise
// misattribute onto whichever chat thread the console is watching.
Expand Down Expand Up @@ -633,6 +636,10 @@ impl HarnessBrain {
// guard's `Drop` is what deletes the tree on every exit — success,
// error, cancel, redirect exhaustion and panic-unwind alike.
let _checkout_janitor = CheckoutJanitor::claim(&self.deps.checkouts);
// Issue #735: this is a dispatched card, so `repo_publish` names its
// branch `oc/<company>/<card>`. Stamped on the same per-turn cell the
// janitor above claims.
self.deps.checkouts.set_task(Some(card.id.clone()));
// Issue #339, same argument for staged workflow references: an operator
// chat turn earlier in this cycle may have run a workflow through the
// orchestrator's tool, and that run belongs to the conversation, not to
Expand Down Expand Up @@ -2456,6 +2463,10 @@ impl HarnessBrain {
// so it can clone a repository, and the guard's `Drop`
// removes it when this turn ends.
let _checkout_janitor = CheckoutJanitor::claim(&self.deps.checkouts);
// Issue #735: a conversation is not a task card, so clear any
// task a prior turn stamped — `repo_publish` requires a task
// and refuses on a chat turn (task turns only, this tier).
self.deps.checkouts.set_task(None);
// Drive the brain-agnostic delegation seam (issue #176): the
// orchestrator turn, its queued delegations, and the CEO-relay
// hand-back all run behind the `RunTurn` impl. `HarnessDeps` is
Expand Down
128 changes: 128 additions & 0 deletions src/harness/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,8 @@ pub fn build_agent(
bindings: deps.repo_bindings.clone().into(),
workspace: workspace.clone(),
ledger: deps.checkouts.clone(),
agent: manifest_agent.id.clone(),
approvals: deps.approval_requests.clone(),
},
));
}
Expand All @@ -506,6 +508,61 @@ pub fn build_agent(
}
}

// Repository WRITE tier (issues #734, #735). A distinct, tighter grant than
// the read `repo` above: `grants_repo_write_explicit` matches ONLY the exact
// `repo.write`, so a bare `repo` (which every read-tier company writes) and
// the catch-all `*` confer nothing here — a company that asked for agents
// reading code does not silently get agents pushing it.
//
// FOUR gates, all fail-closed, and the fourth is the one #734 added: an
// explicit `repo.write` grant, a wired manager, at least one binding, AND a
// bound credential that can actually push (`can_push == Some(true)`; `None` —
// unprobed or pre-field — reads as cannot-push). Missing any one wires
// `repo_publish` NOT AT ALL and says which, rather than offering a publish
// that would fail at push time on a read-only credential.
//
// Like the read tier, NOT feature-gated: the mirror and git runner are always
// compiled, and `repo_publish`'s push waits on an operator approval the
// runtime performs, so there is no forge client to gate the tool behind.
if crate::company::grants_repo_write_explicit(grants) {
let push_capable = deps
.repo_bindings
.iter()
.any(|binding| binding.can_push == Some(true));
match (&deps.repos, deps.repo_bindings.is_empty(), push_capable) {
(Some(repos), false, true) => {
tools.push(crate::harness::repo::repo_publish_tool(
crate::harness::repo::RepoToolContext {
repos: repos.clone(),
bindings: deps.repo_bindings.clone().into(),
workspace: workspace.clone(),
ledger: deps.checkouts.clone(),
agent: manifest_agent.id.clone(),
approvals: deps.approval_requests.clone(),
},
));
}
(None, _, _) => tracing::warn!(
company = %company,
agent = %manifest_agent.id,
"[build] agent explicitly grants `repo.write` but no repository cache is configured \
on this host; repo_publish NOT wired (fail-closed)"
),
(Some(_), true, _) => tracing::warn!(
company = %company,
agent = %manifest_agent.id,
"[build] agent explicitly grants `repo.write` but this company has bound no \
repositories; repo_publish NOT wired (fail-closed)"
),
(Some(_), false, false) => tracing::warn!(
company = %company,
agent = %manifest_agent.id,
"[build] agent explicitly grants `repo.write` but no bound repository has a \
push-capable credential; repo_publish NOT wired (fail-closed)"
),
}
}

// Company workspace (issues #237, #551) — live read (and optionally
// create/write) tools over the shared note tree, so an agent can ground an
// answer in the company's own `Standards/` / `Playbooks/` instead of
Expand Down Expand Up @@ -1588,6 +1645,17 @@ mod tests {
/// `deps.repo_bindings` — so the difference between the two is exactly
/// "the operator bound something", which is two of the four gate states.
fn built_tool_names_with_repos(grants: &[&str], bindings: usize) -> Vec<String> {
built_tool_names_with_repos_cap(grants, bindings, false)
}

/// [`built_tool_names_with_repos`], with control over whether the bound
/// credentials read as push-capable (issue #735) — the fourth gate the write
/// tier adds. `false` matches every read-tier caller (`can_push: None`).
fn built_tool_names_with_repos_cap(
grants: &[&str],
bindings: usize,
push_capable: bool,
) -> Vec<String> {
use crate::runtime::repo_manager::types::RepoBinding;
let dir = tempfile::tempdir().expect("tempdir");
let mut deps = pin_deps(dir.path().to_path_buf());
Expand All @@ -1607,6 +1675,7 @@ mod tests {
last_fetched_millis: None,
size_bytes: 0,
bound_at_millis: 1,
can_push: if push_capable { Some(true) } else { None },
})
.collect();
let manifest_agent = ManifestAgent {
Expand Down Expand Up @@ -1717,6 +1786,65 @@ mod tests {
assert_eq!(granted, baseline, "the `repo` grant widened the belt");
}

/// The repository *write* tier (issue #734) wires NO tool of its own yet —
/// `repo_publish` lands in #735. Granting `repo.write` confers the read pair
/// (write implies read, since `repo.write` matches the read predicate's
/// `repo.` prefix) and nothing more, whatever the bound credential's push
/// capability. This pins "#734 wires nothing": if a later change wires a
/// write tool without the push-capability gate, the exact-set assertion here
/// breaks rather than shipping an ungated push surface. The bindings the
/// helper builds carry `can_push: None`, so the write tier fails closed
/// (warns) — and still adds no tool.
#[test]
fn repo_write_grant_wires_no_tool_beyond_the_read_pair() {
let mut baseline = built_tool_names(&[], false);
baseline.push("repo_checkout".to_string());
baseline.push("repo_pr".to_string());
baseline.sort();

let write_granted = built_tool_names_with_repos(&["repo.write"], 1);
assert_eq!(
write_granted, baseline,
"repo.write with a non-push-capable credential wires only the read pair"
);
}

/// The write tier's fourth gate (issue #735): `repo_publish` is wired only
/// with `repo.write` **and** a push-capable credential, and never by the read
/// `repo` grant. The non-push-capable half is
/// `repo_write_grant_wires_no_tool_beyond_the_read_pair` above.
#[test]
fn repo_write_with_a_push_capable_credential_wires_repo_publish() {
let publish = "repo_publish".to_string();

// repo.write + a push-capable credential → repo_publish joins the belt,
// and the read pair is still there (write implies read).
let pushable = built_tool_names_with_repos_cap(&["repo.write"], 1, true);
assert!(
pushable.contains(&publish),
"a push-capable `repo.write` must wire repo_publish: {pushable:?}"
);
assert!(
pushable.contains(&"repo_checkout".to_string())
&& pushable.contains(&"repo_pr".to_string()),
"the read pair must still be wired: {pushable:?}"
);

// repo.write but a read-only credential → fail-closed, no publish.
let read_only = built_tool_names_with_repos_cap(&["repo.write"], 1, false);
assert!(
!read_only.contains(&publish),
"a read-only credential must not wire repo_publish: {read_only:?}"
);

// A bare `repo` never confers it, push-capable credential or not.
let bare = built_tool_names_with_repos_cap(&["repo"], 1, true);
assert!(
!bare.contains(&publish),
"bare `repo` (the read tier) must never wire repo_publish: {bare:?}"
);
}

/// Granting `search` must not quietly hand over anything *else*: the
/// credentialed `["search"]` belt is the ungranted belt plus exactly one
/// tool. A namespace that widens the belt beyond its own family is how a
Expand Down
3 changes: 3 additions & 0 deletions src/harness/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3791,6 +3791,7 @@ description = "Builds the product."
last_fetched_millis: None,
size_bytes: 0,
bound_at_millis: 1,
can_push: None,
};

pool.ensure(&rec, &deps).await.expect("first ensure");
Expand Down Expand Up @@ -3886,6 +3887,7 @@ description = "Builds the product."
last_fetched_millis: None,
size_bytes: 0,
bound_at_millis: 1,
can_push: None,
}]
}))
.unwrap();
Expand Down Expand Up @@ -5242,6 +5244,7 @@ budget_usd_daily = 0.0
last_fetched_millis: None,
size_bytes: 0,
bound_at_millis: 1,
can_push: None,
}];
// A registered MCP server is what puts `mcp_list_servers`,
// `mcp_list_tools` and `mcp_call_tool` on the belt — the three
Expand Down
Loading
Loading