scriptoriumadmin/fabro
1
0
Fork 0
mirror of https://github.com/fabro-sh/fabro.git synced 2026-09-07 08:27:12 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

fix(sandbox): make git push failures log-safe

Browse source 
Add structured exec errors whose Display output keeps raw command output out of logs and notices while preserving stdout/stderr through explicit accessors. Stop Daytona from logging raw command strings and propagate git_push_ref errors so metadata push warnings include safe failure detail.
This commit is contained in:
Bryan Helmkamp 2026-04-29 12:40:50 -04:00
parent 890fbb8fa4
commit 067fa3ee82
No known key found for this signature in database
12 changed files with 654 additions and 204 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

86
lib/crates/fabro-sandbox/src/daytona/mod.rs
View file

@ -13,6 +13,7 @@ use tokio::{fs, time};
use tokio_util::sync::CancellationToken;
use crate::clone_source::{self, CloneDecision, EmptyWorkspaceReason};
use crate::redact::{classify_credential_refresh_failure, redact_auth_url};
use crate::sandbox::resolve_path;
use crate::{
DirEntry, ExecResult, GrepOptions, Sandbox, SandboxEvent, SandboxEventCallback,
@ -54,6 +55,23 @@ fn elapsed_ms(start: &Instant) -> u64 {
u64::try_from(start.elapsed().as_millis()).unwrap_or(u64::MAX)
}
fn command_kind(command: &str) -> &'static str {
match command.split_whitespace().next().unwrap_or_default() {
"git" => "git",
"sh" | "/bin/sh" => "sh",
"bash" | "/bin/bash" => "bash",
"rg" => "rg",
"grep" => "grep",
"find" => "find",
"cat" => "cat",
"ls" => "ls",
"mkdir" => "mkdir",
"rm" => "rm",
"printf" => "printf",
_ => "other",
}
}
/// Sandbox that runs all operations inside a Daytona cloud sandbox.
pub struct DaytonaSandbox {
config: DaytonaConfig,
@ -639,22 +657,25 @@ impl Sandbox for DaytonaSandbox {
let wrapped = wrap_bash_command(&cmd);
match ps.execute_command(&wrapped, opts).await {
Ok(r) if r.exit_code != 0 => {
let stderr = r.result.replace(
&auth_url.raw_string(),
&auth_url.redacted_string(),
let err = crate::Error::exec(
"git remote set-url origin (Daytona post-clone)",
r.exit_code,
false,
0,
redact_auth_url(&r.result, Some(&auth_url)),
String::new(),
);
tracing::warn!(
exit_code = r.exit_code,
output = %stderr.trim(),
error = %err,
"Failed to set Daytona sandbox push credentials \
on origin — subsequent git push from this \
sandbox will fail"
);
}
Ok(_) => {}
Err(e) => {
Err(_) => {
tracing::warn!(
error = %e,
error_class = "daytona_set_url_exec_failed",
"Daytona exec failed while setting push credentials \
on origin — subsequent git push from this \
sandbox will fail"
@ -788,9 +809,9 @@ impl Sandbox for DaytonaSandbox {
)]
}
async fn git_push_ref(&self, refspec: &str) -> bool {
async fn git_push_ref(&self, refspec: &str) -> crate::Result<()> {
if !self.repo_cloned() {
return false;
return Ok(());
}
crate::git_push_via_exec(self, refspec).await
}
@ -857,7 +878,10 @@ impl Sandbox for DaytonaSandbox {
origin_url,
)
.await
.map_err(|e| crate::Error::message(format!("Failed to refresh GitHub App token: {e}")))?;
.map_err(|e| {
let class = classify_credential_refresh_failure(&format!("token_mint_failed: {e}"));
crate::Error::message(format!("Failed to refresh push credentials: {class}"))
})?;
let cmd = format!(
"git -c maintenance.auto=0 remote set-url origin {}",
@ -866,15 +890,16 @@ impl Sandbox for DaytonaSandbox {
let result = self
.exec_command(&cmd, 10_000, None, None, None)
.await
.map_err(|e| crate::Error::context("Failed to set refreshed push credentials", e))?;
.map_err(|e| {
let class =
classify_credential_refresh_failure(&format!("set_url_exec_failed: {e}"));
crate::Error::message(format!("Failed to refresh push credentials: {class}"))
})?;
if result.exit_code != 0 {
let stderr = result
.stderr
.replace(&auth_url.raw_string(), &auth_url.redacted_string());
return Err(crate::Error::message(format!(
"Failed to set refreshed push credentials (exit {}): {}",
result.exit_code, stderr
)));
return Err(result.into_exec_error_with_redactor(
"git remote set-url origin (refresh push credentials)",
|s| redact_auth_url(s, Some(&auth_url)),
));
}
Ok(())
@ -1021,7 +1046,12 @@ impl Sandbox for DaytonaSandbox {
env_vars: Option<&HashMap<String, String>>,
cancel_token: Option<CancellationToken>,
) -> crate::Result<ExecResult> {
tracing::info!(command, timeout_ms, "exec_command: entered");
tracing::info!(
timeout_ms,
command_kind = command_kind(command),
command_len = command.len(),
"exec_command: entered"
);
let sandbox = self.sandbox()?;
let start = Instant::now();
@ -1248,6 +1278,24 @@ mod tests {
assert!(config.labels.is_none());
}
#[test]
fn command_kind_classifies_known_prefixes() {
assert_eq!(command_kind(" git status"), "git");
assert_eq!(command_kind("bash -lc 'echo ok'"), "bash");
assert_eq!(command_kind("/bin/sh -c 'echo ok'"), "sh");
assert_eq!(command_kind("rg --version"), "rg");
assert_eq!(command_kind("find . -maxdepth 1"), "find");
assert_eq!(command_kind(""), "other");
}
#[test]
fn command_kind_does_not_echo_auth_url_commands() {
assert_eq!(
command_kind("https://x-access-token:ghs_FAKE@github.com/owner/repo.git"),
"other"
);
}
#[test]
fn wrap_bash_uses_base64_encoding() {
let wrapped = wrap_bash_command("echo hello");

30
lib/crates/fabro-sandbox/src/docker.rs
View file

@ -22,6 +22,7 @@ use tokio::{fs, time};
use tokio_util::sync::CancellationToken;
use crate::clone_source::{self, CloneDecision, EmptyWorkspaceReason};
use crate::redact::{classify_credential_refresh_failure, redact_auth_url};
use crate::sandbox::resolve_path;
use crate::{
DirEntry, ExecResult, GrepOptions, Sandbox, SandboxEvent, SandboxEventCallback,
@ -423,10 +424,12 @@ impl DockerSandbox {
.docker_exec_shell(&command, 10_000, Some(WORKING_DIRECTORY), None, None)
.await?;
if result.exit_code != 0 {
let stderr = redact_auth_url(&result.stderr, Some(auth_url));
let err = result
.into_exec_error_with_redactor("git remote set-url origin (post-clone)", |s| {
redact_auth_url(s, Some(auth_url))
});
tracing::warn!(
exit_code = result.exit_code,
stderr = %stderr.trim(),
error = %err,
"Failed to set Docker sandbox push credentials on origin — \
subsequent git push from this sandbox will fail"
);
@ -634,13 +637,6 @@ fn bash_remediation(error: &DockerError, image: &str) -> String {
)
}
fn redact_auth_url(text: &str, auth_url: Option<&fabro_redact::DisplaySafeUrl>) -> String {
let Some(auth_url) = auth_url else {
return text.to_string();
};
text.replace(&auth_url.raw_string(), &auth_url.redacted_string())
}
fn build_single_file_tar(file_name: &str, bytes: &[u8]) -> crate::Result<Vec<u8>> {
let mut tar_builder = tar::Builder::new(Vec::new());
let mut header = tar::Header::new_gnu();
@ -1208,9 +1204,9 @@ impl Sandbox for DockerSandbox {
)]
}
async fn git_push_ref(&self, refspec: &str) -> bool {
async fn git_push_ref(&self, refspec: &str) -> crate::Result<()> {
if !self.repo_cloned() {
return false;
return Ok(());
}
crate::git_push_via_exec(self, refspec).await
}
@ -1254,7 +1250,10 @@ impl Sandbox for DockerSandbox {
origin_url,
)
.await
.map_err(|e| crate::Error::message(format!("Failed to refresh GitHub App token: {e}")))?;
.map_err(|e| {
let class = classify_credential_refresh_failure(&format!("token_mint_failed: {e}"));
crate::Error::message(format!("Failed to refresh push credentials: {class}"))
})?;
let command = format!(
"git -c maintenance.auto=0 remote set-url origin {}",
@ -1264,10 +1263,9 @@ impl Sandbox for DockerSandbox {
.docker_exec_shell(&command, 10_000, Some(WORKING_DIRECTORY), None, None)
.await?;
if result.exit_code != 0 {
let stderr = redact_auth_url(&result.stderr, Some(&auth_url));
let class = classify_credential_refresh_failure("set_url_nonzero");
return Err(crate::Error::message(format!(
"Failed to set refreshed push credentials (exit {}): {}",
result.exit_code, stderr
"Failed to refresh push credentials: {class}"
)));
}

216
lib/crates/fabro-sandbox/src/error.rs
View file

@ -36,6 +36,21 @@ pub enum Error {
#[source]
source: BollardError,
},
#[error(
"{label} failed (exit {exit_code}, timed_out={timed_out}, duration_ms={duration_ms}) - hint: {hint}",
hint = classify_exec_failure(stderr)
.or_else(|| classify_exec_failure(stdout))
.unwrap_or("unclassified")
)]
Exec {
label: String,
exit_code: i32,
timed_out: bool,
duration_ms: u64,
stderr: String,
stdout: String,
},
}
impl Error {
@ -53,6 +68,66 @@ impl Error {
}
}
pub fn exec(
label: impl Into<String>,
exit_code: i32,
timed_out: bool,
duration_ms: u64,
stderr: impl Into<String>,
stdout: impl Into<String>,
) -> Self {
Self::Exec {
label: label.into(),
exit_code,
timed_out,
duration_ms,
stderr: stderr.into(),
stdout: stdout.into(),
}
}
pub fn exec_stderr(&self) -> Option<&str> {
match self {
Self::Exec { stderr, .. } => Some(stderr),
_ => None,
}
}
pub fn exec_stdout(&self) -> Option<&str> {
match self {
Self::Exec { stdout, .. } => Some(stdout),
_ => None,
}
}
pub fn exec_label(&self) -> Option<&str> {
match self {
Self::Exec { label, .. } => Some(label),
_ => None,
}
}
pub fn exec_exit_code(&self) -> Option<i32> {
match self {
Self::Exec { exit_code, .. } => Some(*exit_code),
_ => None,
}
}
pub fn exec_timed_out(&self) -> Option<bool> {
match self {
Self::Exec { timed_out, .. } => Some(*timed_out),
_ => None,
}
}
pub fn exec_duration_ms(&self) -> Option<u64> {
match self {
Self::Exec { duration_ms, .. } => Some(*duration_ms),
_ => None,
}
}
#[cfg(feature = "docker")]
pub fn docker_connect(source: BollardError) -> Self {
Self::DockerConnect { source }
@ -95,4 +170,145 @@ impl From<&str> for Error {
}
}
pub(crate) fn classify_exec_failure(stderr: &str) -> Option<&'static str> {
let lower = stderr.to_ascii_lowercase();
if lower.contains("could not read username") || lower.contains("terminal prompts disabled") {
Some(
"no credentials in origin URL - check that the sandbox forwarded \
GITHUB_APP_PRIVATE_KEY (or GITHUB_TOKEN) and that refresh_push_credentials succeeded",
)
} else if lower.contains("permission to") && lower.contains("denied") {
Some(
"github denied the push - installation token lacks contents:write \
on this repo, or a branch protection / push ruleset is rejecting the ref",
)
} else if lower.contains("protected branch")
|| lower.contains("ruleset")
|| lower.contains("rejected")
{
Some("github rejected the ref - likely a branch protection rule or push ruleset")
} else if lower.contains("authentication failed") || lower.contains("invalid username") {
Some("github authentication failed - installation token may be expired or wrong scope")
} else if lower.contains("could not resolve host") || lower.contains("network is unreachable") {
Some("network failure inside sandbox - check DNS / egress from the run container")
} else if lower.contains("repository not found") {
Some("github 404 - the App installation may not include this repo")
} else if lower.contains("no such remote") && lower.contains("origin") {
Some("origin remote missing - push credentials could not be installed")
} else if lower.contains("not a git repository")
|| lower.contains("does not appear to be a git repository")
{
Some("git repository unavailable in sandbox working directory")
} else {
None
}
}
pub type Result<T> = std::result::Result<T, Error>;
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn exec_display_is_log_safe() {
let stderr = "fatal: unable to access \
'https://x-access-token:ghs_xK9mZ2vL8nQ5rT1wY4bC7dF0gH3jE6pA@github.com/owner/repo/':\n\
remote: Permission to owner/repo.git denied\n\
identity ~/.ssh/id_rsa_work";
let error = Error::exec(
"git push origin refs/heads/run",
128,
false,
210,
stderr,
"",
);
let rendered = error.to_string();
for forbidden in [
"fatal:",
"remote:",
"x-access-token",
"ghs_xK9mZ2vL8nQ5rT1wY4bC7dF0gH3jE6pA",
"~/.ssh",
"id_rsa_work",
] {
assert!(
!rendered.contains(forbidden),
"Display leaked {forbidden:?}: {rendered}"
);
}
assert!(rendered.contains("git push origin refs/heads/run"));
assert!(rendered.contains("exit 128"));
assert!(rendered.contains("timed_out=false"));
assert!(rendered.contains("duration_ms=210"));
assert!(rendered.contains("hint:"));
}
#[test]
fn exec_accessors_return_stored_values() {
let error = Error::exec("git push", 1, true, 5000, "stored stderr", "stored stdout");
assert_eq!(error.exec_label(), Some("git push"));
assert_eq!(error.exec_exit_code(), Some(1));
assert_eq!(error.exec_timed_out(), Some(true));
assert_eq!(error.exec_duration_ms(), Some(5000));
assert_eq!(error.exec_stderr(), Some("stored stderr"));
assert_eq!(error.exec_stdout(), Some("stored stdout"));
}
#[test]
fn non_exec_accessors_return_none() {
let message = Error::message("plain");
assert_eq!(message.exec_label(), None);
assert_eq!(message.exec_exit_code(), None);
assert_eq!(message.exec_timed_out(), None);
assert_eq!(message.exec_duration_ms(), None);
assert_eq!(message.exec_stderr(), None);
assert_eq!(message.exec_stdout(), None);
let source = std::io::Error::other("source");
let context = Error::context("context", source);
assert_eq!(context.exec_label(), None);
assert_eq!(context.exec_stderr(), None);
}
#[test]
fn classify_exec_failure_documents_known_branches() {
let cases = [
(
"fatal: could not read Username for 'https://github.com'",
"no credentials in origin URL",
),
(
"remote: Permission to owner/repo.git denied to fabro-app[bot].",
"github denied the push",
),
(
"remote: error: GH013: Repository rule violations found due to ruleset",
"github rejected the ref",
),
(
"fatal: Authentication failed for 'https://github.com/owner/repo'",
"github authentication failed",
),
(
"fatal: could not resolve host: github.com",
"network failure",
),
("remote: Repository not found.", "github 404"),
("error: No such remote 'origin'", "origin remote missing"),
("fatal: not a git repository", "git repository unavailable"),
];
for (stderr, expected) in cases {
let hint = classify_exec_failure(stderr).expect(stderr);
assert!(
hint.contains(expected),
"expected {hint:?} to contain {expected:?}"
);
}
assert_eq!(classify_exec_failure("weird new git error"), None);
}
}

3
lib/crates/fabro-sandbox/src/lib.rs
View file

@ -8,6 +8,9 @@ mod clone_source;
pub mod read_guard;
#[cfg(any(feature = "docker", feature = "daytona", test))]
pub(crate) mod redact;
pub mod reconnect;
pub mod sandbox_provider;

17
lib/crates/fabro-sandbox/src/local.rs
View file

@ -491,14 +491,17 @@ impl Sandbox for LocalSandbox {
result
}
async fn git_push_ref(&self, refspec: &str) -> bool {
let has_origin = matches!(
self.exec_command("git remote get-url origin", 10_000, None, None, None)
.await,
Ok(result) if result.exit_code == 0
);
async fn git_push_ref(&self, refspec: &str) -> crate::Result<()> {
let has_origin = match self
.exec_command("git remote get-url origin", 10_000, None, None, None)
.await
{
Ok(result) if result.exit_code == 0 => true,
Ok(_) => false,
Err(err) => return Err(crate::Error::context("git remote get-url origin", err)),
};
if !has_origin {
return true;
return Ok(());
}
crate::git_push_via_exec(self, refspec).await

59
lib/crates/fabro-sandbox/src/redact.rs Normal file
View file

@ -0,0 +1,59 @@
pub(crate) fn redact_auth_url(
text: &str,
auth_url: Option<&fabro_redact::DisplaySafeUrl>,
) -> String {
let Some(auth_url) = auth_url else {
return text.to_string();
};
text.replace(&auth_url.raw_string(), &auth_url.redacted_string())
}
pub(crate) fn classify_credential_refresh_failure(inner: &str) -> &'static str {
let lower = inner.to_ascii_lowercase();
if lower.contains("set_url_exec_failed")
|| lower.contains("execute command")
|| lower.contains("failed to execute")
{
"set_url_exec_failed"
} else if lower.contains("set_url_nonzero")
|| lower.contains("remote set-url")
|| lower.contains("set refreshed push credentials")
|| lower.contains("exit ")
{
"set_url_nonzero"
} else if lower.contains("token_mint_failed")
|| lower.contains("github app")
|| lower.contains("installation")
|| lower.contains("private key")
|| lower.contains("token")
{
"token_mint_failed"
} else {
"unclassified"
}
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn classify_credential_refresh_failure_documents_known_branches() {
assert_eq!(
classify_credential_refresh_failure("GitHub App installation token request failed"),
"token_mint_failed"
);
assert_eq!(
classify_credential_refresh_failure("set_url_exec_failed: sdk echoed command argv"),
"set_url_exec_failed"
);
assert_eq!(
classify_credential_refresh_failure("git remote set-url origin failed with exit 128"),
"set_url_nonzero"
);
assert_eq!(
classify_credential_refresh_failure("new provider error"),
"unclassified"
);
}
}

343
lib/crates/fabro-sandbox/src/sandbox.rs
View file

@ -145,7 +145,7 @@ macro_rules! delegate_sandbox {
self.$field.resume_setup_commands(run_branch)
}
async fn git_push_ref(&self, refspec: &str) -> bool {
async fn git_push_ref(&self, refspec: &str) -> $crate::Result<()> {
self.$field.git_push_ref(refspec).await
}
@ -380,6 +380,48 @@ pub struct ExecResult {
pub duration_ms: u64,
}
impl ExecResult {
pub fn is_success(&self) -> bool {
self.exit_code == 0 && !self.timed_out
}
pub fn into_exec_error(self, label: impl Into<String>) -> crate::Error {
crate::Error::exec(
label,
self.exit_code,
self.timed_out,
self.duration_ms,
self.stderr,
self.stdout,
)
}
pub fn into_exec_error_with_redactor(
self,
label: impl Into<String>,
redactor: impl Fn(&str) -> String,
) -> crate::Error {
let stderr = redactor(&self.stderr);
let stdout = redactor(&self.stdout);
crate::Error::exec(
label,
self.exit_code,
self.timed_out,
self.duration_ms,
stderr,
stdout,
)
}
pub fn into_result(self, label: impl Into<String>) -> crate::Result<Self> {
if self.is_success() {
Ok(self)
} else {
Err(self.into_exec_error(label))
}
}
}
#[derive(Debug, Clone)]
pub struct DirEntry {
pub name: String,
@ -478,8 +520,10 @@ pub trait Sandbox: Send + Sync {
}
/// Push a full refspec to origin from inside the sandbox.
async fn git_push_ref(&self, _refspec: &str) -> bool {
false
async fn git_push_ref(&self, _refspec: &str) -> crate::Result<()> {
Err(crate::Error::message(
"git_push_ref not implemented for this sandbox",
))
}
/// Compute the filesystem path for a parallel branch worktree.
@ -577,13 +621,8 @@ pub async fn setup_git_via_exec(
let sha_result = sandbox
.exec_command("git rev-parse HEAD", 10_000, None, None, None)
.await
.map_err(|e| crate::Error::message(format!("git rev-parse HEAD failed: {e}")))?;
if sha_result.exit_code != 0 {
return Err(crate::Error::message(format!(
"git rev-parse HEAD failed (exit {}): {}",
sha_result.exit_code, sha_result.stderr
)));
}
.map_err(|e| crate::Error::context("git rev-parse HEAD", e))?
.into_result("git rev-parse HEAD")?;
(
sha_result.stdout.trim().to_string(),
format!("fabro/run/{run_id}"),
@ -604,16 +643,11 @@ pub async fn setup_git_via_exec(
shell_quote(&branch_name),
shell_quote(&base_sha)
);
let checkout_result = sandbox
sandbox
.exec_command(&checkout_cmd, 10_000, None, None, None)
.await
.map_err(|e| crate::Error::message(format!("git checkout failed: {e}")))?;
if checkout_result.exit_code != 0 {
return Err(crate::Error::message(format!(
"git checkout -B failed (exit {}): {}",
checkout_result.exit_code, checkout_result.stderr
)));
}
.map_err(|e| crate::Error::context("git checkout -B", e))?
.into_result("git checkout -B")?;
Ok(GitRunInfo {
base_sha,
@ -646,11 +680,9 @@ pub(crate) async fn fetch_source_run_ref(
.exec_command(&fetch_cmd, 30_000, None, None, None)
.await?;
if fetch.exit_code != 0 {
last_error = format!(
"git fetch source run ref failed (exit {}): {}",
fetch.exit_code,
fetch.stderr.trim()
);
last_error = fetch
.into_exec_error("git fetch source run ref")
.to_string();
} else {
let check = sandbox
.exec_command(&check_cmd, 10_000, None, None, None)
@ -658,11 +690,11 @@ pub(crate) async fn fetch_source_run_ref(
if check.exit_code == 0 {
return Ok(());
}
last_error = format!(
"checkpoint {checkpoint_sha} is not reachable from {remote_ref} (exit {}): {}",
check.exit_code,
check.stderr.trim()
);
last_error = check
.into_exec_error(format!(
"checkpoint {checkpoint_sha} is not reachable from {remote_ref}"
))
.to_string();
}
time::sleep(Duration::from_millis(500)).await;
}
@ -672,93 +704,23 @@ pub(crate) async fn fetch_source_run_ref(
/// Helper for sandbox implementations that manage git internally.
/// Pushes a refspec to origin via exec_command inside the sandbox.
pub async fn git_push_via_exec(sandbox: &dyn Sandbox, refspec: &str) -> bool {
pub async fn git_push_via_exec(sandbox: &dyn Sandbox, refspec: &str) -> crate::Result<()> {
if let Err(e) = sandbox.refresh_push_credentials().await {
tracing::warn!(
refspec,
error = %fabro_redact::redact_string(&e.to_string()),
refspec = %refspec,
error = %e,
"Failed to refresh push credentials before git push"
);
}
let cmd = format!("{GIT} push origin {}", shell_quote(refspec));
match sandbox.exec_command(&cmd, 60_000, None, None, None).await {
Ok(r) if r.exit_code == 0 => {
tracing::info!(refspec, "Pushed git ref to origin");
true
}
Ok(r) => {
tracing::warn!(
refspec,
exit_code = r.exit_code,
timed_out = r.timed_out,
stderr = %trim_for_log(&r.stderr, GIT_LOG_TAIL_BYTES),
stdout = %trim_for_log(&r.stdout, GIT_LOG_TAIL_BYTES),
hint = classify_git_push_failure(&r.stderr).unwrap_or(""),
"Failed to push git ref"
);
false
}
Err(e) => {
tracing::warn!(
refspec,
error = %fabro_redact::redact_string(&e.to_string()),
"Failed to invoke git push in sandbox"
);
false
}
}
}
/// Maximum bytes of git stdout/stderr to include in a single log line.
/// Long enough to capture the typical 1-3 line `fatal:` / `remote:` output
/// without flooding the log when git emits a large progress dump.
const GIT_LOG_TAIL_BYTES: usize = 2048;
/// Redact secrets from `text`, then keep at most the trailing `limit` bytes.
/// Trailing because git's relevant `fatal:` / `remote: rejected` lines are
/// emitted at the end of the output.
fn trim_for_log(text: &str, limit: usize) -> String {
let redacted = fabro_redact::redact_string(text);
let trimmed = redacted.trim_end();
if trimmed.len() <= limit {
return trimmed.to_string();
}
let start = trimmed.len() - limit;
let safe_start = (start..=trimmed.len())
.find(|i| trimmed.is_char_boundary(*i))
.unwrap_or(trimmed.len());
format!("…{}", &trimmed[safe_start..])
}
/// Map a git stderr to a short hint pointing at the likely cause. Returns
/// `None` when no known pattern matches; callers should still log the raw
/// (redacted) stderr so unknown failures stay debuggable.
fn classify_git_push_failure(stderr: &str) -> Option<&'static str> {
let lower = stderr.to_ascii_lowercase();
if lower.contains("could not read username") || lower.contains("terminal prompts disabled") {
Some(
"no credentials in origin URL — check that the sandbox forwarded \
GITHUB_APP_PRIVATE_KEY (or GITHUB_TOKEN) and that refresh_push_credentials succeeded",
)
} else if lower.contains("permission to") && lower.contains("denied") {
Some(
"github denied the push — installation token lacks contents:write \
on this repo, or a branch protection / push ruleset is rejecting the ref",
)
} else if lower.contains("protected branch")
|| lower.contains("ruleset")
|| lower.contains("rejected")
{
Some("github rejected the ref — likely a branch protection rule or push ruleset")
} else if lower.contains("authentication failed") || lower.contains("invalid username") {
Some("github authentication failed — installation token may be expired or wrong scope")
} else if lower.contains("could not resolve host") || lower.contains("network is unreachable") {
Some("network failure inside sandbox — check DNS / egress from the run container")
} else if lower.contains("repository not found") {
Some("github 404 — the App installation may not include this repo")
} else {
None
}
let label = format!("git push origin {refspec}");
sandbox
.exec_command(&cmd, 60_000, None, None, None)
.await
.map_err(|e| crate::Error::context(label.clone(), e))?
.into_result(&label)?;
tracing::info!(refspec = %refspec, "Pushed git ref to origin");
Ok(())
}
#[cfg(test)]
@ -780,57 +742,65 @@ mod tests {
}
#[test]
fn trim_for_log_keeps_short_output_intact() {
assert_eq!(trim_for_log("fatal: nope\n", 2048), "fatal: nope");
fn exec_result_helpers_convert_failure_to_exec_error() {
let result = ExecResult {
stdout: "out".into(),
stderr: "fatal: could not read Username".into(),
exit_code: 128,
timed_out: false,
duration_ms: 42,
};
let error = result.into_result("git push").unwrap_err();
assert_eq!(error.exec_label(), Some("git push"));
assert_eq!(error.exec_exit_code(), Some(128));
assert!(error.to_string().contains("no credentials in origin URL"));
}
#[test]
fn trim_for_log_keeps_trailing_bytes_when_oversized() {
let prefix = "x".repeat(3000);
let suffix = "fatal: rejected";
let trimmed = trim_for_log(&format!("{prefix}{suffix}"), 64);
assert!(trimmed.starts_with('…'));
assert!(trimmed.ends_with(suffix));
assert!(trimmed.chars().count() <= 64 + 1);
fn exec_result_success_honors_timeouts() {
let success = ExecResult {
stdout: String::new(),
stderr: String::new(),
exit_code: 0,
timed_out: false,
duration_ms: 1,
};
assert!(success.is_success());
let timeout = ExecResult {
timed_out: true,
..success
};
assert!(!timeout.is_success());
}
#[test]
fn trim_for_log_redacts_high_entropy_secrets() {
let stderr = "fatal: unable to access \
'https://x-access-token:ghs_xK9mZ2vL8nQ5rT1wY4bC7dF0gH3jE6pA@github.com/owner/repo/'";
let trimmed = trim_for_log(stderr, 2048);
assert!(!trimmed.contains("ghs_xK9mZ2vL8nQ5rT1wY4bC7dF0gH3jE6pA"));
assert!(trimmed.contains("REDACTED"));
fn exec_result_redactor_applies_to_stderr_and_stdout() {
let result = ExecResult {
stdout: "stdout https://token@example.com".into(),
stderr: "stderr https://token@example.com".into(),
exit_code: 1,
timed_out: false,
duration_ms: 1,
};
let error = result.into_exec_error_with_redactor("git set-url", |s| {
s.replace("https://token@example.com", "https://****@example.com")
});
assert_eq!(error.exec_stderr(), Some("stderr https://****@example.com"));
assert_eq!(error.exec_stdout(), Some("stdout https://****@example.com"));
}
#[test]
fn classify_git_push_failure_recognises_missing_credentials() {
let hint = classify_git_push_failure(
"fatal: could not read Username for 'https://github.com': No such device or address",
fn sandbox_tracing_events_do_not_log_raw_command_fields() {
let root = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("src");
let mut failures = Vec::new();
scan_for_command_tracing(&root, &mut failures);
assert!(
failures.is_empty(),
"raw command/cmd tracing fields found:\n{}",
failures.join("\n")
);
assert!(hint.unwrap().contains("no credentials in origin URL"));
}
#[test]
fn classify_git_push_failure_recognises_permission_denied() {
let hint = classify_git_push_failure(
"remote: Permission to owner/repo.git denied to fabro-app[bot].",
);
assert!(hint.unwrap().contains("github denied the push"));
}
#[test]
fn classify_git_push_failure_recognises_branch_protection() {
let hint = classify_git_push_failure(
"remote: error: GH013: Repository rule violations found for refs/heads/main\n\
remote: - Cannot create ref 'refs/heads/fabro/run/X' due to ruleset",
);
assert!(hint.unwrap().contains("ruleset"));
}
#[test]
fn classify_git_push_failure_returns_none_for_unknown() {
assert!(classify_git_push_failure("fatal: weird new git error message").is_none());
}
#[test]
@ -960,4 +930,77 @@ mod tests {
assert_eq!(shell_quote("hello"), "hello");
assert_eq!(shell_quote("hello world"), "'hello world'");
}
#[expect(
clippy::disallowed_methods,
reason = "unit test performs a small synchronous source scan of local Rust files"
)]
fn scan_for_command_tracing(path: &std::path::Path, failures: &mut Vec<String>) {
for entry in std::fs::read_dir(path).unwrap() {
let entry = entry.unwrap();
let path = entry.path();
if path.is_dir() {
scan_for_command_tracing(&path, failures);
continue;
}
if path.extension().and_then(|ext| ext.to_str()) != Some("rs") {
continue;
}
let source = std::fs::read_to_string(&path).unwrap();
for macro_name in [
"tracing::trace!",
"tracing::debug!",
"tracing::info!",
"tracing::warn!",
"tracing::error!",
"trace!",
"debug!",
"info!",
"warn!",
"error!",
] {
let mut rest = source.as_str();
while let Some(idx) = rest.find(macro_name) {
let start = source.len() - rest.len() + idx;
if start > 0 && source.as_bytes()[start - 1] == b'"' {
rest = &source[start + macro_name.len()..];
continue;
}
let Some(call) = tracing_call(&source[start..]) else {
break;
};
if call.contains("command,")
|| call.contains("command =")
|| call.contains("cmd,")
|| call.contains("cmd =")
{
failures.push(format!(
"{}: {}",
path.display(),
call.lines().next().unwrap_or(call)
));
}
rest = &source[start + call.len()..];
}
}
}
}
fn tracing_call(source: &str) -> Option<&str> {
let open = source.find('(')?;
let mut depth = 0usize;
for (idx, ch) in source.char_indices().skip(open) {
match ch {
'(' => depth += 1,
')' => {
depth = depth.saturating_sub(1);
if depth == 0 {
return Some(&source[..=idx]);
}
}
_ => {}
}
}
None
}
}

17
lib/crates/fabro-sandbox/src/worktree.rs
View file

@ -344,14 +344,17 @@ impl Sandbox for WorktreeSandbox {
self.inner.resume_setup_commands(run_branch)
}
async fn git_push_ref(&self, refspec: &str) -> bool {
let has_origin = matches!(
self.exec_command("git remote get-url origin", 10_000, None, None, None)
.await,
Ok(result) if result.exit_code == 0
);
async fn git_push_ref(&self, refspec: &str) -> crate::Result<()> {
let has_origin = match self
.exec_command("git remote get-url origin", 10_000, None, None, None)
.await
{
Ok(result) if result.exit_code == 0 => true,
Ok(_) => false,
Err(err) => return Err(crate::Error::context("git remote get-url origin", err)),
};
if !has_origin {
return true;
return Ok(());
}
crate::git_push_via_exec(self, refspec).await

15
lib/crates/fabro-workflow/src/lifecycle/git.rs
View file

@ -175,7 +175,17 @@ impl RunLifecycle<WorkflowGraph> for GitLifecycle {
.and_then(|g| g.run_branch.as_ref())
{
let refspec = format!("refs/heads/{branch}:refs/heads/{branch}");
let push_ok = self.sandbox.git_push_ref(&refspec).await;
let push_ok = match self.sandbox.git_push_ref(&refspec).await {
Ok(()) => true,
Err(err) => {
tracing::warn!(
refspec = %refspec,
error = %err,
"git push from run lifecycle failed"
);
false
}
};
git_result.push_results.push((refspec, push_ok));
}
}
@ -249,9 +259,10 @@ impl GitLifecycle {
match writer.write_snapshot(dump, message).await {
Ok(snapshot) => {
if !snapshot.pushed {
let detail = snapshot.push_error.as_deref().unwrap_or("unknown error");
self.emit_metadata_warning(
"checkpoint_metadata_push_failed",
format!("failed to push metadata ref refs/heads/{meta_branch}"),
format!("failed to push metadata ref refs/heads/{meta_branch}: {detail}"),
);
}
Some(snapshot.commit_sha)

3
lib/crates/fabro-workflow/src/pipeline/finalize.rs
View file

@ -179,10 +179,11 @@ pub async fn write_finalize_commit(
match writer.write_snapshot(&dump, "finalize run").await {
Ok(snapshot) => {
if !snapshot.pushed {
let detail = snapshot.push_error.as_deref().unwrap_or("unknown error");
emit_metadata_warning(
services,
"checkpoint_metadata_push_failed",
format!("failed to push metadata ref refs/heads/{meta_branch}"),
format!("failed to push metadata ref refs/heads/{meta_branch}: {detail}"),
);
}
}

63
lib/crates/fabro-workflow/src/sandbox_git.rs
View file

@ -942,7 +942,7 @@ mod tests {
self.inner.os_version()
}
async fn git_push_ref(&self, refspec: &str) -> bool {
async fn git_push_ref(&self, refspec: &str) -> fabro_sandbox::Result<()> {
self.pushes.lock().unwrap().push(refspec.to_string());
self.inner.git_push_ref(refspec).await
}
@ -1351,6 +1351,8 @@ mod tests {
);
let snapshot = writer.write_snapshot(&dump, "checkpoint").await.unwrap();
assert!(snapshot.pushed);
assert_eq!(snapshot.push_error, None);
let commit_sha = snapshot.commit_sha;
let current = std::process::Command::new("git")
@ -1447,6 +1449,65 @@ mod tests {
assert_eq!(sandbox.pushes(), vec![refspec.clone(), refspec]);
}
#[tokio::test]
async fn sandbox_metadata_writer_records_log_safe_push_error() {
let repo_dir = tempfile::tempdir().unwrap();
let repo = repo_dir.path();
init_git_repo(repo);
std::fs::write(repo.join("tracked.txt"), "seed\n").unwrap();
git_commit_all(repo, "initial");
let missing_origin = repo_dir.path().join("missing-origin.git");
let add_remote = std::process::Command::new("git")
.args(["remote", "add", "origin", missing_origin.to_str().unwrap()])
.current_dir(repo)
.output()
.unwrap();
assert!(add_remote.status.success());
let sandbox = fabro_agent::LocalSandbox::new(repo.to_path_buf());
let run_id = fabro_types::fixtures::RUN_2.to_string();
let branch = crate::sandbox_metadata::metadata_branch_name(&run_id);
let mut projection = fabro_store::RunProjection::default();
projection.spec = Some(fabro_types::RunSpec {
run_id: fabro_types::fixtures::RUN_2,
settings: fabro_types::WorkflowSettings::default(),
graph: fabro_types::Graph::new("metadata"),
workflow_slug: Some("metadata".to_string()),
source_directory: Some("/Users/client/project".to_string()),
git: None,
labels: HashMap::new(),
provenance: None,
manifest_blob: None,
definition_blob: None,
fork_source_ref: None,
in_place: false,
});
let dump = crate::run_dump::RunDump::from_projection(&projection);
let runtime = crate::sandbox_metadata::SandboxGitRuntime::new();
let writer = crate::sandbox_metadata::SandboxMetadataWriter::new(
&sandbox,
&runtime,
&run_id,
&branch,
crate::git::GitAuthor::default(),
);
let snapshot = writer.write_snapshot(&dump, "checkpoint").await.unwrap();
assert!(!snapshot.pushed);
let push_error = snapshot.push_error.unwrap();
assert!(push_error.contains("git push origin"));
assert!(push_error.contains("hint:"));
assert!(
!push_error.contains("fatal:"),
"push error should be log-safe: {push_error}"
);
assert!(
!push_error.contains(missing_origin.to_str().unwrap()),
"push error should not include raw git stderr paths: {push_error}"
);
}
#[tokio::test]
async fn list_changed_files_raw_classifies_add_modify_delete() {
let repo_dir = tempfile::tempdir().unwrap();

6
lib/crates/fabro-workflow/src/sandbox_metadata.rs
View file

@ -78,6 +78,7 @@ pub(crate) struct SandboxMetadataWriter<'a> {
pub(crate) struct MetadataSnapshot {
pub commit_sha: String,
pub pushed: bool,
pub push_error: Option<String>,
}
impl<'a> SandboxMetadataWriter<'a> {
@ -178,10 +179,13 @@ impl<'a> SandboxMetadataWriter<'a> {
.await?;
let commit = parse_fast_import_mark(&stdout)?;
let refspec = format!("{full_ref}:{full_ref}");
let pushed = self.sandbox.git_push_ref(&refspec).await;
let push_result = self.sandbox.git_push_ref(&refspec).await;
let pushed = push_result.is_ok();
let push_error = push_result.err().map(|err| err.to_string());
Ok(MetadataSnapshot {
commit_sha: commit,
pushed,
push_error,
})
}
}