Skip to content

Commit b0802be

Browse files
committed
Merge PR #1 (incorporate Codex release-blocker fixes)
2 parents ce86952 + 4678344 commit b0802be

3 files changed

Lines changed: 88 additions & 31 deletions

File tree

.github/workflows/ci.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,5 +18,7 @@ jobs:
1818
run: cargo check
1919
- name: Test
2020
run: cargo test
21+
- name: Clean tree
22+
run: git diff --exit-code && git diff --cached --exit-code
2123
- name: Clippy
2224
run: cargo clippy -- -D warnings

src/cli.rs

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -630,6 +630,42 @@ fn normalize_path(path: &std::path::Path) -> String {
630630
s.replace('\\', "/")
631631
}
632632

633+
fn is_sensitive_context_path(path: &str) -> bool {
634+
let normalized = path.replace('\\', "/");
635+
let lower = normalized.to_ascii_lowercase();
636+
let file_name = lower.rsplit('/').next().unwrap_or(lower.as_str());
637+
638+
file_name == ".env"
639+
|| file_name.starts_with(".env.")
640+
|| file_name.ends_with(".key")
641+
|| file_name.ends_with(".pem")
642+
|| file_name.ends_with(".p12")
643+
|| file_name.ends_with(".pfx")
644+
|| file_name.contains("api_key")
645+
|| file_name.contains("apikey")
646+
|| file_name.contains("secret")
647+
|| file_name.contains("token")
648+
|| file_name.contains("credential")
649+
|| matches!(
650+
file_name,
651+
"key" | "keys" | "id_rsa" | "id_dsa" | "id_ecdsa" | "id_ed25519"
652+
)
653+
}
654+
655+
fn ensure_provider_network_allowed(
656+
config: &Config,
657+
profile: &ProviderProfile,
658+
provider_name: &str,
659+
) -> Result<(), String> {
660+
if config.policy.allow_provider_network && profile.network.unwrap_or(true) {
661+
return Ok(());
662+
}
663+
664+
Err(format!(
665+
"Network access denied by security policy for provider '{provider_name}'. Enable allow_provider_network and provider network=true in config to allow live execution."
666+
))
667+
}
668+
633669
fn redact_secrets(content: &str) -> String {
634670
let mut redacted = String::new();
635671
for line in content.lines() {
@@ -676,6 +712,7 @@ fn build_context_pack(task: &str) -> Result<ContextPack, String> {
676712
|| rel_path.ends_with(".dll")
677713
|| rel_path.ends_with(".pdb")
678714
|| rel_path == "Cargo.lock"
715+
|| is_sensitive_context_path(&rel_path)
679716
{
680717
continue;
681718
}
@@ -700,6 +737,14 @@ fn build_context_pack(task: &str) -> Result<ContextPack, String> {
700737
".git/".to_string(),
701738
".comptext/".to_string(),
702739
"reports/".to_string(),
740+
".env".to_string(),
741+
".env.*".to_string(),
742+
"*.key".to_string(),
743+
"*.pem".to_string(),
744+
"*.p12".to_string(),
745+
"*.pfx".to_string(),
746+
"*key*".to_string(),
747+
"*credential*".to_string(),
703748
],
704749
allowed_write_paths: vec![],
705750
forbidden_actions: vec![],
@@ -846,6 +891,8 @@ fn handle_ask(
846891
Ok(())
847892
}
848893
"ollama" => {
894+
ensure_provider_network_allowed(config, profile, resolved_provider)?;
895+
849896
use crate::provider::{OllamaProvider, Provider};
850897
let url = profile
851898
.base_url
@@ -971,6 +1018,8 @@ fn handle_propose(provider_name: Option<&str>, task: &str, config: &Config) -> R
9711018
prov.execute(&request)?
9721019
}
9731020
"ollama" => {
1021+
ensure_provider_network_allowed(config, profile, resolved_provider)?;
1022+
9741023
use crate::provider::{OllamaProvider, Provider};
9751024
let url = profile
9761025
.base_url

tests/cli_smoke.rs

Lines changed: 37 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,29 @@ fn test_lock() -> MutexGuard<'static, ()> {
1010
.unwrap_or_else(|poisoned| poisoned.into_inner())
1111
}
1212

13+
struct FileGuard {
14+
path: std::path::PathBuf,
15+
original: Option<Vec<u8>>,
16+
}
17+
18+
impl FileGuard {
19+
fn new(path: impl Into<std::path::PathBuf>) -> Self {
20+
let path = path.into();
21+
let original = std::fs::read(&path).ok();
22+
Self { path, original }
23+
}
24+
}
25+
26+
impl Drop for FileGuard {
27+
fn drop(&mut self) {
28+
if let Some(ref content) = self.original {
29+
let _ = std::fs::write(&self.path, content);
30+
} else if self.path.exists() {
31+
let _ = std::fs::remove_file(&self.path);
32+
}
33+
}
34+
}
35+
1336
fn run(args: &[&str]) -> String {
1437
let output = Command::new(env!("CARGO_BIN_EXE_ctxt"))
1538
.args(args)
@@ -60,7 +83,7 @@ fn ask_dummy_provider_succeeds() {
6083
}
6184

6285
#[test]
63-
fn ask_ollama_provider_fails_gracefully_offline() {
86+
fn ask_ollama_provider_respects_network_deny_policy() {
6487
let _guard = test_lock();
6588
let output = std::process::Command::new(env!("CARGO_BIN_EXE_ctxt"))
6689
.args(["ask", "--provider", "ollama-local", "hello"])
@@ -69,53 +92,47 @@ fn ask_ollama_provider_fails_gracefully_offline() {
6992

7093
assert!(
7194
!output.status.success(),
72-
"command should fail because local Ollama is offline"
95+
"command should fail because network is denied by policy"
7396
);
7497
let stderr = String::from_utf8(output.stderr).expect("stderr should be UTF-8");
7598
let stderr_lower = stderr.to_ascii_lowercase();
7699
assert!(
77-
stderr_lower.contains("ollama")
78-
&& (stderr_lower.contains("error:")
79-
|| stderr_lower.contains("failed")
80-
|| stderr_lower.contains("refused")
81-
|| stderr_lower.contains("connection")),
82-
"unexpected stderr from offline ollama run: {stderr}"
100+
stderr_lower.contains("network access denied") && stderr_lower.contains("ollama-local"),
101+
"unexpected stderr from network-denied ollama run: {stderr}"
83102
);
84103
}
85104

86105
#[test]
87106
fn propose_dummy_provider_succeeds() {
88107
let _guard = test_lock();
89-
let slugified_path = std::path::Path::new("proposals/proposal_add_context_inspect.json");
90108
let latest_path = std::path::Path::new("proposals/proposal.latest.json");
109+
let _latest_guard = FileGuard::new(latest_path);
110+
let task = "Smoke temporary proposal";
111+
let slugified_path = std::path::Path::new("proposals/proposal_smoke_temporary_proposal.json");
112+
let _slugified_guard = FileGuard::new(slugified_path);
91113
if slugified_path.exists() {
92114
let _ = std::fs::remove_file(slugified_path);
93115
}
94-
if latest_path.exists() {
95-
let _ = std::fs::remove_file(latest_path);
96-
}
97116

98-
let stdout = run(&["propose", "--provider", "dummy", "Add context inspect"]);
117+
let stdout = run(&["propose", "--provider", "dummy", task]);
99118
assert!(stdout.contains("Proposal generated successfully."));
100-
assert!(stdout.contains("Proposal file: proposals/proposal_add_context_inspect.json"));
119+
assert!(stdout.contains("Proposal file: proposals/proposal_smoke_temporary_proposal.json"));
101120
assert!(stdout.contains("Latest reference: proposals/proposal.latest.json"));
102121

103122
assert!(slugified_path.exists());
104123
assert!(latest_path.exists());
105124

106125
let proposal_content = std::fs::read_to_string(latest_path).unwrap();
107-
assert!(proposal_content.contains("\"task\": \"Add context inspect\""));
126+
assert!(proposal_content.contains("\"task\": \"Smoke temporary proposal\""));
108127
assert!(proposal_content.contains("\"schema_version\": \"0.1\""));
109128
assert!(proposal_content.contains("Mock patch generated by dummy provider:"));
110-
111-
let _ = std::fs::remove_file(slugified_path);
112-
let _ = std::fs::remove_file(latest_path);
113129
}
114130

115131
#[test]
116132
fn apply_and_validate_succeeds() {
117133
let _guard = test_lock();
118134
let mock_file = std::path::Path::new("tests/mock_applied_patch.rs");
135+
let _mock_file_guard = FileGuard::new(mock_file);
119136
std::fs::write(mock_file, "// initial\n").unwrap();
120137

121138
let mock_proposal = serde_json::json!({
@@ -137,19 +154,13 @@ fn apply_and_validate_succeeds() {
137154
});
138155

139156
let proposal_path = std::path::Path::new("proposals/proposal_test_apply.json");
157+
let _proposal_guard = FileGuard::new(proposal_path);
140158
std::fs::write(
141159
proposal_path,
142160
serde_json::to_string_pretty(&mock_proposal).unwrap(),
143161
)
144162
.unwrap();
145163

146-
let latest_path = std::path::Path::new("proposals/proposal.latest.json");
147-
std::fs::write(
148-
latest_path,
149-
serde_json::to_string_pretty(&mock_proposal).unwrap(),
150-
)
151-
.unwrap();
152-
153164
let stdout_apply = run(&["apply", "--yes", "proposals/proposal_test_apply.json"]);
154165
assert!(stdout_apply.contains("Applying Proposal:"));
155166
assert!(stdout_apply.contains("Proposal applied and validated successfully."));
@@ -160,10 +171,6 @@ fn apply_and_validate_succeeds() {
160171
let stdout_validate = run(&["validate"]);
161172
assert!(stdout_validate.contains("Standard local validation commands:"));
162173
assert!(stdout_validate.contains("cargo test"));
163-
164-
let _ = std::fs::remove_file(mock_file);
165-
let _ = std::fs::remove_file(proposal_path);
166-
let _ = std::fs::remove_file(latest_path);
167174
}
168175

169176
#[test]
@@ -188,6 +195,7 @@ fn apply_rejects_disallowed_paths() {
188195
});
189196

190197
let path = std::path::Path::new("proposals/proposal_malicious.json");
198+
let _proposal_guard = FileGuard::new(path);
191199
std::fs::write(path, serde_json::to_string_pretty(&mock_proposal).unwrap()).unwrap();
192200

193201
let output = std::process::Command::new(env!("CARGO_BIN_EXE_ctxt"))
@@ -201,6 +209,4 @@ fn apply_rejects_disallowed_paths() {
201209
);
202210
let stderr = String::from_utf8(output.stderr).expect("stderr should be UTF-8");
203211
assert!(stderr.contains("Security Policy Violation: Path '.env' is not an allowed write path."));
204-
205-
let _ = std::fs::remove_file(path);
206212
}

0 commit comments

Comments
 (0)