Skip to content

Commit 131a41f

Browse files
author
Test User
committed
feat(pr-reviewer): agent work [auto-commit]
1 parent 577c719 commit 131a41f

3 files changed

Lines changed: 220 additions & 2 deletions

File tree

Cargo.lock

Lines changed: 84 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/terraphim_spawner/src/config.rs

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -798,4 +798,84 @@ mod tests {
798798
assert!(!debug_output.contains("***REDACTED***"));
799799
assert!(debug_output.contains("AgentConfig"));
800800
}
801+
802+
fn minimal_config(cli_command: &str) -> AgentConfig {
803+
AgentConfig {
804+
agent_id: "test-agent".to_string(),
805+
cli_command: cli_command.to_string(),
806+
args: vec![],
807+
working_dir: None,
808+
env_vars: HashMap::new(),
809+
required_api_keys: vec![],
810+
resource_limits: ResourceLimits::default(),
811+
use_stdin: false,
812+
supports_stdin: false,
813+
}
814+
}
815+
816+
#[tokio::test]
817+
async fn test_validate_missing_cli_returns_cli_not_found() {
818+
let config = minimal_config("__terraphim_no_such_cli_xyz_999__");
819+
let validator = AgentValidator::new(&config);
820+
let result = validator.validate().await;
821+
assert!(
822+
matches!(result, Err(ValidationError::CliNotFound(_))),
823+
"expected CliNotFound, got {:?}",
824+
result
825+
);
826+
}
827+
828+
#[tokio::test]
829+
async fn test_validate_missing_api_key_returns_api_key_not_set() {
830+
const KEY: &str = "TERRAPHIM_TEST_MISSING_KEY_SPAWNER_XYZ";
831+
unsafe { std::env::remove_var(KEY) };
832+
833+
let mut config = minimal_config("echo");
834+
config.required_api_keys = vec![KEY.to_string()];
835+
836+
let validator = AgentValidator::new(&config);
837+
let result = validator.validate().await;
838+
assert!(
839+
matches!(result, Err(ValidationError::ApiKeyNotSet(_))),
840+
"expected ApiKeyNotSet, got {:?}",
841+
result
842+
);
843+
}
844+
845+
#[tokio::test]
846+
async fn test_validate_valid_config_succeeds() {
847+
let config = minimal_config("echo");
848+
let validator = AgentValidator::new(&config);
849+
let result = validator.validate().await;
850+
assert!(result.is_ok(), "expected Ok(()), got {:?}", result);
851+
}
852+
853+
#[tokio::test]
854+
async fn test_validate_api_key_present_succeeds() {
855+
const KEY: &str = "TERRAPHIM_TEST_PRESENT_KEY_SPAWNER_XYZ";
856+
unsafe { std::env::set_var(KEY, "dummy-value") };
857+
858+
let mut config = minimal_config("echo");
859+
config.required_api_keys = vec![KEY.to_string()];
860+
861+
let validator = AgentValidator::new(&config);
862+
let result = validator.validate().await;
863+
864+
unsafe { std::env::remove_var(KEY) };
865+
866+
assert!(result.is_ok(), "expected Ok(()), got {:?}", result);
867+
}
868+
869+
#[tokio::test]
870+
async fn test_validate_missing_working_dir_returns_error() {
871+
let mut config = minimal_config("echo");
872+
config.working_dir = Some(std::path::PathBuf::from("/nonexistent/path/xyz_999"));
873+
let validator = AgentValidator::new(&config);
874+
let result = validator.validate().await;
875+
assert!(
876+
matches!(result, Err(ValidationError::WorkingDirNotFound(_))),
877+
"expected WorkingDirNotFound, got {:?}",
878+
result
879+
);
880+
}
801881
}
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
# Spec Validation Report — Issue #2884
2+
3+
**Agent**: Carthos / spec-validator
4+
**Date**: 2026-06-22 09:32 CEST
5+
**Issue**: [#2884 — validate_path uses string starts_with](https://git.terraphim.cloud/terraphim/terraphim-ai/issues/2884)
6+
7+
## Verdict
8+
9+
| Scope | Verdict |
10+
|-------|---------|
11+
| `main` branch | FAIL |
12+
| PR #2897 (`task/2884-validate-path-dead-code`, mergeable=True) | PASS |
13+
| PR #2888 (`task/2884-validate-path-starts-with`, mergeable=False) | PASS |
14+
15+
## Acceptance Criteria
16+
17+
| # | Criterion | `main` | PR #2897 | PR #2888 |
18+
|---|-----------|--------|----------|----------|
19+
| AC1 | `validate_path` uses `path.starts_with(&self.root)` | FAIL | PASS | PASS |
20+
| AC2 | Dead `key.contains('/')` check removed | FAIL | PASS | PASS |
21+
| AC3 | Sibling-dir regression test added | FAIL | PASS | PASS |
22+
| AC4 | `cargo test -p terraphim_workspace` passes || 27/27 | 29/29 |
23+
24+
## Code Evidence
25+
26+
### Main branch (`lib.rs:240`)
27+
```rust
28+
// BUG: string comparison — "/tmp/ws_evil/f" incorrectly passes against root "/tmp/ws"
29+
let path_str = path.to_string_lossy();
30+
let root_str = self.root.to_string_lossy();
31+
if !path_str.starts_with(root_str.as_ref()) { ... }
32+
// Dead code: lines 248-252 key.contains('/') check
33+
```
34+
35+
### PR #2897 (`task/2884-validate-path-dead-code`, commit `70046217`)
36+
```rust
37+
// FIXED: component-aware Path::starts_with
38+
fn validate_path(&self, path: &Path, _identifier: &str) -> Result<()> {
39+
if !path.starts_with(&self.root) { ... }
40+
Ok(())
41+
}
42+
```
43+
Regression test: `path_prefix_confusion_is_rejected` at `lib.rs:573`
44+
45+
### PR #2888 (`task/2884-validate-path-starts-with`, commit `08456e26`)
46+
Additional security hardening: `..` component pre-rejection + security-sentinel P1-1/P2-1/P2-2 fixes.
47+
mergeable=False (conflict).
48+
49+
## Recommendation
50+
51+
Merge **PR #2897** — satisfies all acceptance criteria, mergeable=True, compound-review GO.
52+
Security-sentinel findings from PR #2888 (cleanup() path guard, env-var stripping) should be tracked separately.
53+
54+
## Correction to Prior Analysis
55+
56+
Comment #52253 (quality-coordinator) claimed "Path::starts_with was already correct on main." This is incorrect. Direct inspection of `main:crates/terraphim_workspace/src/lib.rs:240` confirms the string `starts_with` bug is present.

0 commit comments

Comments
 (0)