mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-09-06 08:18:58 +00:00
Add script_absolute_cd lint rule to warn on absolute cd paths in scripts
Absolute `cd` paths in shell commands (script/tool_command attributes) silently override the engine's worktree CWD, breaking portability across machines, containers, and worktrees. Ported from kilroy (danshapiro/kilroy d9c1fec). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
24cff30518
commit
4c75f2b67f
1 changed files with 141 additions and 1 deletions
|
|
@ -5,7 +5,7 @@ use crate::graph::{AttrValue, Graph};
|
|||
|
||||
use super::{Diagnostic, LintRule, Severity};
|
||||
|
||||
/// Returns all 18 built-in lint rules.
|
||||
/// Returns all 19 built-in lint rules.
|
||||
#[must_use]
|
||||
pub fn built_in_rules() -> Vec<Box<dyn LintRule>> {
|
||||
vec![
|
||||
|
|
@ -27,6 +27,7 @@ pub fn built_in_rules() -> Vec<Box<dyn LintRule>> {
|
|||
Box::new(ReservedKeywordNodeIdRule),
|
||||
Box::new(AllConditionalEdgesRule),
|
||||
Box::new(OrphanCustomOutcomeRule),
|
||||
Box::new(ScriptAbsoluteCdRule),
|
||||
]
|
||||
}
|
||||
|
||||
|
|
@ -834,6 +835,69 @@ impl LintRule for OrphanCustomOutcomeRule {
|
|||
}
|
||||
}
|
||||
|
||||
// --- Rule 19: script_absolute_cd (WARNING) ---
|
||||
|
||||
struct ScriptAbsoluteCdRule;
|
||||
|
||||
/// Returns true if `text` contains `cd` followed by whitespace and then `/`.
|
||||
fn contains_cd_absolute(text: &str) -> bool {
|
||||
let bytes = text.as_bytes();
|
||||
let len = bytes.len();
|
||||
let mut i = 0;
|
||||
while i + 3 < len {
|
||||
if bytes[i] == b'c' && bytes[i + 1] == b'd' && bytes[i + 2].is_ascii_whitespace() {
|
||||
// found "cd<ws>", scan past remaining whitespace to check for '/'
|
||||
let mut j = i + 2;
|
||||
while j < len && bytes[j].is_ascii_whitespace() {
|
||||
j += 1;
|
||||
}
|
||||
if j < len && bytes[j] == b'/' {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
i += 1;
|
||||
}
|
||||
false
|
||||
}
|
||||
|
||||
impl LintRule for ScriptAbsoluteCdRule {
|
||||
fn name(&self) -> &'static str {
|
||||
"script_absolute_cd"
|
||||
}
|
||||
|
||||
fn apply(&self, graph: &Graph) -> Vec<Diagnostic> {
|
||||
let mut diagnostics = Vec::new();
|
||||
for node in graph.nodes.values() {
|
||||
if node.handler_type() != Some("script") {
|
||||
continue;
|
||||
}
|
||||
let script = node
|
||||
.attrs
|
||||
.get("script")
|
||||
.or_else(|| node.attrs.get("tool_command"))
|
||||
.and_then(|v| v.as_str())
|
||||
.unwrap_or("");
|
||||
if contains_cd_absolute(script) {
|
||||
diagnostics.push(Diagnostic {
|
||||
rule: self.name().to_string(),
|
||||
severity: Severity::Warning,
|
||||
message: format!(
|
||||
"Script node '{}' contains `cd /…` with an absolute path",
|
||||
node.id
|
||||
),
|
||||
node_id: Some(node.id.clone()),
|
||||
edge: None,
|
||||
fix: Some(
|
||||
"Use a relative path; the engine sets the working directory to the worktree automatically"
|
||||
.to_string(),
|
||||
),
|
||||
});
|
||||
}
|
||||
}
|
||||
diagnostics
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
|
@ -2505,4 +2569,80 @@ mod tests {
|
|||
let d = rule.apply(&g);
|
||||
assert!(d.is_empty());
|
||||
}
|
||||
|
||||
// script_absolute_cd rule tests
|
||||
|
||||
#[test]
|
||||
fn script_absolute_cd_warns_on_cd_abs_path() {
|
||||
let mut g = minimal_graph();
|
||||
let mut node = Node::new("run");
|
||||
node.attrs.insert(
|
||||
"shape".to_string(),
|
||||
AttrValue::String("parallelogram".to_string()),
|
||||
);
|
||||
node.attrs.insert(
|
||||
"script".to_string(),
|
||||
AttrValue::String("cd /tmp && ls".to_string()),
|
||||
);
|
||||
g.nodes.insert("run".to_string(), node);
|
||||
let rule = ScriptAbsoluteCdRule;
|
||||
let d = rule.apply(&g);
|
||||
assert_eq!(d.len(), 1);
|
||||
assert_eq!(d[0].severity, Severity::Warning);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn script_absolute_cd_no_warning_on_relative() {
|
||||
let mut g = minimal_graph();
|
||||
let mut node = Node::new("run");
|
||||
node.attrs.insert(
|
||||
"shape".to_string(),
|
||||
AttrValue::String("parallelogram".to_string()),
|
||||
);
|
||||
node.attrs.insert(
|
||||
"script".to_string(),
|
||||
AttrValue::String("cd src && ls".to_string()),
|
||||
);
|
||||
g.nodes.insert("run".to_string(), node);
|
||||
let rule = ScriptAbsoluteCdRule;
|
||||
let d = rule.apply(&g);
|
||||
assert!(d.is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn script_absolute_cd_warns_on_legacy_tool_command() {
|
||||
let mut g = minimal_graph();
|
||||
let mut node = Node::new("run");
|
||||
node.attrs.insert(
|
||||
"shape".to_string(),
|
||||
AttrValue::String("parallelogram".to_string()),
|
||||
);
|
||||
node.attrs.insert(
|
||||
"tool_command".to_string(),
|
||||
AttrValue::String("cd /home/user && make".to_string()),
|
||||
);
|
||||
g.nodes.insert("run".to_string(), node);
|
||||
let rule = ScriptAbsoluteCdRule;
|
||||
let d = rule.apply(&g);
|
||||
assert_eq!(d.len(), 1);
|
||||
assert_eq!(d[0].severity, Severity::Warning);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn script_absolute_cd_skips_non_script_nodes() {
|
||||
let mut g = minimal_graph();
|
||||
let mut node = Node::new("gen");
|
||||
node.attrs.insert(
|
||||
"shape".to_string(),
|
||||
AttrValue::String("box".to_string()),
|
||||
);
|
||||
node.attrs.insert(
|
||||
"prompt".to_string(),
|
||||
AttrValue::String("cd /tmp and do stuff".to_string()),
|
||||
);
|
||||
g.nodes.insert("gen".to_string(), node);
|
||||
let rule = ScriptAbsoluteCdRule;
|
||||
let d = rule.apply(&g);
|
||||
assert!(d.is_empty());
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue