mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-10-08 03:10:26 +00:00
Move GitHub token permissions to [run.integrations.github.permissions] (#215)
## Summary
Token scopes describe what *a run* is authorized to do, not server
identity. Today they live under
`[server.integrations.github.permissions]`, which can't be overridden by
`workflow.toml` / `project.toml` (server keys are stripped from
per-workflow layers) — so projects and workflows can't tighten or relax
permissions despite the docs already advertising a per-run config. This
PR moves them under `[run.integrations.github.permissions]`, where the
standard layer-merge (workflow > project > user > defaults) Just Works.
Greenfield, no migration shim.
## What changed
- **New layer/resolved types** in `fabro-config` and `fabro-types`:
`RunIntegrationsLayer`, `RunIntegrationsGithubLayer`, and resolved
counterparts. `permissions` becomes a flat `HashMap<String,
InterpString>` post-resolve; empty = no token requested.
- **Server schema**: `permissions` removed from `GithubIntegrationLayer`
/ `GithubIntegrationSettings`. `deny_unknown_fields` rejects the stale
path.
- **Bundled `workflow.toml` parsing** (`run_manifest.rs`): now goes
through `SettingsLayer` via the new `parse_run_layer_from_settings_toml`
helper, so stale `[server.integrations.github.permissions]` errors
instead of being silently dropped by the old `toml::Table` lift-out.
- **Consumers updated**: server preflight, run launch path, and the CLI
worker (`runner.rs`) all read run-level permissions. CLI worker
previously hardcoded `HashMap::new()` — runs launched via the local CLI
path were getting no `GITHUB_TOKEN` regardless of TOML.
- **Shared helpers** on `RunIntegrationsGithubSettings`:
`is_token_requested()` and `resolve_permissions(lookup)` so server and
CLI don't drift.
- **OpenAPI + TS client** regenerated; new `RunIntegrationsSettings` /
`RunIntegrationsGithubSettings` schemas added, `permissions` removed
from `GithubIntegrationSettings`.
- **Repo workflows + docs** rewritten to the new path. Docs gain a
security-model note (boundary = installation grants; no Fabro-side cap).
## Key design decision: hand-rolled `Combine` for
`RunIntegrationsGithubLayer`
`ReplaceMap`'s "empty inherits from below" semantics (`maps.rs:76-80`)
are wrong here — we want `permissions = {}` in a higher layer to act as
an explicit clear. So the layer field is `Option<HashMap<...>>` with
hand-rolled `Combine`:
| Higher layer | Lower layer | Result |
|---|---|---|
| `None` | anything | lower (inherit) |
| `Some(map)` | anything | `Some(map)` (full replace, including
`Some({})` = clear) |
Not derived: the blanket `Option<T: Combine>` impl would recurse into
the inner `HashMap` and reintroduce empty-fallback. Documented inline in
`layers/run.rs`.
`InterpString` is preserved through resolve and only flattened to
`String` at the start-services boundary, matching the existing pattern.
### Plan Summary
- New `[run.integrations.github.permissions]` layer + resolved types;
remove from server side.
- Hand-rolled `Combine` so empty-wins-as-clear; no change to
`ReplaceMap` semantics for other consumers.
- Strict `SettingsLayer` parse for bundled `workflow.toml` so stale
schema errors loudly.
- Both server and CLI worker paths read run-level permissions via shared
helpers.
- OpenAPI + TS client regenerated; parity test added.
- Repo workflow TOMLs and `integrations/github.mdx` rewritten.
### Fabro Details
<details>
<summary>Ran 0 stages in 61m 23s for $53.41</summary>
| Stage | Duration | Cost | Retries |
|---|---|---|---|
| **Total** | **61m 23s** | **$53.41** | **0** |
</details>
<details>
<summary>Ran <code>ImplementPlan.fabro</code> (12 nodes and 15
edges)</summary>
```dot
digraph ImplementPlan {
graph [
goal="Implement and simplify",
model_stylesheet="
* { model: claude-opus-4-7; }
"
]
rankdir=LR
start [shape=Mdiamond, label="Start"]
exit [shape=Msquare, label="Exit"]
toolchain [label="Toolchain", shape=parallelogram, script="command -v cargo >/dev/null || { curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y && sudo ln -sf $HOME/.cargo/bin/* /usr/local/bin/; }; cargo --version 2>&1", max_retries=0]
preflight_compile [label="Preflight Compile", shape=parallelogram, script="cargo check -q --workspace 2>&1", max_retries=0]
preflight_lint [label="Preflight Lint", shape=parallelogram, script="cargo +nightly-2026-04-14 clippy -q --workspace --all-targets -- -D warnings 2>&1", max_retries=0]
fix_lints [label="Fix Lints", prompt="The preflight lint step failed. Read the build output from context and fix all clippy lint warnings.", max_visits=3]
implement [label="Implement", prompt="Read the plan file referenced in the goal and implement every step. Make all the code changes described in the plan. Use red/green TDD."]
simplify_opus [label="Simplify (Opus)", prompt="@prompts/simplify.md"]
simplify_gpt [label="Simplify (GPT-55)", prompt="@prompts/simplify.md", model="gpt-55"]
verify [label="Verify", shape=parallelogram, script="cargo +nightly-2026-04-14 clippy -q --workspace --all-targets -- -D warnings 2>&1 && cargo nextest run --cargo-quiet --workspace --status-level fail 2>&1 && cargo dev docs refresh 2>&1 && cargo dev docs check 2>&1", goal_gate=true, retry_target="fixup"]
fixup [label="Fixup", prompt="The verify step failed. Read the build output from context and fix all clippy lint warnings, test failures, and generated docs errors.", max_visits=3]
fmt [label="Format", shape=parallelogram, script="cargo +nightly-2026-04-14 fmt --all 2>&1", max_retries=0]
start -> toolchain
toolchain -> preflight_compile [condition="outcome=succeeded"]
toolchain -> exit
preflight_compile -> preflight_lint [condition="outcome=succeeded"]
preflight_compile -> exit
preflight_lint -> implement [condition="outcome=succeeded"]
preflight_lint -> fix_lints
fix_lints -> preflight_lint
implement -> simplify_opus -> simplify_gpt -> verify
verify -> fmt [condition="outcome=succeeded"]
verify -> fixup
fixup -> verify
fmt -> exit
}
```
</details>
⚒️ Generated with [Fabro](https://fabro.sh)
---------
Co-authored-by: Fabro <noreply@fabro.sh>
Co-authored-by: Bryan Helmkamp <bryan@brynary.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
f3784e2e7f
commit
e40dc7d9ad
28 changed files with 808 additions and 111 deletions
|
|
@ -1,7 +1,5 @@
|
|||
_version = 1
|
||||
|
||||
[server.integrations.github]
|
||||
|
||||
[server.integrations.github.permissions]
|
||||
pull_requests = "read"
|
||||
[run.integrations.github.permissions]
|
||||
issues = "read"
|
||||
pull_requests = "write"
|
||||
|
|
|
|||
|
|
@ -1,7 +1,5 @@
|
|||
_version = 1
|
||||
|
||||
[server.integrations.github]
|
||||
|
||||
[server.integrations.github.permissions]
|
||||
[run.integrations.github.permissions]
|
||||
pull_requests = "read"
|
||||
issues = "read"
|
||||
pull_requests = "write"
|
||||
|
|
|
|||
|
|
@ -91,6 +91,7 @@ function sampleSettings({
|
|||
scm: { provider: null, owner: null, repository: null, github: null },
|
||||
pull_request: null,
|
||||
artifacts: { include: [] },
|
||||
integrations: { github: { permissions: {} } },
|
||||
},
|
||||
};
|
||||
}
|
||||
|
|
|
|||
|
|
@ -7135,7 +7135,6 @@ components:
|
|||
- app_id
|
||||
- client_id
|
||||
- slug
|
||||
- permissions
|
||||
- webhooks
|
||||
properties:
|
||||
enabled:
|
||||
|
|
@ -7148,10 +7147,6 @@ components:
|
|||
type: ["string", "null"]
|
||||
slug:
|
||||
type: ["string", "null"]
|
||||
permissions:
|
||||
type: object
|
||||
additionalProperties:
|
||||
type: string
|
||||
webhooks:
|
||||
oneOf:
|
||||
- $ref: "#/components/schemas/IntegrationWebhooksSettings"
|
||||
|
|
@ -7276,6 +7271,7 @@ components:
|
|||
- scm
|
||||
- pull_request
|
||||
- artifacts
|
||||
- integrations
|
||||
properties:
|
||||
goal:
|
||||
oneOf:
|
||||
|
|
@ -7323,6 +7319,24 @@ components:
|
|||
- type: "null"
|
||||
artifacts:
|
||||
$ref: "#/components/schemas/ArtifactsSettings"
|
||||
integrations:
|
||||
$ref: "#/components/schemas/RunIntegrationsSettings"
|
||||
|
||||
RunIntegrationsSettings:
|
||||
type: object
|
||||
required: [github]
|
||||
properties:
|
||||
github:
|
||||
$ref: "#/components/schemas/RunIntegrationsGithubSettings"
|
||||
|
||||
RunIntegrationsGithubSettings:
|
||||
type: object
|
||||
required: [permissions]
|
||||
properties:
|
||||
permissions:
|
||||
type: object
|
||||
additionalProperties:
|
||||
type: string
|
||||
|
||||
RunGoal:
|
||||
oneOf:
|
||||
|
|
|
|||
|
|
@ -32,7 +32,7 @@ The rest of this page describes the `app` strategy, which is required for browse
|
|||
| **Checkpoint pushing** | After each workflow stage, Fabro pushes the run branch and metadata branch back to origin from inside the sandbox |
|
||||
| **Auto-PR** | When `[run.pull_request] enabled = true` in the [run config](/execution/run-configuration#runpull_request), Fabro opens a PR from the agent's working branch after a successful run |
|
||||
| **Auto-merge** | When `[run.pull_request] auto_merge = true`, Fabro enables GitHub's auto-merge on created PRs so they merge automatically once required checks pass |
|
||||
| **Sandbox GITHUB_TOKEN** | When `[server.integrations.github.permissions]` are declared in the server config, Fabro mints a scoped Installation Access Token and injects it as `GITHUB_TOKEN` in the sandbox |
|
||||
| **Sandbox GITHUB_TOKEN** | When `[run.integrations.github.permissions]` are declared at any layer (workflow, project, or user settings), Fabro mints a scoped Installation Access Token and injects it as `GITHUB_TOKEN` in the sandbox |
|
||||
|
||||
## Setup
|
||||
|
||||
|
|
@ -208,16 +208,21 @@ For public repositories, the clone works without credentials. The token is still
|
|||
|
||||
### GITHUB_TOKEN injection
|
||||
|
||||
When a run config declares `[github] permissions`, Fabro mints a scoped Installation Access Token at startup and injects it into the sandbox as the `GITHUB_TOKEN` environment variable. Agents running inside the sandbox can use this token for GitHub API calls, cloning additional private repos, or pushing to branches.
|
||||
When any settings layer declares `[run.integrations.github.permissions]`, Fabro mints a scoped Installation Access Token at startup and injects it into the sandbox as the `GITHUB_TOKEN` environment variable. Agents running inside the sandbox can use this token for GitHub API calls, cloning additional private repos, or pushing to branches.
|
||||
|
||||
```toml title="run.toml"
|
||||
[github]
|
||||
permissions = { contents = "write", pull_requests = "write" }
|
||||
```toml title="workflow.toml"
|
||||
[run.integrations.github.permissions]
|
||||
contents = "write"
|
||||
pull_requests = "write"
|
||||
```
|
||||
|
||||
Only the listed permissions are requested — the token is scoped to the minimum access needed. If the GitHub App isn't configured or the repository lacks an installation, the run logs a warning and continues without the token.
|
||||
|
||||
This also works in `.fabro/project.toml` as a project-level default, so all workflows in the project automatically get a `GITHUB_TOKEN` without repeating the config in each run TOML.
|
||||
The permissions table follows the standard layer-merge order (workflow > project > user > defaults). Set defaults at `[run.integrations.github.permissions]` in `~/.fabro/settings.toml` so every run inherits a baseline; tighten or override per-workflow as needed. A higher layer that defines `permissions = {}` clears the inherited map (no token requested).
|
||||
|
||||
#### Security model
|
||||
|
||||
The upper bound on what Fabro will mint is whatever permissions the GitHub App installation has been granted. Fabro does **not** impose a separate server-side cap on the run-level `permissions` map: any value the App has been granted can be requested by run config. Operators must not run untrusted workflow, project, or user TOML against a broadly-scoped GitHub App installation. Preflight prints the resolved permission set so reviewers can see what each run will request.
|
||||
|
||||
### Checkpoint pushing
|
||||
|
||||
|
|
|
|||
64
lib/crates/fabro-api/tests/run_integrations_round_trip.rs
Normal file
64
lib/crates/fabro-api/tests/run_integrations_round_trip.rs
Normal file
|
|
@ -0,0 +1,64 @@
|
|||
//! JSON parity test for `RunIntegrationsGithubSettings`.
|
||||
//!
|
||||
//! Asserts that the API-side generated `RunIntegrationsGithubSettings` and
|
||||
//! the canonical Rust resolved type round-trip through the same JSON shape.
|
||||
//! Covers both the populated and empty-permissions cases.
|
||||
|
||||
use fabro_api::types::{
|
||||
RunIntegrationsGithubSettings as ApiRunIntegrationsGithubSettings,
|
||||
RunIntegrationsSettings as ApiRunIntegrationsSettings,
|
||||
};
|
||||
use fabro_types::settings::run::{RunIntegrationsGithubSettings, RunIntegrationsSettings};
|
||||
use serde_json::json;
|
||||
|
||||
#[test]
|
||||
fn run_integrations_github_settings_round_trips_with_permissions() {
|
||||
let json_value = json!({
|
||||
"permissions": {
|
||||
"issues": "read",
|
||||
"contents": "write",
|
||||
}
|
||||
});
|
||||
|
||||
let api: ApiRunIntegrationsGithubSettings =
|
||||
serde_json::from_value(json_value.clone()).expect("api type should parse");
|
||||
let canonical: RunIntegrationsGithubSettings =
|
||||
serde_json::from_value(json_value.clone()).expect("canonical type should parse");
|
||||
|
||||
assert_eq!(serde_json::to_value(&api).unwrap(), json_value);
|
||||
assert_eq!(serde_json::to_value(&canonical).unwrap(), json_value);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn run_integrations_github_settings_round_trips_empty_permissions() {
|
||||
// Empty map is the resolved form of "no token requested" — must
|
||||
// serialize as an object, not omitted.
|
||||
let json_value = json!({ "permissions": {} });
|
||||
|
||||
let api: ApiRunIntegrationsGithubSettings =
|
||||
serde_json::from_value(json_value.clone()).expect("api type should parse empty");
|
||||
let canonical: RunIntegrationsGithubSettings =
|
||||
serde_json::from_value(json_value.clone()).expect("canonical type should parse empty");
|
||||
|
||||
assert_eq!(serde_json::to_value(&api).unwrap(), json_value);
|
||||
assert_eq!(serde_json::to_value(&canonical).unwrap(), json_value);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn run_integrations_settings_round_trips() {
|
||||
let json_value = json!({
|
||||
"github": {
|
||||
"permissions": {
|
||||
"issues": "read",
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
let api: ApiRunIntegrationsSettings =
|
||||
serde_json::from_value(json_value.clone()).expect("api wrapper should parse");
|
||||
let canonical: RunIntegrationsSettings =
|
||||
serde_json::from_value(json_value.clone()).expect("canonical wrapper should parse");
|
||||
|
||||
assert_eq!(serde_json::to_value(&api).unwrap(), json_value);
|
||||
assert_eq!(serde_json::to_value(&canonical).unwrap(), json_value);
|
||||
}
|
||||
|
|
@ -4,7 +4,6 @@
|
|||
std::io::BufReader; not on a Tokio path"
|
||||
)]
|
||||
|
||||
use std::collections::HashMap;
|
||||
use std::io::{BufRead as StdBufRead, BufReader as StdBufReader};
|
||||
use std::path::{Path, PathBuf};
|
||||
use std::sync::Arc;
|
||||
|
|
@ -18,7 +17,7 @@ use fabro_interview::{
|
|||
};
|
||||
use fabro_store::{EventEnvelope, RunProjection, RunProjectionReducer};
|
||||
use fabro_types::settings::InterpString;
|
||||
use fabro_types::settings::run::RunMode;
|
||||
use fabro_types::settings::run::{RunMode, RunNamespace};
|
||||
use fabro_types::{
|
||||
ArtifactUpload, EventBody, FailureReason, Principal, RunBlobId, RunEvent, RunId,
|
||||
WorkflowSettings,
|
||||
|
|
@ -117,7 +116,12 @@ pub(crate) async fn execute(
|
|||
artifact_sink,
|
||||
run_control: Some(run_control),
|
||||
github_app,
|
||||
github_permissions: HashMap::new(),
|
||||
github_permissions: run_spec
|
||||
.settings
|
||||
.run
|
||||
.integrations
|
||||
.github
|
||||
.resolve_permissions(process_env_var),
|
||||
vault,
|
||||
on_node: None,
|
||||
registry_override: None,
|
||||
|
|
@ -517,30 +521,23 @@ fn maybe_build_github_credentials(
|
|||
) -> Result<Option<fabro_github::GitHubCredentials>> {
|
||||
let resolved_run = &settings.run;
|
||||
let resolved_server = ServerSettingsBuilder::load_default().ok();
|
||||
let required_github_credentials = (resolved_run.execution.mode != RunMode::DryRun
|
||||
&& clone_sandbox_requires_github_credentials(&resolved_run.sandbox.provider))
|
||||
|| resolved_server
|
||||
.as_ref()
|
||||
.is_some_and(|settings| !settings.server.integrations.github.permissions.is_empty());
|
||||
let pull_request_enabled =
|
||||
resolved_run.execution.mode != RunMode::DryRun && resolved_run.pull_request.is_some();
|
||||
let strategy = resolved_server
|
||||
.as_ref()
|
||||
.map(|settings| settings.server.integrations.github.strategy)
|
||||
let server_ns = resolved_server.as_ref().map(|s| &s.server);
|
||||
let strategy = server_ns
|
||||
.map(|server| server.integrations.github.strategy)
|
||||
.unwrap_or_default();
|
||||
let app_id = resolved_server
|
||||
.as_ref()
|
||||
.and_then(|settings| settings.server.integrations.github.app_id.as_ref())
|
||||
let app_id = server_ns
|
||||
.and_then(|server| server.integrations.github.app_id.as_ref())
|
||||
.map(InterpString::as_source);
|
||||
let app_slug = resolved_server
|
||||
.as_ref()
|
||||
.and_then(|settings| settings.server.integrations.github.slug.as_ref())
|
||||
let app_slug = server_ns
|
||||
.and_then(|server| server.integrations.github.slug.as_ref())
|
||||
.map(InterpString::as_source);
|
||||
|
||||
if required_github_credentials {
|
||||
if requires_github_credentials(resolved_run) {
|
||||
return build_github_credentials(strategy, app_id.as_deref(), app_slug.as_deref(), vault);
|
||||
}
|
||||
|
||||
let pull_request_enabled =
|
||||
resolved_run.execution.mode != RunMode::DryRun && resolved_run.pull_request.is_some();
|
||||
if pull_request_enabled {
|
||||
return Ok(build_github_credentials(
|
||||
strategy,
|
||||
|
|
@ -555,6 +552,26 @@ fn maybe_build_github_credentials(
|
|||
Ok(None)
|
||||
}
|
||||
|
||||
#[expect(
|
||||
clippy::disallowed_methods,
|
||||
reason = "CLI worker InterpString resolution facade for {{ env.* }} values."
|
||||
)]
|
||||
fn process_env_var(name: &str) -> Option<String> {
|
||||
std::env::var(name).ok()
|
||||
}
|
||||
|
||||
/// Hard-gate for the CLI worker path: a run-level token is requested, or
|
||||
/// a clone-based sandbox in non-dry-run mode will need credentials to
|
||||
/// pull the repository. Pull-request-driven credential acquisition is
|
||||
/// handled separately by the caller as a soft fallback.
|
||||
fn requires_github_credentials(run: &RunNamespace) -> bool {
|
||||
if run.integrations.github.is_token_requested() {
|
||||
return true;
|
||||
}
|
||||
run.execution.mode != RunMode::DryRun
|
||||
&& clone_sandbox_requires_github_credentials(&run.sandbox.provider)
|
||||
}
|
||||
|
||||
fn clone_sandbox_requires_github_credentials(provider: &str) -> bool {
|
||||
matches!(provider, "docker" | "daytona")
|
||||
}
|
||||
|
|
@ -941,4 +958,67 @@ mod tests {
|
|||
|
||||
assert!(credential.contains("vault-key"));
|
||||
}
|
||||
|
||||
mod requires_github_credentials_truth_table {
|
||||
//! Truth-table coverage for the worker-side credential gate.
|
||||
//! `InterpString` → `String` resolution is tested in `fabro-types`
|
||||
//! next to `RunIntegrationsGithubSettings::resolve_permissions`.
|
||||
|
||||
use std::collections::HashMap;
|
||||
|
||||
use fabro_types::settings::InterpString;
|
||||
use fabro_types::settings::run::{
|
||||
RunIntegrationsGithubSettings, RunIntegrationsSettings, RunMode, RunNamespace,
|
||||
RunSandboxSettings,
|
||||
};
|
||||
|
||||
use super::super::requires_github_credentials;
|
||||
|
||||
fn run_with(
|
||||
permissions: HashMap<String, InterpString>,
|
||||
provider: &str,
|
||||
mode: RunMode,
|
||||
) -> RunNamespace {
|
||||
let mut run = RunNamespace::default();
|
||||
run.execution.mode = mode;
|
||||
run.sandbox = RunSandboxSettings {
|
||||
provider: provider.to_string(),
|
||||
..RunSandboxSettings::default()
|
||||
};
|
||||
run.integrations = RunIntegrationsSettings {
|
||||
github: RunIntegrationsGithubSettings { permissions },
|
||||
};
|
||||
run
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn requires_github_credentials_when_permissions_non_empty() {
|
||||
let permissions = HashMap::from([("issues".to_string(), InterpString::parse("read"))]);
|
||||
// Even with local sandbox + dry-run, non-empty permissions
|
||||
// force credential acquisition.
|
||||
let run = run_with(permissions, "local", RunMode::DryRun);
|
||||
assert!(requires_github_credentials(&run));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn requires_github_credentials_for_clone_based_provider() {
|
||||
let run = run_with(HashMap::new(), "docker", RunMode::Normal);
|
||||
assert!(requires_github_credentials(&run));
|
||||
|
||||
let daytona = run_with(HashMap::new(), "daytona", RunMode::Normal);
|
||||
assert!(requires_github_credentials(&daytona));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn does_not_require_github_credentials_for_local_clean_run() {
|
||||
let run = run_with(HashMap::new(), "local", RunMode::Normal);
|
||||
assert!(!requires_github_credentials(&run));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn does_not_require_github_credentials_for_clone_provider_in_dry_run() {
|
||||
let run = run_with(HashMap::new(), "docker", RunMode::DryRun);
|
||||
assert!(!requires_github_credentials(&run));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -615,6 +615,11 @@ fn attach_json_errors_without_prompting_for_human_input() {
|
|||
},
|
||||
"hooks": [],
|
||||
"inputs": {},
|
||||
"integrations": {
|
||||
"github": {
|
||||
"permissions": {}
|
||||
}
|
||||
},
|
||||
"interviews": {
|
||||
"discord": null,
|
||||
"provider": null,
|
||||
|
|
|
|||
|
|
@ -24,8 +24,9 @@ pub use run::{
|
|||
GitAuthorLayer, HookAgentMarker, HookEntry, HookTlsMode, InterviewProviderLayer,
|
||||
InterviewsLayer, LocalSandboxLayer, McpEntryLayer, ModelRefOrSplice, NotificationProviderLayer,
|
||||
NotificationRouteLayer, PrepareStep, RunAgentLayer, RunArtifactsLayer, RunCheckpointLayer,
|
||||
RunExecutionLayer, RunGitLayer, RunGoalLayer, RunLayer, RunModelLayer, RunPrepareLayer,
|
||||
RunPullRequestLayer, RunSandboxLayer, RunScmLayer, ScmGitHubLayer, StringOrSplice,
|
||||
RunExecutionLayer, RunGitLayer, RunGoalLayer, RunIntegrationsGithubLayer, RunIntegrationsLayer,
|
||||
RunLayer, RunModelLayer, RunPrepareLayer, RunPullRequestLayer, RunSandboxLayer, RunScmLayer,
|
||||
ScmGitHubLayer, StringOrSplice,
|
||||
};
|
||||
pub use server::{
|
||||
DiscordIntegrationLayer, GithubIntegrationLayer, IntegrationWebhooksLayer,
|
||||
|
|
|
|||
|
|
@ -9,6 +9,7 @@ use fabro_types::settings::run::{
|
|||
use fabro_types::settings::{Duration, InterpString, ModelRef, Size};
|
||||
use serde::{Deserialize, Serialize};
|
||||
|
||||
use super::combine::Combine;
|
||||
use super::maps::{MergeMap, ReplaceMap, StickyMap};
|
||||
use super::splice_array::SPLICE_MARKER;
|
||||
|
||||
|
|
@ -51,6 +52,44 @@ pub struct RunLayer {
|
|||
pub pull_request: Option<RunPullRequestLayer>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub artifacts: Option<RunArtifactsLayer>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub integrations: Option<RunIntegrationsLayer>,
|
||||
}
|
||||
|
||||
/// `[run.integrations]` — run-level integration knobs.
|
||||
#[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize, fabro_macros::Combine)]
|
||||
#[serde(deny_unknown_fields)]
|
||||
pub struct RunIntegrationsLayer {
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub github: Option<RunIntegrationsGithubLayer>,
|
||||
}
|
||||
|
||||
/// `[run.integrations.github]` — runtime GitHub token shape.
|
||||
///
|
||||
/// `Combine` is hand-rolled (not derived) for two reasons:
|
||||
/// 1. `HashMap<String, InterpString>: Combine` is not implemented in this
|
||||
/// crate, so `#[derive(Combine)]` would not even compile.
|
||||
/// 2. The `ReplaceMap` "empty inherits from below" semantics (`maps.rs:76-80`)
|
||||
/// are the wrong fit: we want `Some({})` from a higher layer to be honored
|
||||
/// as an explicit clear (no token requested) rather than fall through to a
|
||||
/// lower layer's permissions.
|
||||
///
|
||||
/// Note: this diverges from sibling map fields like `RunLayer::metadata`
|
||||
/// (`ReplaceMap`), where empty-table-means-inherit. Document the
|
||||
/// difference for workflow authors reading the schema by analogy.
|
||||
#[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize)]
|
||||
#[serde(deny_unknown_fields)]
|
||||
pub struct RunIntegrationsGithubLayer {
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub permissions: Option<HashMap<String, InterpString>>,
|
||||
}
|
||||
|
||||
impl Combine for RunIntegrationsGithubLayer {
|
||||
fn combine(self, other: Self) -> Self {
|
||||
Self {
|
||||
permissions: self.permissions.or(other.permissions),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// The source of a run's goal, either inline literal text or a reference to
|
||||
|
|
|
|||
|
|
@ -8,7 +8,6 @@ use fabro_types::settings::{Duration, InterpString};
|
|||
use serde::{Deserialize, Serialize};
|
||||
|
||||
use super::LogFilter;
|
||||
use super::maps::StickyMap;
|
||||
|
||||
#[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize, fabro_macros::Combine)]
|
||||
#[serde(deny_unknown_fields)]
|
||||
|
|
@ -213,19 +212,17 @@ pub struct ServerIntegrationsLayer {
|
|||
#[serde(deny_unknown_fields)]
|
||||
pub struct GithubIntegrationLayer {
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub enabled: Option<bool>,
|
||||
pub enabled: Option<bool>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub strategy: Option<GithubIntegrationStrategy>,
|
||||
pub strategy: Option<GithubIntegrationStrategy>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub app_id: Option<InterpString>,
|
||||
pub app_id: Option<InterpString>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub client_id: Option<InterpString>,
|
||||
pub client_id: Option<InterpString>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub slug: Option<InterpString>,
|
||||
#[serde(default, skip_serializing_if = "StickyMap::is_empty")]
|
||||
pub permissions: StickyMap<InterpString>,
|
||||
pub slug: Option<InterpString>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub webhooks: Option<IntegrationWebhooksLayer>,
|
||||
pub webhooks: Option<IntegrationWebhooksLayer>,
|
||||
}
|
||||
|
||||
/// `[server.integrations.slack]` — Slack workspace credentials and defaults.
|
||||
|
|
|
|||
|
|
@ -44,12 +44,13 @@ pub use layers::{
|
|||
LogFilter, McpEntryLayer, MergeMap, ModelRefOrSplice, NotificationProviderLayer,
|
||||
NotificationRouteLayer, ObjectStoreLocalLayer, ObjectStoreS3Layer, PrepareStep, ProjectLayer,
|
||||
ReplaceMap, RunAgentLayer, RunArtifactsLayer, RunCheckpointLayer, RunExecutionLayer,
|
||||
RunGitLayer, RunGoalLayer, RunLayer, RunModelLayer, RunPrepareLayer, RunPullRequestLayer,
|
||||
RunSandboxLayer, RunScmLayer, ScmGitHubLayer, ServerApiLayer, ServerArtifactsLayer,
|
||||
ServerAuthGithubLayer, ServerAuthLayer, ServerIntegrationsLayer, ServerIpAllowlistLayer,
|
||||
ServerIpAllowlistOverrideLayer, ServerLayer, ServerListenLayer, ServerLoggingLayer,
|
||||
ServerSchedulerLayer, ServerSlateDbLayer, ServerStorageLayer, ServerWebLayer,
|
||||
SlackIntegrationLayer, StickyMap, StringOrSplice, TeamsIntegrationLayer, WorkflowLayer,
|
||||
RunGitLayer, RunGoalLayer, RunIntegrationsGithubLayer, RunIntegrationsLayer, RunLayer,
|
||||
RunModelLayer, RunPrepareLayer, RunPullRequestLayer, RunSandboxLayer, RunScmLayer,
|
||||
ScmGitHubLayer, ServerApiLayer, ServerArtifactsLayer, ServerAuthGithubLayer, ServerAuthLayer,
|
||||
ServerIntegrationsLayer, ServerIpAllowlistLayer, ServerIpAllowlistOverrideLayer, ServerLayer,
|
||||
ServerListenLayer, ServerLoggingLayer, ServerSchedulerLayer, ServerSlateDbLayer,
|
||||
ServerStorageLayer, ServerWebLayer, SlackIntegrationLayer, StickyMap, StringOrSplice,
|
||||
TeamsIntegrationLayer, WorkflowLayer,
|
||||
};
|
||||
pub(crate) use layers::{Combine, SettingsLayer};
|
||||
pub use logging::{resolve_log_destination, resolve_log_destination_with_env};
|
||||
|
|
|
|||
|
|
@ -115,7 +115,10 @@ fn rename_hint(key: &str) -> Option<String> {
|
|||
"max_concurrent_runs" => "rename to `[server.scheduler]` field",
|
||||
"fabro" => "rename to `[project]`; `fabro.root` becomes `project.directory`",
|
||||
"git" => "split into `[run.git]` (local git behavior) and `[server.integrations.github]`",
|
||||
"github" => "rename to `[server.integrations.github]`",
|
||||
"github" => {
|
||||
"split into `[server.integrations.github]` (App identity/auth) and \
|
||||
`[run.integrations.github.permissions]` (sandbox token scopes)"
|
||||
}
|
||||
"slack" => "move under `[server.integrations.slack]`",
|
||||
"log" => "rename to `[server.logging]` or `[cli.logging]` depending on owner",
|
||||
"prevent_idle_sleep" => "rename to `[cli.exec] prevent_idle_sleep`",
|
||||
|
|
|
|||
|
|
@ -4,9 +4,9 @@ use fabro_types::settings::run::{
|
|||
GitAuthorSettings, HookDefinition, HookType, InterviewProviderSettings, LocalSandboxSettings,
|
||||
McpServerSettings, McpTransport, MergeStrategy, NotificationProviderSettings,
|
||||
NotificationRouteSettings, PullRequestSettings, RunAgentSettings, RunCheckpointSettings,
|
||||
RunExecutionSettings, RunGitSettings, RunGoal, RunInterviewsSettings, RunModelSettings,
|
||||
RunNamespace, RunPrepareSettings, RunSandboxSettings, RunScmSettings, ScmGitHubSettings,
|
||||
TlsMode,
|
||||
RunExecutionSettings, RunGitSettings, RunGoal, RunIntegrationsGithubSettings,
|
||||
RunIntegrationsSettings, RunInterviewsSettings, RunModelSettings, RunNamespace,
|
||||
RunPrepareSettings, RunSandboxSettings, RunScmSettings, ScmGitHubSettings, TlsMode,
|
||||
};
|
||||
|
||||
use super::ResolveError;
|
||||
|
|
@ -14,8 +14,9 @@ use crate::{
|
|||
DaytonaDockerfileLayer, DaytonaSandboxLayer, HookAgentMarker, HookEntry, HookTlsMode,
|
||||
InterviewProviderLayer, InterviewsLayer, McpEntryLayer, ModelRefOrSplice,
|
||||
NotificationProviderLayer, NotificationRouteLayer, RunAgentLayer, RunArtifactsLayer,
|
||||
RunCheckpointLayer, RunExecutionLayer, RunGitLayer, RunGoalLayer, RunLayer, RunModelLayer,
|
||||
RunPrepareLayer, RunPullRequestLayer, RunSandboxLayer, RunScmLayer, StringOrSplice,
|
||||
RunCheckpointLayer, RunExecutionLayer, RunGitLayer, RunGoalLayer, RunIntegrationsLayer,
|
||||
RunLayer, RunModelLayer, RunPrepareLayer, RunPullRequestLayer, RunSandboxLayer, RunScmLayer,
|
||||
StringOrSplice,
|
||||
};
|
||||
|
||||
pub fn resolve_run(layer: &RunLayer, errors: &mut Vec<ResolveError>) -> RunNamespace {
|
||||
|
|
@ -46,9 +47,23 @@ pub fn resolve_run(layer: &RunLayer, errors: &mut Vec<ResolveError>) -> RunNames
|
|||
scm: resolve_scm(layer.scm.as_ref()),
|
||||
pull_request: resolve_pull_request(layer.pull_request.as_ref()),
|
||||
artifacts: resolve_artifacts(layer.artifacts.as_ref()),
|
||||
integrations: resolve_integrations(layer.integrations.as_ref()),
|
||||
}
|
||||
}
|
||||
|
||||
fn resolve_integrations(layer: Option<&RunIntegrationsLayer>) -> RunIntegrationsSettings {
|
||||
let github = layer
|
||||
.and_then(|integrations| integrations.github.as_ref())
|
||||
.map(|github| RunIntegrationsGithubSettings {
|
||||
// Collapse `Option<HashMap<...>>` -> `HashMap<...>`: both `None`
|
||||
// and `Some({})` resolve to an empty map (no token requested).
|
||||
// The presence distinction is only meaningful at merge time.
|
||||
permissions: github.permissions.clone().unwrap_or_default(),
|
||||
})
|
||||
.unwrap_or_default();
|
||||
RunIntegrationsSettings { github }
|
||||
}
|
||||
|
||||
fn resolve_goal(goal: Option<&RunGoalLayer>) -> Option<RunGoal> {
|
||||
match goal? {
|
||||
RunGoalLayer::Inline(value) => Some(RunGoal::Inline(value.clone())),
|
||||
|
|
|
|||
|
|
@ -457,13 +457,12 @@ fn resolve_integrations(
|
|||
github: layer
|
||||
.and_then(|integrations| integrations.github.as_ref())
|
||||
.map(|github| GithubIntegrationSettings {
|
||||
enabled: github.enabled.unwrap_or(true),
|
||||
strategy: github.strategy.unwrap_or_default(),
|
||||
app_id: github.app_id.clone(),
|
||||
client_id: github.client_id.clone(),
|
||||
slug: github.slug.clone(),
|
||||
permissions: github.permissions.clone().into_inner(),
|
||||
webhooks: github.webhooks.as_ref().map(|webhooks| {
|
||||
enabled: github.enabled.unwrap_or(true),
|
||||
strategy: github.strategy.unwrap_or_default(),
|
||||
app_id: github.app_id.clone(),
|
||||
client_id: github.client_id.clone(),
|
||||
slug: github.slug.clone(),
|
||||
webhooks: github.webhooks.as_ref().map(|webhooks| {
|
||||
resolve_github_webhooks(webhooks, "server.integrations.github.webhooks", errors)
|
||||
}),
|
||||
})
|
||||
|
|
|
|||
|
|
@ -25,6 +25,27 @@ pub(crate) fn load_run_config(path: &Path) -> Result<SettingsLayer> {
|
|||
load_settings_path(path)
|
||||
}
|
||||
|
||||
/// Parse a settings TOML source string and extract its `[run]` layer.
|
||||
///
|
||||
/// Goes through the strict `SettingsLayer` `FromStr` impl, so valid settings
|
||||
/// domains (`_version`, `[workflow]`, `[run]`, `[server]`, etc.) parse cleanly,
|
||||
/// while unknown top-level domains or unknown nested keys under a known domain
|
||||
/// trip `deny_unknown_fields`. Bundled
|
||||
/// `workflow.toml` configurations parse through this path so stale
|
||||
/// schema (e.g. `[server.integrations.github.permissions]` after the
|
||||
/// move to `[run.integrations.github.permissions]`) is rejected up front
|
||||
/// rather than silently dropped by `RunLayer::try_from(toml::Value)`.
|
||||
///
|
||||
/// Exists as a public helper because `SettingsLayer` itself is
|
||||
/// `pub(crate)`, so external crates cannot replicate this two-line dance
|
||||
/// inline.
|
||||
pub fn parse_run_layer_from_settings_toml(source: &str) -> Result<RunLayer> {
|
||||
let layer = source
|
||||
.parse::<SettingsLayer>()
|
||||
.map_err(|err| crate::Error::parse("Failed to parse run config TOML", err))?;
|
||||
Ok(layer.run.unwrap_or_default())
|
||||
}
|
||||
|
||||
/// Resolve a graph path relative to a workflow.toml.
|
||||
#[must_use]
|
||||
pub fn resolve_graph_path(workflow_toml: &Path, graph_relative: &str) -> PathBuf {
|
||||
|
|
|
|||
|
|
@ -80,3 +80,183 @@ name = "sonnet"
|
|||
);
|
||||
assert_eq!(settings.model.name, Some(InterpString::parse("sonnet")));
|
||||
}
|
||||
|
||||
mod run_integrations_github_permissions {
|
||||
//! Layer + resolver tests for `[run.integrations.github.permissions]`.
|
||||
//!
|
||||
//! `[run.integrations.github]` uses a hand-rolled `Combine` impl so a
|
||||
//! higher layer that sets `permissions = {}` clears the inherited map
|
||||
//! ("empty wins as clear"), and an absent block inherits from below.
|
||||
|
||||
use std::collections::HashMap;
|
||||
|
||||
use fabro_types::settings::InterpString;
|
||||
|
||||
use crate::layers::Combine;
|
||||
use crate::{SettingsLayer, WorkflowSettingsBuilder};
|
||||
|
||||
fn parse_settings(source: &str) -> SettingsLayer {
|
||||
source
|
||||
.parse::<SettingsLayer>()
|
||||
.expect("fixture should parse via SettingsLayer")
|
||||
}
|
||||
|
||||
fn one_perm(key: &str, value: &str) -> HashMap<String, InterpString> {
|
||||
HashMap::from([(key.to_string(), InterpString::parse(value))])
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn workflow_layer_parses_run_level_permissions() {
|
||||
let layer = parse_settings(
|
||||
r#"
|
||||
_version = 1
|
||||
|
||||
[run.integrations.github.permissions]
|
||||
issues = "read"
|
||||
"#,
|
||||
);
|
||||
let github = layer
|
||||
.run
|
||||
.as_ref()
|
||||
.and_then(|run| run.integrations.as_ref())
|
||||
.and_then(|integrations| integrations.github.as_ref())
|
||||
.expect("permissions block should be parsed into RunIntegrationsGithubLayer");
|
||||
let permissions = github
|
||||
.permissions
|
||||
.as_ref()
|
||||
.expect("permissions table should be present");
|
||||
assert_eq!(permissions.len(), 1);
|
||||
assert_eq!(
|
||||
permissions.get("issues"),
|
||||
Some(&InterpString::parse("read"))
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn workflow_replaces_user_permissions_wholesale() {
|
||||
let workflow = parse_settings(
|
||||
r#"
|
||||
_version = 1
|
||||
|
||||
[run.integrations.github.permissions]
|
||||
issues = "write"
|
||||
"#,
|
||||
);
|
||||
let user = parse_settings(
|
||||
r#"
|
||||
_version = 1
|
||||
|
||||
[run.integrations.github.permissions]
|
||||
contents = "read"
|
||||
"#,
|
||||
);
|
||||
let merged = workflow.combine(user);
|
||||
|
||||
let resolved = WorkflowSettingsBuilder::from_layer(&merged)
|
||||
.expect("merged settings should resolve")
|
||||
.run;
|
||||
|
||||
assert_eq!(
|
||||
resolved.integrations.github.permissions,
|
||||
one_perm("issues", "write",)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn absent_higher_layer_inherits_lower_permissions() {
|
||||
let workflow = parse_settings("_version = 1\n");
|
||||
let user = parse_settings(
|
||||
r#"
|
||||
_version = 1
|
||||
|
||||
[run.integrations.github.permissions]
|
||||
contents = "read"
|
||||
"#,
|
||||
);
|
||||
let merged = workflow.combine(user);
|
||||
|
||||
let resolved = WorkflowSettingsBuilder::from_layer(&merged)
|
||||
.expect("merged settings should resolve")
|
||||
.run;
|
||||
|
||||
assert_eq!(
|
||||
resolved.integrations.github.permissions,
|
||||
one_perm("contents", "read",)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn empty_higher_layer_clears_inherited_permissions() {
|
||||
// Workflow declares `permissions = {}` -> Some(empty map). The
|
||||
// hand-rolled `Combine` keeps Some over fallback, so the resolved
|
||||
// map is empty (no token requested) — empty-wins-as-clear.
|
||||
let workflow = parse_settings(
|
||||
r"
|
||||
_version = 1
|
||||
|
||||
[run.integrations.github]
|
||||
permissions = {}
|
||||
",
|
||||
);
|
||||
let user = parse_settings(
|
||||
r#"
|
||||
_version = 1
|
||||
|
||||
[run.integrations.github.permissions]
|
||||
contents = "read"
|
||||
"#,
|
||||
);
|
||||
let merged = workflow.combine(user);
|
||||
|
||||
let resolved = WorkflowSettingsBuilder::from_layer(&merged)
|
||||
.expect("merged settings should resolve")
|
||||
.run;
|
||||
|
||||
assert!(
|
||||
resolved.integrations.github.permissions.is_empty(),
|
||||
"empty higher layer should clear inherited permissions, got {:?}",
|
||||
resolved.integrations.github.permissions
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn server_integrations_github_permissions_is_now_unknown_field() {
|
||||
let err = r#"
|
||||
_version = 1
|
||||
|
||||
[server.integrations.github.permissions]
|
||||
issues = "read"
|
||||
"#
|
||||
.parse::<SettingsLayer>()
|
||||
.expect_err("stale [server.integrations.github.permissions] must error");
|
||||
let message = err.to_string();
|
||||
assert!(
|
||||
message.contains("permissions") || message.contains("unknown field"),
|
||||
"expected unknown-field error mentioning permissions, got: {message}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolver_preserves_interp_string_in_permissions() {
|
||||
let resolved = WorkflowSettingsBuilder::from_toml(
|
||||
r#"
|
||||
_version = 1
|
||||
|
||||
[run.integrations.github.permissions]
|
||||
issues = "{{ env.GH_PERM_LEVEL }}"
|
||||
"#,
|
||||
)
|
||||
.expect("env-token permissions should resolve")
|
||||
.run;
|
||||
|
||||
let issues = resolved
|
||||
.integrations
|
||||
.github
|
||||
.permissions
|
||||
.get("issues")
|
||||
.expect("issues permission should be present");
|
||||
// Resolver does NOT eagerly resolve env tokens; the `InterpString`
|
||||
// form is preserved for late binding by the consumer.
|
||||
assert_eq!(issues.as_source(), "{{ env.GH_PERM_LEVEL }}");
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@ use std::time::Duration;
|
|||
use anyhow::{Context as _, Result, anyhow, bail};
|
||||
use fabro_api::types;
|
||||
use fabro_auth::auth_issue_message;
|
||||
use fabro_config::run::parse_run_layer_from_settings_toml;
|
||||
use fabro_config::{
|
||||
CliLayer, CliOutputLayer, DaytonaDockerfileLayer, DockerSandboxLayer, LocalSandboxLayer,
|
||||
ReplaceMap, RunExecutionLayer, RunLayer, RunModelLayer, RunSandboxLayer,
|
||||
|
|
@ -24,7 +25,6 @@ use fabro_sandbox::daytona::DaytonaConfig;
|
|||
use fabro_sandbox::redact::redact_auth_url;
|
||||
use fabro_sandbox::{DockerSandboxOptions, Sandbox, SandboxProvider, SandboxSpec};
|
||||
use fabro_static::EnvVars;
|
||||
use fabro_types::settings::ServerNamespace;
|
||||
use fabro_types::settings::cli::OutputVerbosity;
|
||||
use fabro_types::settings::interp::InterpString;
|
||||
use fabro_types::settings::run::{
|
||||
|
|
@ -292,16 +292,14 @@ fn root_workflow_run_layer(workflow: &BundledWorkflow) -> Result<RunLayer> {
|
|||
return Ok(RunLayer::default());
|
||||
};
|
||||
|
||||
let mut document: toml::Table = config
|
||||
.source
|
||||
.parse()
|
||||
// Parse via `SettingsLayer` so unknown nested keys (like a stale
|
||||
// `[server.integrations.github.permissions]` after the move to
|
||||
// `[run.integrations.github.permissions]`) trip
|
||||
// `deny_unknown_fields`. Other valid top-level domains
|
||||
// (`_version`, `[workflow]`, `[server.*]`) parse cleanly and the
|
||||
// builder ignores everything outside `[run]` for this code path.
|
||||
let mut run = parse_run_layer_from_settings_toml(&config.source)
|
||||
.context("Failed to parse run config TOML")?;
|
||||
let mut run = document
|
||||
.remove("run")
|
||||
.map(toml::Value::try_into::<RunLayer>)
|
||||
.transpose()
|
||||
.context("Failed to parse run config TOML")?
|
||||
.unwrap_or_default();
|
||||
resolve_manifest_dockerfile(&mut run, &config.path, &workflow.files)?;
|
||||
Ok(run)
|
||||
}
|
||||
|
|
@ -494,7 +492,7 @@ async fn build_preflight_report(
|
|||
sandbox_provider
|
||||
};
|
||||
let needs_github_credentials =
|
||||
sandbox_provider.is_clone_based() || !github_integration.permissions.is_empty();
|
||||
sandbox_provider.is_clone_based() || resolved_run.integrations.github.is_token_requested();
|
||||
let github_app = if needs_github_credentials {
|
||||
state
|
||||
.github_credentials(github_integration)
|
||||
|
|
@ -529,7 +527,7 @@ async fn build_preflight_report(
|
|||
&configured_providers,
|
||||
)
|
||||
.await;
|
||||
run_github_token_check(&mut checks, prepared, &server_settings.server, github_app).await;
|
||||
run_github_token_check(&mut checks, prepared, &resolved_run, github_app).await;
|
||||
|
||||
let checks_ok = sandbox_ok && repository_access_ok && llm_ok;
|
||||
|
||||
|
|
@ -1184,22 +1182,19 @@ fn runtime_docker_config(settings: &DockerSettings) -> DockerSandboxOptions {
|
|||
async fn run_github_token_check(
|
||||
checks: &mut Vec<CheckResult>,
|
||||
prepared: &PreparedManifest,
|
||||
settings: &ServerNamespace,
|
||||
resolved_run: &RunNamespace,
|
||||
github_app: Option<fabro_github::GitHubCredentials>,
|
||||
) {
|
||||
if settings.integrations.github.permissions.is_empty() {
|
||||
if !resolved_run.integrations.github.is_token_requested() {
|
||||
return;
|
||||
}
|
||||
|
||||
// Resolve InterpString permission values eagerly for token minting and
|
||||
// for display in the preflight report.
|
||||
let github_permissions: HashMap<String, String> = settings
|
||||
let github_permissions = resolved_run
|
||||
.integrations
|
||||
.github
|
||||
.permissions
|
||||
.iter()
|
||||
.map(|(k, v)| (k.clone(), v.as_source()))
|
||||
.collect();
|
||||
.resolve_permissions(process_env_var);
|
||||
|
||||
let perm_details = github_permissions
|
||||
.iter()
|
||||
|
|
@ -1228,7 +1223,7 @@ async fn run_github_token_check(
|
|||
name: "GitHub Token".into(),
|
||||
status: CheckStatus::Warning,
|
||||
summary: "skipped".into(),
|
||||
details: vec![],
|
||||
details: perm_details,
|
||||
remediation: Some("No GitHub credentials or origin URL available".to_string()),
|
||||
}),
|
||||
}
|
||||
|
|
@ -1818,6 +1813,57 @@ app_id = "fixture-app-id"
|
|||
assert_eq!(response.checks.sections[0].checks.len(), 2);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn preflight_runs_github_token_check_when_run_level_permissions_declared() {
|
||||
// When a workflow declares `[run.integrations.github.permissions]`,
|
||||
// `run_github_token_check` is invoked and surfaces a "GitHub Token"
|
||||
// entry in the preflight report. With no configured GitHub App
|
||||
// credentials in the test fixture, the check status is
|
||||
// Warning/skipped — the important assertion is that the entry
|
||||
// *exists*, proving the gate now reads from run-level config.
|
||||
let state = crate::test_support::test_app_state();
|
||||
let mut manifest = minimal_manifest();
|
||||
manifest.workflows.get_mut("workflow.fabro").unwrap().config =
|
||||
Some(types::ManifestWorkflowConfig {
|
||||
path: "workflow.toml".to_string(),
|
||||
source: r#"_version = 1
|
||||
|
||||
[run.sandbox]
|
||||
provider = "local"
|
||||
|
||||
[run.integrations.github.permissions]
|
||||
issues = "read"
|
||||
"#
|
||||
.to_string(),
|
||||
});
|
||||
|
||||
let prepared = prepare_manifest(
|
||||
&manifest_run_defaults(Some(&default_settings_fixture())),
|
||||
&manifest,
|
||||
)
|
||||
.unwrap();
|
||||
let validated = validate_prepared_manifest(&prepared).unwrap();
|
||||
assert!(!validated.has_errors());
|
||||
|
||||
let (response, _ok) = run_preflight(state.as_ref(), &prepared, &validated)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
assert!(
|
||||
response.checks.sections[0]
|
||||
.checks
|
||||
.iter()
|
||||
.any(|check| check.name == "GitHub Token"),
|
||||
"expected GitHub Token check to run when run-level permissions are set; \
|
||||
checks were {:?}",
|
||||
response.checks.sections[0]
|
||||
.checks
|
||||
.iter()
|
||||
.map(|c| c.name.as_str())
|
||||
.collect::<Vec<_>>()
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn preflight_allows_pull_request_enabled_without_github_credentials() {
|
||||
let state = crate::test_support::test_app_state();
|
||||
|
|
@ -1980,4 +2026,89 @@ digraph Demo {
|
|||
);
|
||||
assert!(response_mock.calls_async().await >= 1);
|
||||
}
|
||||
|
||||
mod root_workflow_run_layer_tests {
|
||||
//! `root_workflow_run_layer` parses bundled workflow.toml through
|
||||
//! the strict `SettingsLayer` schema, so unknown fields anywhere in
|
||||
//! the document trip `deny_unknown_fields`.
|
||||
|
||||
use fabro_workflow::ManifestPath;
|
||||
use fabro_workflow::workflow_bundle::{BundledWorkflow, ParsedWorkflowConfig};
|
||||
|
||||
use super::super::root_workflow_run_layer;
|
||||
|
||||
fn workflow_with_config(source: &str) -> BundledWorkflow {
|
||||
BundledWorkflow {
|
||||
path: ManifestPath::from_wire("workflow.fabro").expect("path should be valid"),
|
||||
source: "digraph G {}".to_string(),
|
||||
config: Some(ParsedWorkflowConfig {
|
||||
path: ManifestPath::from_wire("workflow.toml")
|
||||
.expect("config path should be valid"),
|
||||
source: source.to_string(),
|
||||
}),
|
||||
files: std::collections::HashMap::new(),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parses_run_integrations_github_permissions() {
|
||||
let workflow = workflow_with_config(
|
||||
r#"_version = 1
|
||||
|
||||
[run.integrations.github.permissions]
|
||||
issues = "read"
|
||||
"#,
|
||||
);
|
||||
|
||||
let run = root_workflow_run_layer(&workflow).expect("workflow.toml should parse");
|
||||
let github = run
|
||||
.integrations
|
||||
.as_ref()
|
||||
.and_then(|integrations| integrations.github.as_ref())
|
||||
.expect("integrations.github should be present");
|
||||
let permissions = github
|
||||
.permissions
|
||||
.as_ref()
|
||||
.expect("permissions should be present");
|
||||
assert_eq!(permissions.len(), 1);
|
||||
assert!(permissions.contains_key("issues"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rejects_stale_server_integrations_github_permissions() {
|
||||
let workflow = workflow_with_config(
|
||||
r#"_version = 1
|
||||
|
||||
[server.integrations.github.permissions]
|
||||
issues = "read"
|
||||
"#,
|
||||
);
|
||||
|
||||
let err = root_workflow_run_layer(&workflow)
|
||||
.expect_err("stale [server.integrations.github.permissions] should be rejected");
|
||||
let message = format!("{err:#}");
|
||||
assert!(
|
||||
message.contains("permissions") || message.contains("unknown field"),
|
||||
"expected unknown-field error, got: {message}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn accepts_workflow_block_and_version() {
|
||||
let workflow = workflow_with_config(
|
||||
r#"_version = 1
|
||||
|
||||
[workflow]
|
||||
name = "demo"
|
||||
|
||||
[run.integrations.github.permissions]
|
||||
contents = "read"
|
||||
"#,
|
||||
);
|
||||
|
||||
let run =
|
||||
root_workflow_run_layer(&workflow).expect("workflow + run blocks should parse");
|
||||
assert!(run.integrations.is_some());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -2718,7 +2718,7 @@ async fn execute_run_in_process(state: Arc<AppState>, run_id: RunId) {
|
|||
.is_some_and(|origin| !origin.trim().is_empty());
|
||||
let pull_request_can_use_github_credentials =
|
||||
settings.execution.mode != RunMode::DryRun && settings.pull_request.is_some();
|
||||
if !github_settings.permissions.is_empty() {
|
||||
if settings.integrations.github.is_token_requested() {
|
||||
state.github_credentials(github_settings)
|
||||
} else if clone_can_use_github_credentials || pull_request_can_use_github_credentials {
|
||||
match state.github_credentials(github_settings) {
|
||||
|
|
@ -2754,16 +2754,13 @@ async fn execute_run_in_process(state: Arc<AppState>, run_id: RunId) {
|
|||
return;
|
||||
}
|
||||
};
|
||||
let github_permissions = github_settings
|
||||
.permissions
|
||||
.iter()
|
||||
.map(|(name, value)| {
|
||||
let resolved = value
|
||||
.resolve(process_env_var)
|
||||
.map_or_else(|_| value.as_source(), |resolved| resolved.value);
|
||||
(name.clone(), resolved)
|
||||
})
|
||||
.collect();
|
||||
let github_permissions = persisted
|
||||
.run_spec()
|
||||
.settings
|
||||
.run
|
||||
.integrations
|
||||
.github
|
||||
.resolve_permissions(process_env_var);
|
||||
let services = operations::StartServices {
|
||||
run_id,
|
||||
cancel_token: cancel_token.clone(),
|
||||
|
|
|
|||
|
|
@ -40,8 +40,9 @@ pub use run::{
|
|||
GitAuthorSettings, HookDefinition, HookType, InterviewProviderSettings, McpServerSettings,
|
||||
McpTransport, NotificationProviderSettings, NotificationRouteSettings, PullRequestSettings,
|
||||
RunAgentSettings, RunCheckpointSettings, RunExecutionSettings, RunGitSettings, RunGoal,
|
||||
RunInterviewsSettings, RunModelSettings, RunNamespace, RunPrepareSettings, RunSandboxSettings,
|
||||
RunScmSettings, ScmGitHubSettings, TlsMode,
|
||||
RunIntegrationsGithubSettings, RunIntegrationsSettings, RunInterviewsSettings,
|
||||
RunModelSettings, RunNamespace, RunPrepareSettings, RunSandboxSettings, RunScmSettings,
|
||||
ScmGitHubSettings, TlsMode,
|
||||
};
|
||||
pub use server::{
|
||||
DiscordIntegrationSettings, GithubIntegrationSettings, IntegrationWebhooksSettings,
|
||||
|
|
|
|||
|
|
@ -35,6 +35,101 @@ pub struct RunNamespace {
|
|||
pub scm: RunScmSettings,
|
||||
pub pull_request: Option<PullRequestSettings>,
|
||||
pub artifacts: ArtifactsSettings,
|
||||
pub integrations: RunIntegrationsSettings,
|
||||
}
|
||||
|
||||
/// `[run.integrations]` — run-level integration knobs.
|
||||
#[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize)]
|
||||
pub struct RunIntegrationsSettings {
|
||||
pub github: RunIntegrationsGithubSettings,
|
||||
}
|
||||
|
||||
/// `[run.integrations.github]` — runtime GitHub token shape.
|
||||
///
|
||||
/// `permissions` is empty when no token should be requested. The
|
||||
/// presence-vs-clear distinction is only meaningful at the layer-merge
|
||||
/// stage; the resolved form collapses both `None` and `Some({})` into an
|
||||
/// empty map.
|
||||
#[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize)]
|
||||
pub struct RunIntegrationsGithubSettings {
|
||||
pub permissions: HashMap<String, InterpString>,
|
||||
}
|
||||
|
||||
impl RunIntegrationsGithubSettings {
|
||||
/// Whether the run config asks Fabro to mint a `GITHUB_TOKEN` for the
|
||||
/// sandbox. Empty `permissions` means "no token requested" — the only
|
||||
/// resolved-form sentinel for this state.
|
||||
pub fn is_token_requested(&self) -> bool {
|
||||
!self.permissions.is_empty()
|
||||
}
|
||||
|
||||
/// Resolve every `permissions` value's `{{ env.* }}` tokens via
|
||||
/// `lookup`, falling back to `InterpString::as_source()` when
|
||||
/// resolution fails so callers see a recognizable diagnostic instead
|
||||
/// of a silently dropped key. The `lookup` seam keeps tests free of
|
||||
/// process-env coupling; production callers pass a thin wrapper over
|
||||
/// `std::env::var`.
|
||||
pub fn resolve_permissions<F>(&self, mut lookup: F) -> HashMap<String, String>
|
||||
where
|
||||
F: FnMut(&str) -> Option<String>,
|
||||
{
|
||||
self.permissions
|
||||
.iter()
|
||||
.map(|(name, value)| {
|
||||
let resolved = value
|
||||
.resolve(&mut lookup)
|
||||
.map_or_else(|_| value.as_source(), |resolved| resolved.value);
|
||||
(name.clone(), resolved)
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod run_integrations_github_tests {
|
||||
use super::{HashMap, InterpString, RunIntegrationsGithubSettings};
|
||||
|
||||
fn settings(permissions: &[(&str, &str)]) -> RunIntegrationsGithubSettings {
|
||||
RunIntegrationsGithubSettings {
|
||||
permissions: permissions
|
||||
.iter()
|
||||
.map(|(k, v)| ((*k).to_string(), InterpString::parse(v)))
|
||||
.collect(),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn is_token_requested_reflects_permissions_presence() {
|
||||
assert!(!settings(&[]).is_token_requested());
|
||||
assert!(settings(&[("issues", "read")]).is_token_requested());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_permissions_substitutes_env_tokens_via_lookup() {
|
||||
let s = settings(&[("issues", "{{ env.GH_PERM_LEVEL }}"), ("contents", "read")]);
|
||||
let resolved = s.resolve_permissions(|name| match name {
|
||||
"GH_PERM_LEVEL" => Some("write".to_string()),
|
||||
_ => None,
|
||||
});
|
||||
assert_eq!(resolved.get("issues"), Some(&"write".to_string()));
|
||||
assert_eq!(resolved.get("contents"), Some(&"read".to_string()));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_permissions_falls_back_to_source_when_lookup_fails() {
|
||||
let s = settings(&[("issues", "{{ env.GH_PERM_MISSING }}")]);
|
||||
let resolved = s.resolve_permissions(|_| None);
|
||||
assert_eq!(
|
||||
resolved.get("issues"),
|
||||
Some(&"{{ env.GH_PERM_MISSING }}".to_string())
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_permissions_is_empty_for_empty_settings() {
|
||||
let s: HashMap<String, String> = settings(&[]).resolve_permissions(|_| None);
|
||||
assert!(s.is_empty());
|
||||
}
|
||||
}
|
||||
|
||||
/// The resolved source of a run goal.
|
||||
|
|
|
|||
|
|
@ -5,7 +5,6 @@
|
|||
//! scheduler, logging, integrations). Same-host and split-host deployments
|
||||
//! use the same schema.
|
||||
|
||||
use std::collections::HashMap;
|
||||
use std::net::SocketAddr;
|
||||
use std::time::Duration as StdDuration;
|
||||
|
||||
|
|
@ -267,13 +266,12 @@ pub struct ServerIntegrationsSettings {
|
|||
|
||||
#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)]
|
||||
pub struct GithubIntegrationSettings {
|
||||
pub enabled: bool,
|
||||
pub strategy: GithubIntegrationStrategy,
|
||||
pub app_id: Option<InterpString>,
|
||||
pub client_id: Option<InterpString>,
|
||||
pub slug: Option<InterpString>,
|
||||
pub permissions: HashMap<String, InterpString>,
|
||||
pub webhooks: Option<IntegrationWebhooksSettings>,
|
||||
pub enabled: bool,
|
||||
pub strategy: GithubIntegrationStrategy,
|
||||
pub app_id: Option<InterpString>,
|
||||
pub client_id: Option<InterpString>,
|
||||
pub slug: Option<InterpString>,
|
||||
pub webhooks: Option<IntegrationWebhooksSettings>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)]
|
||||
|
|
|
|||
|
|
@ -231,6 +231,8 @@ models/run-goal-file.ts
|
|||
models/run-goal-inline.ts
|
||||
models/run-goal.ts
|
||||
models/run-interviews-settings.ts
|
||||
models/run-integrations-github-settings.ts
|
||||
models/run-integrations-settings.ts
|
||||
models/run-list-item.ts
|
||||
models/run-manifest.ts
|
||||
models/run-mode.ts
|
||||
|
|
|
|||
|
|
@ -26,7 +26,6 @@ export interface GithubIntegrationSettings {
|
|||
'app_id': string | null;
|
||||
'client_id': string | null;
|
||||
'slug': string | null;
|
||||
'permissions': { [key: string]: string; };
|
||||
'webhooks': IntegrationWebhooksSettings | null;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -210,6 +210,8 @@ export * from './run-goal';
|
|||
export * from './run-goal-file';
|
||||
export * from './run-goal-inline';
|
||||
export * from './run-interviews-settings';
|
||||
export * from './run-integrations-github-settings';
|
||||
export * from './run-integrations-settings';
|
||||
export * from './run-list-item';
|
||||
export * from './run-manifest';
|
||||
export * from './run-mode';
|
||||
|
|
|
|||
|
|
@ -0,0 +1,22 @@
|
|||
/* tslint:disable */
|
||||
/* eslint-disable */
|
||||
/**
|
||||
* Fabro Run API
|
||||
* HTTP API for managing Fabro workflow run executions.
|
||||
*
|
||||
* The version of the OpenAPI document: 0.1.0
|
||||
*
|
||||
*
|
||||
* NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech).
|
||||
* https://openapi-generator.tech
|
||||
* Do not edit the class manually.
|
||||
*/
|
||||
|
||||
|
||||
|
||||
export interface RunIntegrationsGithubSettings {
|
||||
'permissions': { [key: string]: string; };
|
||||
}
|
||||
|
||||
|
||||
|
||||
|
|
@ -0,0 +1,25 @@
|
|||
/* tslint:disable */
|
||||
/* eslint-disable */
|
||||
/**
|
||||
* Fabro Run API
|
||||
* HTTP API for managing Fabro workflow run executions.
|
||||
*
|
||||
* The version of the OpenAPI document: 0.1.0
|
||||
*
|
||||
*
|
||||
* NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech).
|
||||
* https://openapi-generator.tech
|
||||
* Do not edit the class manually.
|
||||
*/
|
||||
|
||||
|
||||
// May contain unused imports in some cases
|
||||
// @ts-ignore
|
||||
import type { RunIntegrationsGithubSettings } from './run-integrations-github-settings';
|
||||
|
||||
export interface RunIntegrationsSettings {
|
||||
'github': RunIntegrationsGithubSettings;
|
||||
}
|
||||
|
||||
|
||||
|
||||
|
|
@ -42,6 +42,9 @@ import type { RunGitSettings } from './run-git-settings';
|
|||
import type { RunGoal } from './run-goal';
|
||||
// May contain unused imports in some cases
|
||||
// @ts-ignore
|
||||
import type { RunIntegrationsSettings } from './run-integrations-settings';
|
||||
// May contain unused imports in some cases
|
||||
// @ts-ignore
|
||||
import type { RunInterviewsSettings } from './run-interviews-settings';
|
||||
// May contain unused imports in some cases
|
||||
// @ts-ignore
|
||||
|
|
@ -77,5 +80,6 @@ export interface RunNamespace {
|
|||
'scm': RunScmSettings;
|
||||
'pull_request': PullRequestSettings | null;
|
||||
'artifacts': ArtifactsSettings;
|
||||
'integrations': RunIntegrationsSettings;
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue