From 05b47576323c05d6f869cff64a62e57d58f71b56 Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Fri, 4 Sep 2026 10:02:21 +0100 Subject: [PATCH 1/6] fix(automaton): restore corrupt fixer.rs and clear repo-path scars `robot-repo-automaton/src/fixer.rs` was committed as a raw diff fragment rather than the file it described (dd5945b1, 2026-07-29), leaving the fleet's only actuator non-compiling on `main` for roughly four weeks while `lib.rs` declared `pub mod fixer;` and `main.rs` called `fixer.apply_and_commit(...)`. `rust.yml` caught it on 08-24 and 08-25 and nothing acted, because it is not a required context. Restores the intact 940-line module and clears the `"/var$REPOS_DIR"` path-scrub scars in `main.rs::resolve_repo_path`, `repo-path-overrides.json` and the supporting scripts. The overrides file is reclassified against disk: 35 entries kept and rewritten relative, 42 dropped as resolving nowhere, 13 dropped as redundant with the flat top-level layout, 6 dropped as identity mappings. Gates: cargo build --all-targets clean; cargo test 101 passed / 0 failed; clippy --all-targets -D warnings 0 diagnostics; bash -n across 8 scripts; Elixir parse check on sync-all-parallel.exs (with a deliberately-broken control proving the checker can fail); 3 shell smoke suites, 17 assertions, 0 failures. Co-Authored-By: Claude Opus 5 --- Justfile | 55 +- README.adoc | 2 +- docs/wiki-source/Build-and-Run.md | 2 +- fleet-coordinator.sh | 7 +- robot-repo-automaton/SONNET-TASKS.adoc | 2 +- robot-repo-automaton/src/fixer.rs | 954 ++++++++++++++++++++++++- robot-repo-automaton/src/hypatia.rs | 9 +- robot-repo-automaton/src/main.rs | 25 +- scripts/dispatch-runner.sh | 23 +- scripts/enroll-hypatia-fleet.sh | 5 +- scripts/fix-license-hygiene.sh | 9 +- scripts/fix-proven-substitute.sh | 9 +- scripts/list-supervised-repos.sh | 4 +- scripts/maintenance-hard-pass.sh | 11 +- scripts/process-review-findings.sh | 2 +- scripts/repo-path-overrides.json | 131 +--- scripts/sync-all-parallel.exs | 8 +- shared-context/enrollment/README.adoc | 2 +- 18 files changed, 1080 insertions(+), 180 deletions(-) diff --git a/Justfile b/Justfile index 24ece57a..7bedcd96 100644 --- a/Justfile +++ b/Justfile @@ -6,6 +6,9 @@ set shell := ["bash", "-euo", "pipefail", "-c"] +# Base directory holding local repo checkouts; override with REPOS_BASE. +repos_base := env_var_or_default("REPOS_BASE", env_var("HOME") / "developer/hyper-repos") + # Default recipe: show help import? "contractile.just" @@ -59,10 +62,10 @@ hypatia-scan: # Run panic-attack static analysis panic-scan: - @if [ -x "/var$REPOS_DIR/panic-attacker/target/release/panic-attack" ]; then \ - /var$REPOS_DIR/panic-attacker/target/release/panic-attack assail . --verbose; \ + @if [ -x "{{repos_base}}/panic-attack/target/release/panic-attack" ]; then \ + {{repos_base}}/panic-attack/target/release/panic-attack assail . --verbose; \ else \ - echo "panic-attack not built — run 'cd /var$REPOS_DIR/panic-attacker && cargo build --release'"; \ + echo "panic-attack not built — run 'cd {{repos_base}}/panic-attack && cargo build --release'"; \ fi # Run release maintenance hard-pass on a target repository @@ -70,7 +73,7 @@ maintenance-hard-pass repo *ARGS: bash scripts/maintenance-hard-pass.sh --repo "{{repo}}" {{ARGS}} # Discover and register repo coverage for gitbot-fleet/hypatia -enroll-repos repos_root="/var$REPOS_DIR" apply="false": +enroll-repos repos_root=repos_base apply="false": @if [ "{{apply}}" = "true" ]; then \ bash scripts/enroll-hypatia-fleet.sh --repos-root "{{repos_root}}" --apply; \ else \ @@ -131,14 +134,14 @@ doctor: } check "just" just "1.25" check "git" git "2.40" -# Optional tools -if command -v panic-attack >/dev/null 2>&1; then - echo " [OK] panic-attack — available" - PASS=$((PASS + 1)) -else - echo " [WARN] panic-attack — not found (pre-commit scanner)" - WARN=$((WARN + 1)) -fi + # Optional tools + if command -v panic-attack >/dev/null 2>&1; then + echo " [OK] panic-attack — available" + PASS=$((PASS + 1)) + else + echo " [WARN] panic-attack — not found (pre-commit scanner)" + WARN=$((WARN + 1)) + fi echo "" echo " Result: $PASS passed, $FAIL failed, $WARN warnings" if [ "$FAIL" -gt 0 ]; then @@ -154,10 +157,10 @@ heal: echo " Gitbot Fleet Heal — Automatic Tool Installation" echo "═══════════════════════════════════════════════════" echo "" -if ! command -v just >/dev/null 2>&1; then - echo "Installing just..." - cargo install just 2>/dev/null || echo "Install just from https://just.systems" -fi + if ! command -v just >/dev/null 2>&1; then + echo "Installing just..." + cargo install just 2>/dev/null || echo "Install just from https://just.systems" + fi echo "" echo "Heal complete. Run 'just doctor' to verify." @@ -191,16 +194,16 @@ help-me: echo " Gitbot Fleet — Common Workflows" echo "═══════════════════════════════════════════════════" echo "" -echo "FIRST TIME SETUP:" -echo " just doctor Check toolchain" -echo " just heal Fix missing tools" -echo "" -echo "PRE-COMMIT:" -echo " just assail Run panic-attacker scan" -echo "" -echo "LEARN:" -echo " just tour Guided project tour" -echo " just default List all recipes" + echo "FIRST TIME SETUP:" + echo " just doctor Check toolchain" + echo " just heal Fix missing tools" + echo "" + echo "PRE-COMMIT:" + echo " just assail Run panic-attacker scan" + echo "" + echo "LEARN:" + echo " just tour Guided project tour" + echo " just default List all recipes" # Print the current CRG grade (reads from READINESS.md '**Current Grade:** X' line) diff --git a/README.adoc b/README.adoc index 76924e64..5b17d7ba 100644 --- a/README.adoc +++ b/README.adoc @@ -111,7 +111,7 @@ just scan-supervised `maintenance-hard-pass` enforces fail-on-warn release gating using the target repo's maintenance script. `enroll-repos` refreshes repository -coverage metadata; pass `/var$REPOS_DIR true` to also write enrollment +coverage metadata; pass `~/developer/hyper-repos true` to also write enrollment directives into repos that already have `.machine_readable/`. `scan-supervised` runs Hypatia across the supervised inventory (`~/.git-private-farm.scm`, `~/.git-private-repos`, or the enrollment diff --git a/docs/wiki-source/Build-and-Run.md b/docs/wiki-source/Build-and-Run.md index 88af2200..99396cff 100644 --- a/docs/wiki-source/Build-and-Run.md +++ b/docs/wiki-source/Build-and-Run.md @@ -22,7 +22,7 @@ just scan-supervised - **`maintenance-hard-pass`** enforces fail-on-warn release gating using the target repo's maintenance script. - **`enroll-repos`** refreshes repository coverage metadata. Pass - `/var$REPOS_DIR true` to write enrollment directives into repos that + `~/developer/hyper-repos true` to write enrollment directives into repos that already have `.machine_readable/`. - **`scan-supervised`** runs Hypatia across the supervised inventory (`~/.git-private-farm.scm`, `~/.git-private-repos`, or the enrollment diff --git a/fleet-coordinator.sh b/fleet-coordinator.sh index eb41f5d7..350c67f9 100755 --- a/fleet-coordinator.sh +++ b/fleet-coordinator.sh @@ -6,6 +6,7 @@ set -euo pipefail FLEET_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +REPOS_BASE="${REPOS_BASE:-$HOME/developer/hyper-repos}" SHARED_CONTEXT="$FLEET_DIR/shared-context" FINDINGS_DIR="$SHARED_CONTEXT/findings" SESSION_ID="$(date +%Y%m%d-%H%M%S)" @@ -214,7 +215,7 @@ process_findings() { fi # Load recipes for triangle routing - local recipes_dir="/var$REPOS_DIR/verisim-data/recipes" + local recipes_dir="$REPOS_BASE/verisimdb-data/recipes" local substitutions_file="$recipes_dir/proven-substitutions.json" # Scan for findings in new directory structure: findings//.json @@ -326,9 +327,9 @@ process_findings() { # Process eliminate-tier findings via dispatch-runner (auto-execute) local dispatch_runner="$FLEET_DIR/scripts/dispatch-runner.sh" - if [[ -x "$dispatch_runner" && -f "/var$REPOS_DIR/verisim-data/dispatch/pending.jsonl" ]]; then + if [[ -x "$dispatch_runner" && -f "$REPOS_BASE/verisimdb-data/dispatch/pending.jsonl" ]]; then local auto_count - auto_count=$(jq -c 'select(.strategy == "auto_execute")' /var$REPOS_DIR/verisim-data/dispatch/pending.jsonl 2>/dev/null | wc -l) + auto_count=$(jq -c 'select(.strategy == "auto_execute")' "$REPOS_BASE/verisimdb-data/dispatch/pending.jsonl" 2>/dev/null | wc -l) if [[ $auto_count -gt 0 ]]; then log_bot "robot-repo-automaton" "Dispatch runner: $auto_count auto-execute entries pending" fi diff --git a/robot-repo-automaton/SONNET-TASKS.adoc b/robot-repo-automaton/SONNET-TASKS.adoc index f074368c..b18b90f3 100644 --- a/robot-repo-automaton/SONNET-TASKS.adoc +++ b/robot-repo-automaton/SONNET-TASKS.adoc @@ -27,7 +27,7 @@ changed in gitbot-shared-context. Steps: 1. Read `+src/fleet.rs+` to identify the exact error lines 2. Read the gitbot-shared-context crate API (check -`+/var$REPOS_DIR/gitbot-fleet/crates/gitbot-shared-context/src/lib.rs+` +`+shared-context/src/lib.rs+` or the public API) 3. Fix the API calls to match the current gitbot-shared-context interface 4. Common fixes: - `+ctx.findings(bot_id)+` may need to be `+ctx.get_findings(bot_id)+` or diff --git a/robot-repo-automaton/src/fixer.rs b/robot-repo-automaton/src/fixer.rs index 942f5c85..7b55b274 100644 --- a/robot-repo-automaton/src/fixer.rs +++ b/robot-repo-automaton/src/fixer.rs @@ -1,14 +1,940 @@ -@@ -- content -- .replace("gitbot-fleet", repo_name) -- .replace("{{LICENSE}}", "MPL-2.0") -- .replace("{{YEAR}}", &year) -- .replace("{{AUTHOR}}", "Jonathan D.A. Jewell") -- .replace("{{EMAIL}}", "j.d.a.jewell@open.ac.uk"); -+ content -+ .replace("gitbot-fleet", repo_name) -+ .replace("{{LICENSE}}", "MPL-2.0") -+ .replace("{{YEAR}}", &year) -+ .replace("{{AUTHOR}}", "Jonathan D.A. Jewell") -+ .replace("{{EMAIL}}", "j.d.a.jewell@open.ac.uk") -@@ +// SPDX-License-Identifier: MPL-2.0 +//! Fix application for detected issues +//! +//! Provides functionality to apply automated fixes to repositories: +//! - **Delete**: Remove files that should not exist +//! - **Modify**: Apply line-level transformations with safety checks and rollback +//! - **Create**: Create missing files from templates with variable expansion +//! - **Disable**: Rename files to .disabled extension + +use git2::{Repository, Signature}; +use regex::Regex; +use std::path::{Path, PathBuf}; +use tracing::{debug, info, warn}; + +use crate::catalog::{Fix, FixAction}; +use crate::detector::DetectedIssue; +use crate::error::{Error, Result}; + +/// Result of applying a fix +#[derive(Debug)] +pub struct FixResult { + /// The issue ID that was addressed + pub issue_id: String, + /// Whether the fix was successfully applied + pub success: bool, + /// Human-readable description of the action taken + pub action_taken: String, + /// Files that were modified by this fix + pub files_modified: Vec, + /// Error message if the fix failed + pub error: Option, +} + +/// Specification for a line-level modification +#[derive(Debug, Clone)] +pub enum ModifySpec { + /// Replace entire line content at a specific line number (1-indexed) + ReplaceLine { line: usize, content: String }, + /// Insert content before a specific line number (1-indexed) + InsertBefore { line: usize, content: String }, + /// Insert content after a specific line number (1-indexed) + InsertAfter { line: usize, content: String }, + /// Replace all occurrences of a regex pattern with a replacement string + ReplacePattern { pattern: String, replacement: String }, + /// Prepend content to the beginning of the file + Prepend { content: String }, + /// Append content to the end of the file + Append { content: String }, +} + +/// Repository fixer that applies automated corrections +pub struct Fixer { + /// Root path of the repository being fixed + repo_path: PathBuf, + /// When true, no actual changes are made (only logged) + dry_run: bool, +} + +/// Known binary file extensions that should never be modified +const BINARY_EXTENSIONS: &[&str] = &[ + "png", "jpg", "jpeg", "gif", "bmp", "ico", "webp", "svg", + "pdf", "doc", "docx", "xls", "xlsx", "ppt", "pptx", + "zip", "tar", "gz", "bz2", "xz", "7z", "rar", + "exe", "dll", "so", "dylib", "o", "a", + "wasm", "pyc", "class", + "ttf", "otf", "woff", "woff2", "eot", + "mp3", "mp4", "avi", "mkv", "flac", "ogg", "wav", + "db", "sqlite", "sqlite3", +]; + +impl Fixer { + /// Create a new fixer for a repository + pub fn new(repo_path: PathBuf, dry_run: bool) -> Self { + Fixer { repo_path, dry_run } + } + + /// Apply a fix for a detected issue + pub fn apply(&self, issue: &DetectedIssue, fix: &Fix) -> Result { + // EXCLUSION REGISTRY GUARD: refuse the write if the target repo, + // origin, or target path is on the estate-wide denylist. In dry-run + // mode we still check so operators can preview denials without + // surprises. The guard returns Err on denial; map it to a + // FixResult::failure so one denied fix does not abort a batch. + if let Err(e) = crate::registry_guard::check_write( + &self.repo_path, + crate::exclusion_registry::Action::Write, + Some(&fix.target), + ) { + warn!(target = %fix.target, error = %e, "registry guard denied fix"); + return Ok(FixResult { + issue_id: issue.error_type_id.clone(), + success: false, + action_taken: format!("DENIED by bot_exclusion_registry: {e}"), + files_modified: vec![], + error: Some(e.to_string()), + }); + } + + let target_path = self.repo_path.join(&fix.target); + + // SECURITY: Reject any target path that escapes the repository root. + // Normalise both paths and verify the target is a child of repo_path. + // This prevents path traversal attacks (e.g. target = "../../etc/passwd"). + let canonical_repo = self.repo_path.canonicalize().unwrap_or_else(|_| self.repo_path.clone()); + // For the target we normalise without requiring the path to exist yet + // (it may be a to-be-created file), so we use a manual component walk. + let normalised_target = normalise_path(&target_path); + if !normalised_target.starts_with(&canonical_repo) { + warn!( + target = %fix.target, + repo = %self.repo_path.display(), + "SECURITY: fix target escapes repository root — rejecting" + ); + return Ok(FixResult { + issue_id: issue.error_type_id.clone(), + success: false, + action_taken: format!( + "REJECTED: target '{}' escapes repository boundary", + fix.target + ), + files_modified: vec![], + error: Some(format!( + "Security violation: target path '{}' is outside the repository directory", + fix.target + )), + }); + } + + match fix.action { + FixAction::Delete => self.apply_delete(&target_path, issue), + FixAction::Modify => self.apply_modify(&target_path, issue, fix), + FixAction::Create => self.apply_create(&target_path, issue, fix), + FixAction::Disable => self.apply_disable(&target_path, issue), + } + } + + /// Check if a file is binary based on its extension + fn is_binary(path: &Path) -> bool { + path.extension() + .and_then(|ext| ext.to_str()) + .map(|ext| BINARY_EXTENSIONS.contains(&ext.to_lowercase().as_str())) + .unwrap_or(false) + } + + /// Parse a modification specification string into structured operations + /// + /// Supported formats: + /// - `replace-line::` - Replace line N with content + /// - `insert-before::` - Insert content before line N + /// - `insert-after::` - Insert content after line N + /// - `replace-pattern::` - Replace regex matches + /// - `prepend:` - Add content at file beginning + /// - `append:` - Add content at file end + fn parse_modification(spec: &str) -> Result { + let parts: Vec<&str> = spec.splitn(3, ':').collect(); + + match parts.first().copied() { + Some("replace-line") => { + if parts.len() < 3 { + return Err(Error::Fix("replace-line requires line number and content".into())); + } + let line: usize = parts[1] + .parse() + .map_err(|_| Error::Fix(format!("Invalid line number: {}", parts[1])))?; + Ok(ModifySpec::ReplaceLine { + line, + content: parts[2].to_string(), + }) + } + Some("insert-before") => { + if parts.len() < 3 { + return Err(Error::Fix("insert-before requires line number and content".into())); + } + let line: usize = parts[1] + .parse() + .map_err(|_| Error::Fix(format!("Invalid line number: {}", parts[1])))?; + Ok(ModifySpec::InsertBefore { + line, + content: parts[2].to_string(), + }) + } + Some("insert-after") => { + if parts.len() < 3 { + return Err(Error::Fix("insert-after requires line number and content".into())); + } + let line: usize = parts[1] + .parse() + .map_err(|_| Error::Fix(format!("Invalid line number: {}", parts[1])))?; + Ok(ModifySpec::InsertAfter { + line, + content: parts[2].to_string(), + }) + } + Some("replace-pattern") => { + if parts.len() < 3 { + return Err(Error::Fix("replace-pattern requires pattern and replacement".into())); + } + Ok(ModifySpec::ReplacePattern { + pattern: parts[1].to_string(), + replacement: parts[2].to_string(), + }) + } + Some("prepend") => { + if parts.len() < 2 { + return Err(Error::Fix("prepend requires content".into())); + } + // Rejoin parts[1..] in case content contained ':' + let content = parts[1..].join(":"); + Ok(ModifySpec::Prepend { content }) + } + Some("append") => { + if parts.len() < 2 { + return Err(Error::Fix("append requires content".into())); + } + let content = parts[1..].join(":"); + Ok(ModifySpec::Append { content }) + } + _ => Err(Error::Fix(format!("Unknown modification type: {}", spec))), + } + } + + /// Apply a modification specification to file content + fn apply_modification(content: &str, spec: &ModifySpec) -> Result { + let mut lines: Vec = content.lines().map(|l| l.to_string()).collect(); + + match spec { + ModifySpec::ReplaceLine { line, content: new_content } => { + if *line == 0 || *line > lines.len() { + return Err(Error::Fix(format!( + "Line {} out of range (file has {} lines)", + line, + lines.len() + ))); + } + lines[*line - 1] = new_content.clone(); + } + ModifySpec::InsertBefore { line, content: new_content } => { + if *line == 0 || *line > lines.len() + 1 { + return Err(Error::Fix(format!( + "Line {} out of range for insertion (file has {} lines)", + line, + lines.len() + ))); + } + lines.insert(*line - 1, new_content.clone()); + } + ModifySpec::InsertAfter { line, content: new_content } => { + if *line == 0 || *line > lines.len() { + return Err(Error::Fix(format!( + "Line {} out of range for insertion (file has {} lines)", + line, + lines.len() + ))); + } + lines.insert(*line, new_content.clone()); + } + ModifySpec::ReplacePattern { pattern, replacement } => { + let re = Regex::new(pattern) + .map_err(|e| Error::Fix(format!("Invalid regex pattern '{}': {}", pattern, e)))?; + let result = re.replace_all(content, replacement.as_str()); + return Ok(result.into_owned()); + } + ModifySpec::Prepend { content: new_content } => { + lines.insert(0, new_content.clone()); + } + ModifySpec::Append { content: new_content } => { + lines.push(new_content.clone()); + } + } + + // Preserve trailing newline if original had one + let mut result = lines.join("\n"); + if content.ends_with('\n') { + result.push('\n'); + } + Ok(result) + } + + /// Delete a file + fn apply_delete( + &self, + target_path: &Path, + issue: &DetectedIssue, + ) -> Result { + if !target_path.exists() { + return Ok(FixResult { + issue_id: issue.error_type_id.clone(), + success: true, + action_taken: "File already deleted".to_string(), + files_modified: vec![], + error: None, + }); + } + + if self.dry_run { + info!("[DRY RUN] Would delete: {}", target_path.display()); + return Ok(FixResult { + issue_id: issue.error_type_id.clone(), + success: true, + action_taken: format!("[DRY RUN] Would delete {}", target_path.display()), + files_modified: vec![target_path.to_path_buf()], + error: None, + }); + } + + std::fs::remove_file(target_path)?; + info!("Deleted: {}", target_path.display()); + + Ok(FixResult { + issue_id: issue.error_type_id.clone(), + success: true, + action_taken: format!("Deleted {}", target_path.display()), + files_modified: vec![target_path.to_path_buf()], + error: None, + }) + } + + /// Modify a file with safety checks and rollback support + /// + /// Reads the modification specification from the fix, applies it to the file, + /// and rolls back if the modification produces invalid content. + fn apply_modify( + &self, + target_path: &Path, + issue: &DetectedIssue, + fix: &Fix, + ) -> Result { + if !target_path.exists() { + return Ok(FixResult { + issue_id: issue.error_type_id.clone(), + success: false, + action_taken: "File does not exist".to_string(), + files_modified: vec![], + error: Some("Cannot modify non-existent file".to_string()), + }); + } + + // Safety: never modify binary files + if Self::is_binary(target_path) { + warn!("Skipping binary file: {}", target_path.display()); + return Ok(FixResult { + issue_id: issue.error_type_id.clone(), + success: false, + action_taken: "Skipped binary file".to_string(), + files_modified: vec![], + error: Some("Cannot modify binary file".to_string()), + }); + } + + let modification = fix + .modification + .as_deref() + .unwrap_or("unspecified modification"); + + if self.dry_run { + info!( + "[DRY RUN] Would modify {}: {}", + target_path.display(), + modification + ); + return Ok(FixResult { + issue_id: issue.error_type_id.clone(), + success: true, + action_taken: format!( + "[DRY RUN] Would modify {}: {}", + target_path.display(), + modification + ), + files_modified: vec![target_path.to_path_buf()], + error: None, + }); + } + + // Snapshot original content for rollback + let original_content = std::fs::read_to_string(target_path) + .map_err(|e| Error::Fix(format!("Failed to read {}: {}", target_path.display(), e)))?; + + // Parse and apply the modification + let spec = Self::parse_modification(modification)?; + let new_content = match Self::apply_modification(&original_content, &spec) { + Ok(content) => content, + Err(e) => { + warn!( + "Modification failed for {}: {}", + target_path.display(), + e + ); + return Ok(FixResult { + issue_id: issue.error_type_id.clone(), + success: false, + action_taken: format!("Modification failed: {}", e), + files_modified: vec![], + error: Some(format!("Modification failed: {}", e)), + }); + } + }; + + // Verify the modification produced different content + if new_content == original_content { + debug!("No changes needed for {}", target_path.display()); + return Ok(FixResult { + issue_id: issue.error_type_id.clone(), + success: true, + action_taken: "No changes needed".to_string(), + files_modified: vec![], + error: None, + }); + } + + // Write modified content + if let Err(e) = std::fs::write(target_path, &new_content) { + // Attempt rollback on write failure + let _ = std::fs::write(target_path, &original_content); + return Err(Error::Fix(format!( + "Failed to write modified file {}: {}", + target_path.display(), + e + ))); + } + + info!( + "Modified {}: {}", + target_path.display(), + modification + ); + + Ok(FixResult { + issue_id: issue.error_type_id.clone(), + success: true, + action_taken: format!("Modified {}: {}", target_path.display(), modification), + files_modified: vec![target_path.to_path_buf()], + error: None, + }) + } + + /// Create a file with template expansion + /// + /// Supports template variables: + /// - `gitbot-fleet` - Repository name + /// - `hyperpolymath` - Repository owner + /// - `{{LICENSE}}` - License identifier + /// - `{{YEAR}}` - Current year + fn apply_create( + &self, + target_path: &Path, + issue: &DetectedIssue, + fix: &Fix, + ) -> Result { + if target_path.exists() { + return Ok(FixResult { + issue_id: issue.error_type_id.clone(), + success: true, + action_taken: "File already exists".to_string(), + files_modified: vec![], + error: None, + }); + } + + // Check if the file would be gitignored + if self.would_be_gitignored(target_path) { + warn!( + "Skipping creation of gitignored file: {}", + target_path.display() + ); + return Ok(FixResult { + issue_id: issue.error_type_id.clone(), + success: false, + action_taken: "File would be gitignored".to_string(), + files_modified: vec![], + error: Some("Cannot create file that would be gitignored".to_string()), + }); + } + + if self.dry_run { + info!("[DRY RUN] Would create: {}", target_path.display()); + return Ok(FixResult { + issue_id: issue.error_type_id.clone(), + success: true, + action_taken: format!("[DRY RUN] Would create {}", target_path.display()), + files_modified: vec![target_path.to_path_buf()], + error: None, + }); + } + + // Create parent directories if needed + if let Some(parent) = target_path.parent() { + std::fs::create_dir_all(parent)?; + } + + // Get content from template or fix specification + let content = self.get_template_content(&fix.target, fix); + let expanded = self.expand_template(&content); + + // Guard: refuse to create files with empty or near-empty content. + // This prevents bots from pushing useless boilerplate when no + // template exists for the target file. + if expanded.trim().is_empty() { + warn!( + "Refusing to create {} — template produced empty content", + target_path.display() + ); + return Ok(FixResult { + issue_id: issue.error_type_id.clone(), + success: false, + action_taken: "Skipped — no template content available".to_string(), + files_modified: vec![], + error: Some(format!( + "No template for '{}'; file would be empty", + fix.target + )), + }); + } + + std::fs::write(target_path, &expanded)?; + info!("Created: {}", target_path.display()); + + Ok(FixResult { + issue_id: issue.error_type_id.clone(), + success: true, + action_taken: format!("Created {}", target_path.display()), + files_modified: vec![target_path.to_path_buf()], + error: None, + }) + } + + /// Disable a workflow (rename to .disabled) + fn apply_disable( + &self, + target_path: &Path, + issue: &DetectedIssue, + ) -> Result { + if !target_path.exists() { + return Ok(FixResult { + issue_id: issue.error_type_id.clone(), + success: true, + action_taken: "File already absent".to_string(), + files_modified: vec![], + error: None, + }); + } + + let disabled_path = target_path.with_extension("yml.disabled"); + + if self.dry_run { + info!( + "[DRY RUN] Would disable: {} -> {}", + target_path.display(), + disabled_path.display() + ); + return Ok(FixResult { + issue_id: issue.error_type_id.clone(), + success: true, + action_taken: format!( + "[DRY RUN] Would rename {} to {}", + target_path.display(), + disabled_path.display() + ), + files_modified: vec![target_path.to_path_buf()], + error: None, + }); + } + + std::fs::rename(target_path, &disabled_path)?; + info!( + "Disabled: {} -> {}", + target_path.display(), + disabled_path.display() + ); + + Ok(FixResult { + issue_id: issue.error_type_id.clone(), + success: true, + action_taken: format!( + "Renamed {} to {}", + target_path.display(), + disabled_path.display() + ), + files_modified: vec![target_path.to_path_buf(), disabled_path], + error: None, + }) + } + + /// Check if a path would be gitignored + fn would_be_gitignored(&self, path: &Path) -> bool { + if let Ok(repo) = Repository::open(&self.repo_path) { + if let Ok(relative) = path.strip_prefix(&self.repo_path) { + return repo.is_path_ignored(relative).unwrap_or(false); + } + } + false + } + + /// Get template content for a file creation + fn get_template_content(&self, target: &str, fix: &Fix) -> String { + // If the fix has explicit content in the fallback field, use it + if let Some(ref fallback) = fix.fallback { + return fallback.clone(); + } + + // Built-in templates for common files + match target { + "LICENSE" | "LICENSE.txt" => include_str!("../templates/LICENSE.tmpl").to_string(), + ".editorconfig" => include_str!("../templates/editorconfig.tmpl").to_string(), + "SECURITY.md" => include_str!("../templates/SECURITY.tmpl").to_string(), + _ => String::new(), + } + } + + /// Expand template variables in content + fn expand_template(&self, content: &str) -> String { + let repo_name = self + .repo_path + .file_name() + .and_then(|n| n.to_str()) + .unwrap_or("unknown-repo"); + + let year = chrono::Utc::now().format("%Y").to_string(); + + content + .replace("gitbot-fleet", repo_name) + .replace("{{LICENSE}}", "MPL-2.0") + .replace("{{YEAR}}", &year) + .replace("{{AUTHOR}}", "Jonathan D.A. Jewell") + .replace("{{EMAIL}}", "j.d.a.jewell@open.ac.uk") + } + + /// Commit changes to the repository + pub fn commit(&self, message: &str, files: &[PathBuf]) -> Result<()> { + // EXCLUSION REGISTRY GUARD: a commit is a write action even though + // apply() has already checked each file individually, because some + // commits come from non-apply paths (bulk tooling). Fail closed. + crate::registry_guard::check_write( + &self.repo_path, + crate::exclusion_registry::Action::Commit, + None, + )?; + + if self.dry_run { + info!("[DRY RUN] Would commit: {}", message); + return Ok(()); + } + + let repo = Repository::open(&self.repo_path)?; + let mut index = repo.index()?; + + // Stage the modified files + for file in files { + if let Ok(relative) = file.strip_prefix(&self.repo_path) { + if file.exists() { + index.add_path(relative)?; + } else { + index.remove_path(relative)?; + } + } + } + + index.write()?; + let tree_id = index.write_tree()?; + let tree = repo.find_tree(tree_id)?; + + let sig = Signature::now("robot-repo-automaton", "robot@hyperpolymath.dev")?; + let parent = repo.head()?.peel_to_commit()?; + + repo.commit( + Some("HEAD"), + &sig, + &sig, + message, + &tree, + &[&parent], + )?; + + info!("Committed: {}", message); + Ok(()) + } + + /// Apply multiple fixes and commit + pub fn apply_and_commit( + &self, + _issues: &[DetectedIssue], + fixes: &[(DetectedIssue, Fix)], + ) -> Result> { + let mut results = Vec::new(); + let mut all_modified_files = Vec::new(); + + for (issue, fix) in fixes { + let result = self.apply(issue, fix)?; + if result.success { + all_modified_files.extend(result.files_modified.clone()); + } + results.push(result); + } + + if !all_modified_files.is_empty() && !self.dry_run { + let commit_message = if fixes.len() == 1 { + fixes[0].0.commit_message.clone() + } else { + format!("fix: apply {} automated fixes", fixes.len()) + }; + self.commit(&commit_message, &all_modified_files)?; + } + + Ok(results) + } +} + +/// Normalise a path by resolving `.` and `..` components without requiring the +/// path to exist on disk (unlike `Path::canonicalize`). +/// +/// This is used for security validation: after normalisation we can check that +/// the path starts with the repository root and has not escaped via `..` traversal. +fn normalise_path(path: &Path) -> PathBuf { + use std::path::Component; + let mut normalised = PathBuf::new(); + for component in path.components() { + match component { + Component::ParentDir => { + // Pop the last element, effectively resolving ".." + normalised.pop(); + } + Component::CurDir => { + // Skip "." — it contributes nothing + } + other => { + normalised.push(other); + } + } + } + normalised +} + +#[cfg(test)] +mod tests { + use super::*; + use tempfile::TempDir; + + fn make_issue(id: &str) -> DetectedIssue { + DetectedIssue { + error_type_id: id.to_string(), + error_name: "Test Issue".to_string(), + severity: crate::catalog::Severity::Medium, + description: "Test issue description".to_string(), + affected_files: vec![], + confidence: 1.0, + suggested_fix: "Test fix".to_string(), + commit_message: "fix: test".to_string(), + } + } + + fn make_fix(action: FixAction, target: &str) -> Fix { + Fix { + action, + target: target.to_string(), + reason: None, + modification: None, + fallback: None, + } + } + + #[test] + fn test_modify_replace_line() { + let temp = TempDir::new().unwrap(); + let file_path = temp.path().join("test.txt"); + std::fs::write(&file_path, "line 1\nline 2\nline 3\n").unwrap(); + + let fixer = Fixer::new(temp.path().to_path_buf(), false); + let issue = make_issue("TEST-001"); + let fix = Fix { + action: FixAction::Modify, + target: "test.txt".to_string(), + reason: None, + modification: Some("replace-line:2:replaced line".to_string()), + fallback: None, + }; + + let result = fixer.apply(&issue, &fix).unwrap(); + assert!(result.success); + + let content = std::fs::read_to_string(&file_path).unwrap(); + assert!(content.contains("replaced line")); + assert!(!content.contains("line 2")); + } + + #[test] + fn test_modify_replace_pattern() { + let temp = TempDir::new().unwrap(); + let file_path = temp.path().join("test.txt"); + std::fs::write(&file_path, "old_value = 42\nold_value = 99\n").unwrap(); + + let fixer = Fixer::new(temp.path().to_path_buf(), false); + let issue = make_issue("TEST-002"); + let fix = Fix { + action: FixAction::Modify, + target: "test.txt".to_string(), + reason: None, + modification: Some("replace-pattern:old_value:new_value".to_string()), + fallback: None, + }; + + let result = fixer.apply(&issue, &fix).unwrap(); + assert!(result.success); + + let content = std::fs::read_to_string(&file_path).unwrap(); + assert!(content.contains("new_value")); + assert!(!content.contains("old_value")); + } + + #[test] + fn test_modify_invalid_line_rollback() { + let temp = TempDir::new().unwrap(); + let file_path = temp.path().join("test.txt"); + let original = "line 1\nline 2\n"; + std::fs::write(&file_path, original).unwrap(); + + let fixer = Fixer::new(temp.path().to_path_buf(), false); + let issue = make_issue("TEST-003"); + let fix = Fix { + action: FixAction::Modify, + target: "test.txt".to_string(), + reason: None, + modification: Some("replace-line:999:impossible".to_string()), + fallback: None, + }; + + let result = fixer.apply(&issue, &fix).unwrap(); + assert!(!result.success); + + // Verify file content unchanged + let content = std::fs::read_to_string(&file_path).unwrap(); + assert_eq!(content, original); + } + + #[test] + fn test_modify_binary_file_skipped() { + let temp = TempDir::new().unwrap(); + let file_path = temp.path().join("image.png"); + std::fs::write(&file_path, b"\x89PNG\r\n").unwrap(); + + let fixer = Fixer::new(temp.path().to_path_buf(), false); + let issue = make_issue("TEST-004"); + let fix = Fix { + action: FixAction::Modify, + target: "image.png".to_string(), + reason: None, + modification: Some("replace-line:1:hacked".to_string()), + fallback: None, + }; + + let result = fixer.apply(&issue, &fix).unwrap(); + assert!(!result.success); + assert!(result.error.unwrap().contains("binary")); + } + + #[test] + fn test_modify_prepend() { + let temp = TempDir::new().unwrap(); + let file_path = temp.path().join("test.rs"); + std::fs::write(&file_path, "fn main() {}\n").unwrap(); + + let fixer = Fixer::new(temp.path().to_path_buf(), false); + let issue = make_issue("TEST-005"); + let fix = Fix { + action: FixAction::Modify, + target: "test.rs".to_string(), + reason: None, + modification: Some("prepend:// SPDX-License-Identifier: MPL-2.0".to_string()), + fallback: None, + }; + + let result = fixer.apply(&issue, &fix).unwrap(); + assert!(result.success); + + let content = std::fs::read_to_string(&file_path).unwrap(); + assert!(content.starts_with("// SPDX-License-Identifier: MPL-2.0")); + } + + #[test] + fn test_modify_nonexistent_file() { + let temp = TempDir::new().unwrap(); + let fixer = Fixer::new(temp.path().to_path_buf(), false); + let issue = make_issue("TEST-006"); + let fix = Fix { + action: FixAction::Modify, + target: "nonexistent.txt".to_string(), + reason: None, + modification: Some("replace-line:1:test".to_string()), + fallback: None, + }; + + let result = fixer.apply(&issue, &fix).unwrap(); + assert!(!result.success); + assert!(result.error.unwrap().contains("non-existent")); + } + + #[test] + fn test_delete_removes_file() { + let temp = TempDir::new().unwrap(); + let file_path = temp.path().join("to_delete.txt"); + std::fs::write(&file_path, "content").unwrap(); + assert!(file_path.exists()); + + let fixer = Fixer::new(temp.path().to_path_buf(), false); + let issue = make_issue("TEST-007"); + let fix = make_fix(FixAction::Delete, "to_delete.txt"); + + let result = fixer.apply(&issue, &fix).unwrap(); + assert!(result.success); + assert!(!file_path.exists()); + } + + #[test] + fn test_parse_modification_specs() { + // Test replace-line + let spec = Fixer::parse_modification("replace-line:5:new content").unwrap(); + assert!(matches!(spec, ModifySpec::ReplaceLine { line: 5, .. })); + + // Test insert-before + let spec = Fixer::parse_modification("insert-before:1:header").unwrap(); + assert!(matches!(spec, ModifySpec::InsertBefore { line: 1, .. })); + + // Test insert-after + let spec = Fixer::parse_modification("insert-after:10:footer").unwrap(); + assert!(matches!(spec, ModifySpec::InsertAfter { line: 10, .. })); + + // Test replace-pattern + let spec = Fixer::parse_modification("replace-pattern:old:new").unwrap(); + assert!(matches!(spec, ModifySpec::ReplacePattern { .. })); + + // Test prepend + let spec = Fixer::parse_modification("prepend:header line").unwrap(); + assert!(matches!(spec, ModifySpec::Prepend { .. })); + + // Test append + let spec = Fixer::parse_modification("append:footer line").unwrap(); + assert!(matches!(spec, ModifySpec::Append { .. })); + + // Test invalid + assert!(Fixer::parse_modification("invalid-spec").is_err()); + } +} diff --git a/robot-repo-automaton/src/hypatia.rs b/robot-repo-automaton/src/hypatia.rs index e0ab4ba8..0e2b8781 100644 --- a/robot-repo-automaton/src/hypatia.rs +++ b/robot-repo-automaton/src/hypatia.rs @@ -619,13 +619,16 @@ fn recipe_to_rule(recipe: &serde_json::Value) -> Option { // Build pattern from recipe detection info let pattern = if let Some(glob) = recipe.get("file_glob").and_then(|v| v.as_str()) { RulePattern::FileGlob { glob: glob.to_string() } - } else if let Some(regex) = recipe.get("pattern").and_then(|v| v.as_str()) { + } else { + // No file_glob: a content regex is then mandatory -- `?` returns None + // for a recipe that declares neither, which is the same contract the + // old explicit `else { return None; }` had. Written with `?` because + // clippy::question_mark is deny-level under `-Dwarnings`. + let regex = recipe.get("pattern").and_then(|v| v.as_str())?; RulePattern::ContentRegex { regex: regex.to_string(), file_glob: recipe.get("applies_to").and_then(|v| v.as_str()).map(|s| s.to_string()), } - } else { - return None; }; // Build fix from recipe diff --git a/robot-repo-automaton/src/main.rs b/robot-repo-automaton/src/main.rs index 18077b07..bdf9acbd 100644 --- a/robot-repo-automaton/src/main.rs +++ b/robot-repo-automaton/src/main.rs @@ -742,6 +742,19 @@ fn cmd_catalog(path: &Path, severity_filter: Option<&str>) -> anyhow::Result<()> Ok(()) } +/// Base directory holding local repo checkouts. +/// +/// Override with `REPOS_BASE`; otherwise defaults to the canonical estate tree. +fn repos_base() -> PathBuf { + if let Ok(base) = std::env::var("REPOS_BASE") { + return PathBuf::from(base); + } + dirs::home_dir() + .unwrap_or_else(|| PathBuf::from(".")) + .join("developer") + .join("hyper-repos") +} + /// Resolve a repo argument to a local path. /// /// Accepts either a local path or a GitHub owner/name format. @@ -751,15 +764,15 @@ fn resolve_repo_path(repo: &str) -> anyhow::Result { return Ok(path); } - // Try as a relative path from common locations - let eclipse_path = PathBuf::from("/var$REPOS_DIR").join(repo); - if eclipse_path.exists() { - return Ok(eclipse_path); + // Try as a relative path under the repos base + let candidate = repos_base().join(repo); + if candidate.exists() { + return Ok(candidate); } Err(anyhow::anyhow!( - "Repository not found: {} (tried local path and /var$REPOS_DIR/{})", + "Repository not found: {} (tried local path and {})", repo, - repo + candidate.display() )) } diff --git a/scripts/dispatch-runner.sh b/scripts/dispatch-runner.sh index 840ecd75..3e2a7895 100755 --- a/scripts/dispatch-runner.sh +++ b/scripts/dispatch-runner.sh @@ -49,13 +49,20 @@ validate_path_within() { } # --- Configuration --- +# This repo's own root, resolved relative to this script so the fleet never +# reaches into a different checkout of itself. +FLEET_ROOT="${FLEET_ROOT:-$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)}" + +# Base directory holding local repo checkouts. Override with REPOS_BASE. +# This value is also the containment boundary enforced by validate_path_within. +REPOS_BASE="${REPOS_BASE:-$HOME/developer/hyper-repos}" + # Hypatia's local data store is the primary source for dispatch manifests. -# Falls back to central verisim-data if HYPATIA_DATA is not set. -HYPATIA_DATA="${HYPATIA_DATA:-/var$REPOS_DIR/nextgen-databases/verisim/verisim-data}" -VERISIMDB_DATA="${VERISIMDB_DATA:-/var$REPOS_DIR/nextgen-databases/verisim/verisim-data}" -REPOS_BASE="${REPOS_BASE:-/var$REPOS_DIR}" -FLEET_SCRIPTS="${FLEET_SCRIPTS:-/var$REPOS_DIR/gitbot-fleet/scripts}" -RRA_BIN="${RRA_BIN:-/var$REPOS_DIR/gitbot-fleet/robot-repo-automaton/target/release/robot-repo-automaton}" +# Falls back to central verisimdb-data if HYPATIA_DATA is not set. +HYPATIA_DATA="${HYPATIA_DATA:-$REPOS_BASE/verisimdb-data}" +VERISIMDB_DATA="${VERISIMDB_DATA:-$REPOS_BASE/verisimdb-data}" +FLEET_SCRIPTS="${FLEET_SCRIPTS:-$FLEET_ROOT/scripts}" +RRA_BIN="${RRA_BIN:-$FLEET_ROOT/robot-repo-automaton/target/release/robot-repo-automaton}" # Third-party subdirectories inside monorepos that must NOT be modified. # Fix scripts will skip these paths entirely. @@ -312,8 +319,8 @@ execute_entry() { if [[ -f "$overrides" ]]; then local override override=$(jq -r --arg r "$repo" '.[$r] // empty' "$overrides" 2>/dev/null || true) - if [[ -n "$override" && -d "$override" ]]; then - repo_path="$override" + if [[ -n "$override" && -d "$REPOS_BASE/$override" ]]; then + repo_path="$REPOS_BASE/$override" fi fi fi diff --git a/scripts/enroll-hypatia-fleet.sh b/scripts/enroll-hypatia-fleet.sh index bbe2c290..c7052117 100755 --- a/scripts/enroll-hypatia-fleet.sh +++ b/scripts/enroll-hypatia-fleet.sh @@ -2,6 +2,7 @@ # SPDX-License-Identifier: MPL-2.0 set -euo pipefail +REPOS_BASE="${REPOS_BASE:-$HOME/developer/hyper-repos}" usage() { cat < Root containing repos (default: /var$REPOS_DIR) + --repos-root Root containing repos (default: $REPOS_BASE) --registry Registry JSON output (default: shared-context/enrollment/repos.json) --apply Write enrollment directives into discovered repos @@ -18,7 +19,7 @@ Options: USAGE } -repos_root="/var$REPOS_DIR" +repos_root="$REPOS_BASE" registry="" apply=false diff --git a/scripts/fix-license-hygiene.sh b/scripts/fix-license-hygiene.sh index ea8f06f9..43528233 100755 --- a/scripts/fix-license-hygiene.sh +++ b/scripts/fix-license-hygiene.sh @@ -12,9 +12,10 @@ # Idempotent: only modifies what's missing or incorrect. # # Requires canonical templates at: -# /var$REPOS_DIR/palimpsest-license/legal/MPL-2.0.txt -# /var$REPOS_DIR/palimpsest-license/legal/PALIMPSEST-MPL-1.0.txt +# $REPOS_BASE/palimpsest-license/legal/MPL-2.0.txt +# $REPOS_BASE/palimpsest-license/legal/PALIMPSEST-MPL-1.0.txt set -euo pipefail +REPOS_BASE="${REPOS_BASE:-$HOME/developer/hyper-repos}" echo "REFUSED: fix-license-hygiene.sh is disabled per estate policy 2026-06-02." >&2 echo " Licence/SPDX edits MUST be manual, per-file, owner-approved." >&2 @@ -48,13 +49,13 @@ fi cd "$REPO_DIR" # Template sources -PALIMPSEST_REPO="/var$REPOS_DIR/palimpsest-license" +PALIMPSEST_REPO="$REPOS_BASE/palimpsest-license" MPL2_SRC="$PALIMPSEST_REPO/legal/MPL-2.0.txt" PMPL_SRC="$PALIMPSEST_REPO/legal/PALIMPSEST-MPL-1.0.txt" # Fallback to boj-server if palimpsest-license not available if [[ ! -f "$MPL2_SRC" ]]; then - MPL2_SRC="/var$REPOS_DIR/boj-server/LICENSE" + MPL2_SRC="$REPOS_BASE/boj-server/LICENSE" fi changes=false diff --git a/scripts/fix-proven-substitute.sh b/scripts/fix-proven-substitute.sh index a48f245f..7afc99c6 100755 --- a/scripts/fix-proven-substitute.sh +++ b/scripts/fix-proven-substitute.sh @@ -9,6 +9,7 @@ # Usage: fix-proven-substitute.sh set -euo pipefail +REPOS_BASE="${REPOS_BASE:-$HOME/developer/hyper-repos}" REPO_PATH="${1:?Usage: $0 }" FINDING_JSON="${2:?Missing finding JSON file}" @@ -28,7 +29,7 @@ if [[ ! "$LANGUAGE" =~ ^[a-z]+$ ]]; then exit 1 fi -PROVEN_BINDINGS_BASE="/var$REPOS_DIR/proven/bindings" +PROVEN_BINDINGS_BASE="$REPOS_BASE/proven/bindings" # Extract finding details FILE=$(jq -r '.file // .location // "unknown"' "$FINDING_JSON") @@ -70,7 +71,7 @@ case "$LANGUAGE" in echo "" echo "CARGO.TOML DEPENDENCY:" echo " [dependencies]" - echo " proven = { path = \"/var$REPOS_DIR/proven/bindings/rust\" }" + echo " proven = { path = \"$REPOS_BASE/proven/bindings/rust\" }" ;; elixir) @@ -79,7 +80,7 @@ case "$LANGUAGE" in echo " alias Proven.${PROVEN_MODULE}" echo "" echo "MIX.EXS DEPENDENCY:" - echo " {:proven, path: \"/var$REPOS_DIR/proven/bindings/elixir\"}" + echo " {:proven, path: \"$REPOS_BASE/proven/bindings/elixir\"}" ;; affinescript) @@ -95,7 +96,7 @@ case "$LANGUAGE" in echo "" echo "SHELL SUBSTITUTION:" echo " Source the proven shell wrapper:" - echo " . /var$REPOS_DIR/proven/bindings/bash/proven.sh" + echo " . $REPOS_BASE/proven/bindings/bash/proven.sh" echo " ${PROVEN_MODULE}_call \"\$@\"" ;; diff --git a/scripts/list-supervised-repos.sh b/scripts/list-supervised-repos.sh index 2104a3ac..2e03d741 100755 --- a/scripts/list-supervised-repos.sh +++ b/scripts/list-supervised-repos.sh @@ -13,7 +13,7 @@ set -euo pipefail SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" FLEET_DIR="$(cd "${SCRIPT_DIR}/.." && pwd)" -DEFAULT_REPOS_ROOT="/var/mnt/eclipse/repos" +DEFAULT_REPOS_ROOT="${REPOS_BASE:-$HOME/developer/hyper-repos}" REPOS_ROOT="${REPOS_ROOT:-$DEFAULT_REPOS_ROOT}" LIMIT=0 INVENTORY_FILE="${FLEET_SUPERVISED_REPOS_FILE:-}" @@ -71,7 +71,7 @@ resolve_repo_path() { # Expand placeholders sometimes used in generated enrollment metadata. candidate="${candidate//\/var\$REPOS_DIR/$REPOS_ROOT}" - candidate="${candidate//\$REPOS_DIR/${REPOS_ROOT#/var/}}" + candidate="${candidate//\$REPOS_DIR/$REPOS_ROOT}" if [[ "$candidate" != /* ]]; then candidate="$REPOS_ROOT/$candidate" diff --git a/scripts/maintenance-hard-pass.sh b/scripts/maintenance-hard-pass.sh index 62d94e57..60fc577e 100755 --- a/scripts/maintenance-hard-pass.sh +++ b/scripts/maintenance-hard-pass.sh @@ -2,6 +2,7 @@ # SPDX-License-Identifier: MPL-2.0 set -euo pipefail +REPOS_BASE="${REPOS_BASE:-$HOME/developer/hyper-repos}" usage() { cat <&2 echo "expected one of:" >&2 echo " $repo/scripts/maintenance/run-maintenance.sh" >&2 echo " $repo/run-maintenance.sh" >&2 - echo " /var$REPOS_DIR/run-maintenance.sh" >&2 + echo " $REPOS_BASE/run-maintenance.sh" >&2 exit 2 fi -if [[ -z "$panic_bin" && -x "/var$REPOS_DIR/panic-attacker/target/release/panic-attack" ]]; then - panic_bin="/var$REPOS_DIR/panic-attacker/target/release/panic-attack" +if [[ -z "$panic_bin" && -x "$REPOS_BASE/panic-attack/target/release/panic-attack" ]]; then + panic_bin="$REPOS_BASE/panic-attack/target/release/panic-attack" fi cmd=("$runner" --repo "$repo" --output "$output" --strict --fail-on-warn) diff --git a/scripts/process-review-findings.sh b/scripts/process-review-findings.sh index 9e1eef42..0653f311 100755 --- a/scripts/process-review-findings.sh +++ b/scripts/process-review-findings.sh @@ -21,7 +21,7 @@ set -euo pipefail -FLEET_BASE="${FLEET_BASE:-/var$REPOS_DIR/gitbot-fleet}" +FLEET_BASE="${FLEET_BASE:-$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)}" PENDING_DIR="${FLEET_BASE}/shared-context/findings/pending" GH_OWNER="hyperpolymath" diff --git a/scripts/repo-path-overrides.json b/scripts/repo-path-overrides.json index 1eadb1a5..f037b03d 100644 --- a/scripts/repo-path-overrides.json +++ b/scripts/repo-path-overrides.json @@ -1,98 +1,37 @@ { - "absolute-zero": "/var$REPOS_DIR/maa-framework/absolute-zero", - "accessibilitybot": "/var$REPOS_DIR/gitbot-fleet/bots/accessibilitybot", - "affinescript": "/var$REPOS_DIR/nextgen-languages/affinescript", - "aggregate-library": "/var$REPOS_DIR/developer-ecosystem/aggregate-library", - "aletheia": "/var$REPOS_DIR/maa-framework/aletheia", - "algorithm-shield": "/var$REPOS_DIR/misinformation-defence-platform/algorithm-shield", - "asdf-augmenters": "/var$REPOS_DIR/asdf-tool-plugins/asdf-augmenters", - "avow-protocol": "/var$REPOS_DIR/standards/avow-protocol", - "axel-protocol": "/var$REPOS_DIR/standards/axel-protocol", - "Axiom.jl": "/var$REPOS_DIR/developer-ecosystem/julia-ecosystem/packages/Axiom.jl", - "betlang": "/var$REPOS_DIR/nextgen-languages/betlang", - "bitfuckit": "/var$REPOS_DIR/reposystem/bitfuckit", - "blue-screen-of-app": "/var$REPOS_DIR/games & trivia/blue-screen-of-app", - "BowtieRisk.jl": "/var$REPOS_DIR/developer-ecosystem/julia-ecosystem/packages/BowtieRisk.jl", - "broad-spectrum": "/var$REPOS_DIR/ambientops/broad-spectrum", - "cadre-router": "/var$REPOS_DIR/developer-ecosystem/rescript-ecosystem/cadre-router", - "candy-crash": "/var$REPOS_DIR/games & trivia/candy-crash", - "casket-ssg": "/var$REPOS_DIR/asdf-tool-plugins/asdf-plugin-collection/plugins/casket-ssg", - "cerro-torre": "/var$REPOS_DIR/odds-and-sods-package-manager/services/cerro-torre", - "cipherbot": "/var$REPOS_DIR/gitbot-fleet/bots/cipherbot", - "claim-forge": "/var$REPOS_DIR/reposystem/claim-forge", - "claude-integrations": "/var$REPOS_DIR/patallm-gallery/claude-integrations", - "coq-jr": "/var$REPOS_DIR/developer-ecosystem/coq-ecosystem/coq-jr", - "czech-file-knife": "/var$REPOS_DIR/ambientops/czech-file-knife", - "deno-ecosystem": "/var$REPOS_DIR/developer-ecosystem/deno-ecosystem", - "dicti0nary-attack": "/var$REPOS_DIR/games & trivia/dicti0nary-attack", - "did-you-actually-do-that": "/var$REPOS_DIR/patallm-gallery/did-you-actually-do-that", - "disinfo-nesy-detector": "/var$REPOS_DIR/neural-foundations/satellites/neurosymbolic/disinfo-nesy-detector", - "dnfinition": "/var$REPOS_DIR/ambientops/total-update/elixir/dnfinition", - "echidnabot": "/var$REPOS_DIR/echidna/echidnabot", - "eclexia": "/var$REPOS_DIR/nextgen-languages/eclexia", - "elegant-state": "/var$REPOS_DIR/neural-foundations/satellites/agentic/elegant-state", - "error-lang": "/var$REPOS_DIR/nextgen-languages/error-lang", - "esn": "/var$REPOS_DIR/neural-foundations/satellites/neurosymbolic/esn", - "finishingbot": "/var$REPOS_DIR/gitbot-fleet/bots/finishingbot", - "fogbinder": "/var$REPOS_DIR/zotero-tools/fogbinder", - "formdb-http": "/var$REPOS_DIR/nextgen-databases/lithoglyph/formdb-http", - "games": "/var$REPOS_DIR/games & trivia", - "glambot": "/var$REPOS_DIR/gitbot-fleet/bots/glambot", - "glyphbase": "/var$REPOS_DIR/nextgen-databases/lithoglyph/glyphbase", - "gql-dt": "/var$REPOS_DIR/nextgen-databases/lithoglyph/gql-dt", - "hybrid-automation-router": "/var$REPOS_DIR/ambientops/hybrid-automation-router", - "IDApixiTIK": "/var$REPOS_DIR/idaptik", - "idris2-ecosystem": "/var$REPOS_DIR/developer-ecosystem/idris2-ecosystem", - "immutable-linux-auditor": "/var$REPOS_DIR/ambientops/immutable-linux-auditor", - "indieweb2-bastion": "/var$REPOS_DIR/civic-connect/indieweb2-bastion", - "julia-the-viper": "/var$REPOS_DIR/nextgen-languages/julia-the-viper", - "k9-svc": "/var$REPOS_DIR/standards/k9-svc", - "kea-tools": "/var$REPOS_DIR/kea/kea-tools", - "kith": "/var$REPOS_DIR/developer-ecosystem/well-known-ecosystem/kith", - "language-bridges": "/var$REPOS_DIR/nextgen-languages/language-bridges", - "language-interop-compiler": "/var$REPOS_DIR/nextgen-languages/language-interop-compiler", - "lithoglyph": "/var$REPOS_DIR/nextgen-databases/lithoglyph", - "llm-tools": "/var$REPOS_DIR/patallm-gallery/llm-tools", - "lol": "/var$REPOS_DIR/standards/lol", - "lsm": "/var$REPOS_DIR/neural-foundations/satellites/neurosymbolic/lsm", - "mcp-repo-guardian": "/var$REPOS_DIR/standards/0-ai-gatekeeper-protocol/mcp-repo-guardian", - "my-lang": "/var$REPOS_DIR/nextgen-languages/my-lang", - "nerdsafe-restart": "/var$REPOS_DIR/ambientops/nerdsafe-restart", - "nick-shells": "/var$REPOS_DIR/ambientops/nick-shells", - "oblibeny": "/var$REPOS_DIR/nextgen-languages/oblibeny", - "package-publishers": "/var$REPOS_DIR/developer-ecosystem/package-publishers", - "_pathroot": "/var$REPOS_DIR/ambientops/_pathroot", - "personal-sysadmin": "/var$REPOS_DIR/ambientops/personal-sysadmin", - "phantom-metal-taste": "/var$REPOS_DIR/games & trivia/phantom-metal-taste", - "phronesis": "/var$REPOS_DIR/nextgen-languages/phronesis", - "poly-k8s-mcp": "/var$REPOS_DIR/flatracoon/netstack/modules/poly-k8s-mcp", - "poly-secret-mcp": "/var$REPOS_DIR/flatracoon/netstack/modules/poly-secret-mcp", - "ProvenCrypto.jl": "/var$REPOS_DIR/developer-ecosystem/julia-ecosystem/packages/ProvenCrypto.jl", - "qubes-sdp": "/var$REPOS_DIR/aerie/qubes-sdp", - "reasonably-good-token-vault": "/var$REPOS_DIR/ambientops/reasonably-good-token-vault", - "recon-silly-ation": "/var$REPOS_DIR/developer-ecosystem/satellites/developer-ux/recon-silly-ation", - "repo-batcher": "/var$REPOS_DIR/reposystem/scaffoldia/repo-batcher", - "repo-guardian-fs": "/var$REPOS_DIR/standards/0-ai-gatekeeper-protocol/repo-guardian-fs", - "rescript-ecosystem": "/var$REPOS_DIR/developer-ecosystem/rescript-ecosystem", - "rhodibot": "/var$REPOS_DIR/gitbot-fleet/bots/rhodibot", - "rhodium-standard-repositories": "/var$REPOS_DIR/standards/rhodium-standard-repositories", - "robot-repo-automaton": "/var$REPOS_DIR/developer-ecosystem/satellites/repo-management/robot-repo-automaton", - "safe-brute-force": "/var$REPOS_DIR/games & trivia/safe-brute-force", - "scaffoldia": "/var$REPOS_DIR/reposystem/scaffoldia", - "seambot": "/var$REPOS_DIR/gitbot-fleet/bots/seambot", - "selur": "/var$REPOS_DIR/odds-and-sods-package-manager/services/selur", - "SMTLib.jl": "/var$REPOS_DIR/developer-ecosystem/julia-ecosystem/packages/SMTLib.jl", - "sustainabot": "/var$REPOS_DIR/gitbot-fleet/bots/sustainabot", - "svalinn": "/var$REPOS_DIR/project-wharf/infra/svalinn", - "system-tools": "/var$REPOS_DIR/ambientops/system-tools", - "test-repo": "/var$REPOS_DIR/hypatia/integration/fixtures/test-repo", - "hotchocolabot": "/var$REPOS_DIR/hotchocolabot", - "thejeffparadox": "/var$REPOS_DIR/games & trivia/thejeffparadox", - "total-update": "/var$REPOS_DIR/ambientops/total-update", - "union-policy-parser": "/var$REPOS_DIR/palimpsest-plasma/union-policy-parser", - "verified-container-spec": "/var$REPOS_DIR/stapeln/verified-container-spec", - "verisimdb": "/var$REPOS_DIR/nextgen-databases/verisimdb", - "vordr": "/var$REPOS_DIR/stapeln/container-stack/vordr", - "well-known-ecosystem": "/var$REPOS_DIR/developer-ecosystem/well-known-ecosystem", - "zig-ffi": "/var$REPOS_DIR/developer-ecosystem/rescript-ecosystem/packages/ffi/zig-ffi" + "accessibilitybot": "gitbot-fleet/bots/accessibilitybot", + "aletheia": "maa-framework/aletheia", + "algorithm-shield": "misinformation-defence-platform/algorithm-shield", + "asdf-augmenters": "asdf-tool-plugins/asdf-augmenters", + "avow-protocol": "standards/avow-protocol", + "axel-protocol": "standards/axel-protocol", + "broad-spectrum": "ambientops/broad-spectrum", + "cadre-router": "developer-ecosystem/rescript-ecosystem/cadre-router", + "casket-ssg": "asdf-tool-plugins/asdf-plugin-collection/plugins/casket-ssg", + "cipherbot": "gitbot-fleet/bots/cipherbot", + "czech-file-knife": "ambientops/czech-file-knife", + "deno-ecosystem": "developer-ecosystem/deno-ecosystem", + "did-you-actually-do-that": "patallm-gallery/did-you-actually-do-that", + "dnfinition": "ambientops/total-update/elixir/dnfinition", + "finishingbot": "gitbot-fleet/bots/finishingbot", + "glambot": "gitbot-fleet/bots/glambot", + "idris2-ecosystem": "developer-ecosystem/idris2-ecosystem", + "immutable-linux-auditor": "ambientops/immutable-linux-auditor", + "indieweb2-bastion": "civic-connect/indieweb2-bastion", + "k9-svc": "standards/k9-svc", + "kith": "developer-ecosystem/well-known-ecosystem/kith", + "llm-tools": "patallm-gallery/llm-tools", + "nerdsafe-restart": "ambientops/nerdsafe-restart", + "nick-shells": "ambientops/nick-shells", + "personal-sysadmin": "ambientops/personal-sysadmin", + "rhodibot": "gitbot-fleet/bots/rhodibot", + "rhodium-standard-repositories": "standards/rhodium-standard-repositories", + "seambot": "gitbot-fleet/bots/seambot", + "selur": "odds-and-sods-package-manager/services/selur", + "sustainabot": "gitbot-fleet/bots/sustainabot", + "system-tools": "ambientops/system-tools", + "test-repo": "hypatia/integration/fixtures/test-repo", + "total-update": "ambientops/total-update", + "well-known-ecosystem": "developer-ecosystem/well-known-ecosystem", + "zig-ffi": "developer-ecosystem/rescript-ecosystem/packages/ffi/zig-ffi" } diff --git a/scripts/sync-all-parallel.exs b/scripts/sync-all-parallel.exs index d115fc06..a112811e 100644 --- a/scripts/sync-all-parallel.exs +++ b/scripts/sync-all-parallel.exs @@ -10,7 +10,7 @@ # elixir sync-all-parallel.exs [OPTIONS] # # Options: -# --repos-dir PATH Base directory (default: /var$REPOS_DIR) +# --repos-dir PATH Base directory (default: $REPOS_BASE or ~/developer/hyper-repos) # --dry-run Show what would happen # --auto Non-interactive mode (skip all issues) # --concurrency N Max concurrent git operations (default: 32) @@ -140,9 +140,13 @@ defmodule SyncAll do # --- Argument Parsing --- + defp default_repos_dir do + System.get_env("REPOS_BASE") || Path.join(System.user_home!(), "developer/hyper-repos") + end + defp parse_args(args) do parse_args(args, %SyncAll{ - repos_dir: "/var$REPOS_DIR", + repos_dir: default_repos_dir(), dry_run: false, auto_mode: false, concurrency: 32, diff --git a/shared-context/enrollment/README.adoc b/shared-context/enrollment/README.adoc index fbdc60de..97515e37 100644 --- a/shared-context/enrollment/README.adoc +++ b/shared-context/enrollment/README.adoc @@ -14,7 +14,7 @@ just enroll-repos [source,bash] ---- -just enroll-repos /var$REPOS_DIR true +just enroll-repos ~/developer/hyper-repos true ---- This writes `+.machine_readable/bot_directives/FLEET-ENROLLMENT.a2ml+` From 6b9c9b1f16573c0dec13bec3e72ef88516bfa541 Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Fri, 4 Sep 2026 17:45:18 +0100 Subject: [PATCH 2/6] Update scripts/fix-proven-substitute.sh Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Jonathan D.A. Jewell <6759885+hyperpolymath@users.noreply.github.com> --- scripts/fix-proven-substitute.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/fix-proven-substitute.sh b/scripts/fix-proven-substitute.sh index 7afc99c6..508bea08 100755 --- a/scripts/fix-proven-substitute.sh +++ b/scripts/fix-proven-substitute.sh @@ -96,7 +96,7 @@ case "$LANGUAGE" in echo "" echo "SHELL SUBSTITUTION:" echo " Source the proven shell wrapper:" - echo " . $REPOS_BASE/proven/bindings/bash/proven.sh" + echo " . \"$REPOS_BASE/proven/bindings/bash/proven.sh\"" echo " ${PROVEN_MODULE}_call \"\$@\"" ;; From 2b9be5fc753a47ea29d01b331362a7f00da470e7 Mon Sep 17 00:00:00 2001 From: "coderabbitai[bot]" <136622811+coderabbitai[bot]@users.noreply.github.com> Date: Fri, 4 Sep 2026 17:03:44 +0000 Subject: [PATCH 3/6] Fix CodeRabbit issues in PR #507 --- Justfile | 5 +- README.adoc | 2 +- docs/wiki-source/Build-and-Run.md | 2 +- robot-repo-automaton/Cargo.lock | 21 ++ robot-repo-automaton/Cargo.toml | 6 +- robot-repo-automaton/src/fixer.rs | 453 ++++++++++++++++++++++++-- robot-repo-automaton/src/main.rs | 4 +- scripts/sync-all-parallel.exs | 5 +- shared-context/enrollment/README.adoc | 2 +- 9 files changed, 464 insertions(+), 36 deletions(-) diff --git a/Justfile b/Justfile index 7bedcd96..6e0c509a 100644 --- a/Justfile +++ b/Justfile @@ -7,7 +7,8 @@ set shell := ["bash", "-euo", "pipefail", "-c"] # Base directory holding local repo checkouts; override with REPOS_BASE. -repos_base := env_var_or_default("REPOS_BASE", env_var("HOME") / "developer/hyper-repos") +repos_base_env := env_var_or_default("REPOS_BASE", "") +repos_base := if repos_base_env == "" { env_var("HOME") / "developer/hyper-repos" } else { repos_base_env } # Default recipe: show help import? "contractile.just" @@ -63,7 +64,7 @@ hypatia-scan: # Run panic-attack static analysis panic-scan: @if [ -x "{{repos_base}}/panic-attack/target/release/panic-attack" ]; then \ - {{repos_base}}/panic-attack/target/release/panic-attack assail . --verbose; \ + "{{repos_base}}/panic-attack/target/release/panic-attack" assail . --verbose; \ else \ echo "panic-attack not built — run 'cd {{repos_base}}/panic-attack && cargo build --release'"; \ fi diff --git a/README.adoc b/README.adoc index 5b17d7ba..01f091af 100644 --- a/README.adoc +++ b/README.adoc @@ -111,7 +111,7 @@ just scan-supervised `maintenance-hard-pass` enforces fail-on-warn release gating using the target repo's maintenance script. `enroll-repos` refreshes repository -coverage metadata; pass `~/developer/hyper-repos true` to also write enrollment +coverage metadata; pass `"${REPOS_BASE:-$HOME/developer/hyper-repos}" true` to also write enrollment directives into repos that already have `.machine_readable/`. `scan-supervised` runs Hypatia across the supervised inventory (`~/.git-private-farm.scm`, `~/.git-private-repos`, or the enrollment diff --git a/docs/wiki-source/Build-and-Run.md b/docs/wiki-source/Build-and-Run.md index 99396cff..7cd93ece 100644 --- a/docs/wiki-source/Build-and-Run.md +++ b/docs/wiki-source/Build-and-Run.md @@ -22,7 +22,7 @@ just scan-supervised - **`maintenance-hard-pass`** enforces fail-on-warn release gating using the target repo's maintenance script. - **`enroll-repos`** refreshes repository coverage metadata. Pass - `~/developer/hyper-repos true` to write enrollment directives into repos that + `"${REPOS_BASE:-$HOME/developer/hyper-repos}" true` to write enrollment directives into repos that already have `.machine_readable/`. - **`scan-supervised`** runs Hypatia across the supervised inventory (`~/.git-private-farm.scm`, `~/.git-private-repos`, or the enrollment diff --git a/robot-repo-automaton/Cargo.lock b/robot-repo-automaton/Cargo.lock index d8393b6e..a02fb92c 100644 --- a/robot-repo-automaton/Cargo.lock +++ b/robot-repo-automaton/Cargo.lock @@ -2883,9 +2883,11 @@ dependencies = [ "lexpr", "regex", "reqwest 0.12.28", + "rustix", "secrecy", "serde", "serde_json", + "serde_yaml_ng", "tempfile", "thiserror 2.0.18", "tokio", @@ -3145,6 +3147,19 @@ dependencies = [ "serde", ] +[[package]] +name = "serde_yaml_ng" +version = "0.10.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7b4db627b98b36d4203a7b458cf3573730f2bb591b28871d916dfa9efabfd41f" +dependencies = [ + "indexmap", + "itoa", + "ryu", + "serde", + "unsafe-libyaml", +] + [[package]] name = "sha1" version = "0.10.6" @@ -3680,6 +3695,12 @@ version = "0.2.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ebc1c04c71510c7f702b52b7c350734c9ff1295c464a03335b00bb84fc54f853" +[[package]] +name = "unsafe-libyaml" +version = "0.2.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "673aac59facbab8a9007c7f6108d11f63b603f7cabff99fabf650fea5c32b861" + [[package]] name = "untrusted" version = "0.9.0" diff --git a/robot-repo-automaton/Cargo.toml b/robot-repo-automaton/Cargo.toml index 23e6aae2..b4eac3cb 100644 --- a/robot-repo-automaton/Cargo.toml +++ b/robot-repo-automaton/Cargo.toml @@ -42,6 +42,7 @@ reqwest = { version = "0.12.28", features = ["json", "rustls-tls"], default-feat # Serialization serde = { version = "1.0.228", features = ["derive"] } serde_json = "1.0.150" +serde_yaml_ng = "0.10.0" toml = "1.1.2" # Git operations @@ -65,6 +66,7 @@ glob = "0.3.3" walkdir = "2.5.0" regex = "1.12.3" dirs = "6.0.0" +tempfile = "3.27.0" # Async utilities futures = "0.3.32" @@ -78,8 +80,10 @@ uuid = { version = "1.23.2", features = ["v4", "serde"] } # Secrets secrecy = { version = "0.10.3", features = ["serde"] } +[target.'cfg(target_os = "linux")'.dependencies] +rustix = { version = "1.1.4", features = ["fs"] } + [dev-dependencies] -tempfile = "3.27.0" tokio-test = "0.4.5" wiremock = "0.6.5" diff --git a/robot-repo-automaton/src/fixer.rs b/robot-repo-automaton/src/fixer.rs index 7b55b274..8905a7d9 100644 --- a/robot-repo-automaton/src/fixer.rs +++ b/robot-repo-automaton/src/fixer.rs @@ -9,6 +9,9 @@ use git2::{Repository, Signature}; use regex::Regex; +use std::ffi::OsString; +use std::fs::{self, OpenOptions}; +use std::io::{self, Write}; use std::path::{Path, PathBuf}; use tracing::{debug, info, warn}; @@ -96,16 +99,19 @@ impl Fixer { }); } - let target_path = self.repo_path.join(&fix.target); - // SECURITY: Reject any target path that escapes the repository root. - // Normalise both paths and verify the target is a child of repo_path. - // This prevents path traversal attacks (e.g. target = "../../etc/passwd"). - let canonical_repo = self.repo_path.canonicalize().unwrap_or_else(|_| self.repo_path.clone()); - // For the target we normalise without requiring the path to exist yet - // (it may be a to-be-created file), so we use a manual component walk. - let normalised_target = normalise_path(&target_path); - if !normalised_target.starts_with(&canonical_repo) { + // Canonicalise every existing component so symlinks cannot redirect a + // target outside the repository. The final file may not exist yet. + let canonical_repo = match self.repo_path.canonicalize() { + Ok(path) => path, + Err(error) => return Ok(Self::boundary_failure(issue, fix, &error.to_string())), + }; + let target_path = normalise_path(&canonical_repo.join(&fix.target)); + let resolved_target = match resolve_existing_path(&target_path) { + Ok(path) => path, + Err(error) => return Ok(Self::boundary_failure(issue, fix, &error.to_string())), + }; + if !resolved_target.starts_with(&canonical_repo) { warn!( target = %fix.target, repo = %self.repo_path.display(), @@ -128,18 +134,46 @@ impl Fixer { match fix.action { FixAction::Delete => self.apply_delete(&target_path, issue), - FixAction::Modify => self.apply_modify(&target_path, issue, fix), + FixAction::Modify => self.apply_modify(&resolved_target, issue, fix), FixAction::Create => self.apply_create(&target_path, issue, fix), FixAction::Disable => self.apply_disable(&target_path, issue), } } - /// Check if a file is binary based on its extension - fn is_binary(path: &Path) -> bool { - path.extension() + fn boundary_failure(issue: &DetectedIssue, fix: &Fix, detail: &str) -> FixResult { + warn!( + target = %fix.target, + detail, + "SECURITY: could not resolve fix target inside repository — rejecting" + ); + FixResult { + issue_id: issue.error_type_id.clone(), + success: false, + action_taken: format!( + "REJECTED: target '{}' could not be verified inside repository boundary", + fix.target + ), + files_modified: vec![], + error: Some(format!( + "Security violation: target path '{}' could not be verified inside the repository directory: {}", + fix.target, detail + )), + } + } + + /// Check if a file is binary based on its extension and bytes. + fn is_binary(path: &Path) -> io::Result { + let has_binary_extension = path + .extension() .and_then(|ext| ext.to_str()) .map(|ext| BINARY_EXTENSIONS.contains(&ext.to_lowercase().as_str())) - .unwrap_or(false) + .unwrap_or(false); + if has_binary_extension { + return Ok(true); + } + + let bytes = fs::read(path)?; + Ok(bytes.contains(&0) || std::str::from_utf8(&bytes).is_err()) } /// Parse a modification specification string into structured operations @@ -148,7 +182,8 @@ impl Fixer { /// - `replace-line::` - Replace line N with content /// - `insert-before::` - Insert content before line N /// - `insert-after::` - Insert content after line N - /// - `replace-pattern::` - Replace regex matches + /// - `replace-pattern::` - Replace regex matches. The + /// final unescaped colon separates the fields; use `\:` in a replacement. /// - `prepend:` - Add content at file beginning /// - `append:` - Add content at file end fn parse_modification(spec: &str) -> Result { @@ -192,12 +227,15 @@ impl Fixer { }) } Some("replace-pattern") => { - if parts.len() < 3 { - return Err(Error::Fix("replace-pattern requires pattern and replacement".into())); - } + let fields = spec + .strip_prefix("replace-pattern:") + .and_then(Self::split_replace_pattern) + .ok_or_else(|| { + Error::Fix("replace-pattern requires pattern and replacement".into()) + })?; Ok(ModifySpec::ReplacePattern { - pattern: parts[1].to_string(), - replacement: parts[2].to_string(), + pattern: Self::unescape_colons(fields.0), + replacement: Self::unescape_colons(fields.1), }) } Some("prepend") => { @@ -219,6 +257,34 @@ impl Fixer { } } + /// Split legacy replace-pattern fields without consuming colons in the regex. + /// Replacement colons must be escaped as `\:` to keep the representation + /// unambiguous. + fn split_replace_pattern(fields: &str) -> Option<(&str, &str)> { + fields.match_indices(':').rev().find_map(|(index, _)| { + let preceding_backslashes = fields[..index] + .chars() + .rev() + .take_while(|character| *character == '\\') + .count(); + (preceding_backslashes % 2 == 0).then(|| (&fields[..index], &fields[index + 1..])) + }) + } + + fn unescape_colons(value: &str) -> String { + let mut result = String::with_capacity(value.len()); + let mut characters = value.chars().peekable(); + while let Some(character) = characters.next() { + if character == '\\' && characters.peek() == Some(&':') { + characters.next(); + result.push(':'); + } else { + result.push(character); + } + } + result + } + /// Apply a modification specification to file content fn apply_modification(content: &str, spec: &ModifySpec) -> Result { let mut lines: Vec = content.lines().map(|l| l.to_string()).collect(); @@ -276,6 +342,44 @@ impl Fixer { Ok(result) } + /// Validate edited formats for which this crate has a parser available. + fn validate_content(path: &Path, content: &str) -> Result<()> { + let extension = path + .extension() + .and_then(|extension| extension.to_str()) + .map(str::to_ascii_lowercase); + + match extension.as_deref() { + Some("json") => { + serde_json::from_str::(content) + .map_err(|error| Error::Validation(error.to_string()))?; + } + Some("jsonl") => { + for (index, line) in content.lines().enumerate() { + if !line.trim().is_empty() { + serde_json::from_str::(line).map_err(|error| { + Error::Validation(format!("JSONL line {}: {}", index + 1, error)) + })?; + } + } + } + Some("yaml" | "yml") => { + serde_yaml_ng::from_str::(content) + .map_err(|error| Error::Validation(error.to_string()))?; + } + Some("toml") => { + toml::from_str::(content) + .map_err(|error| Error::Validation(error.to_string()))?; + } + Some("scm") => { + lexpr::from_str(content).map_err(|error| Error::Validation(error.to_string()))?; + } + _ => {} + } + + Ok(()) + } + /// Delete a file fn apply_delete( &self, @@ -336,7 +440,7 @@ impl Fixer { } // Safety: never modify binary files - if Self::is_binary(target_path) { + if Self::is_binary(target_path)? { warn!("Skipping binary file: {}", target_path.display()); return Ok(FixResult { issue_id: issue.error_type_id.clone(), @@ -407,10 +511,24 @@ impl Fixer { }); } - // Write modified content - if let Err(e) = std::fs::write(target_path, &new_content) { - // Attempt rollback on write failure - let _ = std::fs::write(target_path, &original_content); + if let Err(error) = Self::validate_content(target_path, &new_content) { + warn!( + "Modification produced invalid content for {}: {}", + target_path.display(), + error + ); + return Ok(FixResult { + issue_id: issue.error_type_id.clone(), + success: false, + action_taken: format!("Modification failed validation: {}", error), + files_modified: vec![], + error: Some(format!("Modification failed validation: {}", error)), + }); + } + + // Write to a temporary file in the same directory, then atomically + // replace the target. Any failure leaves the original file untouched. + if let Err(e) = atomic_replace(target_path, new_content.as_bytes()) { return Err(Error::Fix(format!( "Failed to write modified file {}: {}", target_path.display(), @@ -511,7 +629,28 @@ impl Fixer { }); } - std::fs::write(target_path, &expanded)?; + let mut output = match OpenOptions::new() + .write(true) + .create_new(true) + .open(target_path) + { + Ok(file) => file, + Err(error) if error.kind() == io::ErrorKind::AlreadyExists => { + return Ok(FixResult { + issue_id: issue.error_type_id.clone(), + success: true, + action_taken: "File already exists".to_string(), + files_modified: vec![], + error: None, + }); + } + Err(error) => return Err(error.into()), + }; + if let Err(error) = output.write_all(expanded.as_bytes()) { + drop(output); + let _ = fs::remove_file(target_path); + return Err(error.into()); + } info!("Created: {}", target_path.display()); Ok(FixResult { @@ -560,7 +699,7 @@ impl Fixer { }); } - std::fs::rename(target_path, &disabled_path)?; + rename_noreplace(target_path, &disabled_path)?; info!( "Disabled: {} -> {}", target_path.display(), @@ -583,7 +722,11 @@ impl Fixer { /// Check if a path would be gitignored fn would_be_gitignored(&self, path: &Path) -> bool { if let Ok(repo) = Repository::open(&self.repo_path) { - if let Ok(relative) = path.strip_prefix(&self.repo_path) { + if let Ok(repo_root) = self.repo_path.canonicalize() { + if let Ok(relative) = path.strip_prefix(repo_root) { + return repo.is_path_ignored(relative).unwrap_or(false); + } + } else if let Ok(relative) = path.strip_prefix(&self.repo_path) { return repo.is_path_ignored(relative).unwrap_or(false); } } @@ -704,6 +847,89 @@ impl Fixer { } } +/// Resolve all existing path components and append any missing suffix. +/// Canonicalisation errors on existing components are returned to the caller so +/// repository-boundary validation fails closed. +fn resolve_existing_path(path: &Path) -> io::Result { + let mut candidate = path.to_path_buf(); + let mut missing_components: Vec = Vec::new(); + + loop { + match fs::symlink_metadata(&candidate) { + Ok(_) => { + let mut resolved = candidate.canonicalize()?; + for component in missing_components.iter().rev() { + resolved.push(component); + } + return Ok(normalise_path(&resolved)); + } + Err(error) if error.kind() == io::ErrorKind::NotFound => { + let component = candidate.file_name().ok_or_else(|| { + io::Error::new( + io::ErrorKind::NotFound, + format!("no existing ancestor for {}", path.display()), + ) + })?; + missing_components.push(component.to_os_string()); + if !candidate.pop() { + return Err(error); + } + } + Err(error) => return Err(error), + } + } +} + +/// Replace a file atomically using a same-directory temporary file. +fn atomic_replace(target: &Path, content: &[u8]) -> io::Result<()> { + atomic_replace_with(target, content, |temporary, target| { + temporary + .persist(target) + .map(|_| ()) + .map_err(|error| error.error) + }) +} + +fn atomic_replace_with(target: &Path, content: &[u8], persist: F) -> io::Result<()> +where + F: FnOnce(tempfile::NamedTempFile, &Path) -> io::Result<()>, +{ + let parent = target.parent().ok_or_else(|| { + io::Error::new( + io::ErrorKind::InvalidInput, + format!("target has no parent directory: {}", target.display()), + ) + })?; + let permissions = fs::metadata(target)?.permissions(); + let mut temporary = tempfile::NamedTempFile::new_in(parent)?; + temporary.write_all(content)?; + temporary.as_file().set_permissions(permissions)?; + temporary.as_file_mut().sync_all()?; + persist(temporary, target) +} + +/// Atomically rename without replacing an existing destination. +#[cfg(target_os = "linux")] +fn rename_noreplace(source: &Path, destination: &Path) -> io::Result<()> { + rustix::fs::renameat_with( + rustix::fs::CWD, + source, + rustix::fs::CWD, + destination, + rustix::fs::RenameFlags::NOREPLACE, + ) + .map_err(io::Error::from) +} + +/// Fail closed on platforms without an atomic no-replace rename primitive. +#[cfg(not(target_os = "linux"))] +fn rename_noreplace(_source: &Path, _destination: &Path) -> io::Result<()> { + Err(io::Error::new( + io::ErrorKind::Unsupported, + "atomic no-replace rename is unsupported on this platform", + )) +} + /// Normalise a path by resolving `.` and `..` components without requiring the /// path to exist on disk (unlike `Path::canonicalize`). /// @@ -937,4 +1163,175 @@ mod tests { // Test invalid assert!(Fixer::parse_modification("invalid-spec").is_err()); } + + #[test] + fn test_replace_pattern_preserves_colons_in_regex() { + let spec = Fixer::parse_modification("replace-pattern:https?://old:new").unwrap(); + let ModifySpec::ReplacePattern { + pattern, + replacement, + } = &spec + else { + panic!("expected replace-pattern specification"); + }; + assert_eq!(pattern, "https?://old"); + assert_eq!(replacement, "new"); + assert_eq!( + Fixer::apply_modification("http://old https://old", &spec).unwrap(), + "new new" + ); + } + + #[test] + fn test_replace_pattern_supports_escaped_colon_in_replacement() { + let spec = Fixer::parse_modification(r"replace-pattern:old:new\:value").unwrap(); + let ModifySpec::ReplacePattern { replacement, .. } = spec else { + panic!("expected replace-pattern specification"); + }; + assert_eq!(replacement, "new:value"); + } + + #[test] + fn test_modify_nul_content_is_rejected_without_changes() { + let temp = TempDir::new().unwrap(); + let file_path = temp.path().join("data.txt"); + let original = b"text\0more text"; + fs::write(&file_path, original).unwrap(); + + let fixer = Fixer::new(temp.path().to_path_buf(), false); + let issue = make_issue("TEST-BINARY-CONTENT"); + let fix = Fix { + action: FixAction::Modify, + target: "data.txt".to_string(), + reason: None, + modification: Some("replace-line:1:changed".to_string()), + fallback: None, + }; + + let result = fixer.apply(&issue, &fix).unwrap(); + assert!(!result.success); + assert_eq!(fs::read(file_path).unwrap(), original); + } + + #[test] + fn test_invalid_json_modification_is_not_persisted() { + let temp = TempDir::new().unwrap(); + let file_path = temp.path().join("data.json"); + let original = "{\n \"valid\": true\n}\n"; + fs::write(&file_path, original).unwrap(); + + let fixer = Fixer::new(temp.path().to_path_buf(), false); + let issue = make_issue("TEST-VALIDATION"); + let fix = Fix { + action: FixAction::Modify, + target: "data.json".to_string(), + reason: None, + modification: Some("replace-line:2:not valid json".to_string()), + fallback: None, + }; + + let result = fixer.apply(&issue, &fix).unwrap(); + assert!(!result.success); + assert!(result.error.unwrap().contains("validation")); + assert_eq!(fs::read_to_string(file_path).unwrap(), original); + } + + #[test] + fn test_invalid_yaml_modification_is_not_persisted() { + let temp = TempDir::new().unwrap(); + let file_path = temp.path().join("workflow.yml"); + let original = "name: CI\njobs:\n check:\n runs-on: ubuntu-latest\n"; + fs::write(&file_path, original).unwrap(); + + let fixer = Fixer::new(temp.path().to_path_buf(), false); + let issue = make_issue("TEST-YAML-VALIDATION"); + let fix = Fix { + action: FixAction::Modify, + target: "workflow.yml".to_string(), + reason: None, + modification: Some("replace-line:4: runs-on: [unterminated".to_string()), + fallback: None, + }; + + let result = fixer.apply(&issue, &fix).unwrap(); + assert!(!result.success); + assert_eq!(fs::read_to_string(file_path).unwrap(), original); + } + + #[cfg(unix)] + #[test] + fn test_symlinked_create_target_outside_repo_is_rejected() { + use std::os::unix::fs::symlink; + + let repo = TempDir::new().unwrap(); + let outside = TempDir::new().unwrap(); + symlink(outside.path(), repo.path().join("escape")).unwrap(); + + let fixer = Fixer::new(repo.path().to_path_buf(), false); + let issue = make_issue("TEST-SYMLINK-ESCAPE"); + let fix = Fix { + action: FixAction::Create, + target: "escape/injected.txt".to_string(), + reason: None, + modification: None, + fallback: Some("must stay inside".to_string()), + }; + + let result = fixer.apply(&issue, &fix).unwrap(); + assert!(!result.success); + assert!(!outside.path().join("injected.txt").exists()); + } + + #[test] + fn test_relative_repository_path_is_accepted() { + let current = std::env::current_dir().unwrap(); + let repo = tempfile::Builder::new() + .prefix("fixer-relative-") + .tempdir_in(¤t) + .unwrap(); + let absolute_repo = repo.path().canonicalize().unwrap(); + let relative_repo = absolute_repo.strip_prefix(¤t).unwrap().to_path_buf(); + let file_path = absolute_repo.join("safe.txt"); + fs::write(&file_path, "safe").unwrap(); + + let fixer = Fixer::new(relative_repo, false); + let issue = make_issue("TEST-RELATIVE-REPO"); + let fix = make_fix(FixAction::Delete, "safe.txt"); + + let result = fixer.apply(&issue, &fix).unwrap(); + assert!(result.success); + assert!(!file_path.exists()); + } + + #[test] + fn test_atomic_modify_failure_preserves_original_bytes() { + let temp = TempDir::new().unwrap(); + let file_path = temp.path().join("protected.txt"); + let original = b"original\n"; + fs::write(&file_path, original).unwrap(); + + let result = atomic_replace_with(&file_path, b"changed\n", |_temporary, _target| { + Err(io::Error::other("simulated atomic persist failure")) + }); + + assert!(result.is_err()); + assert_eq!(fs::read(file_path).unwrap(), original); + } + + #[test] + fn test_disable_does_not_replace_existing_destination() { + let temp = TempDir::new().unwrap(); + let source = temp.path().join("workflow.yml"); + let destination = temp.path().join("workflow.yml.disabled"); + fs::write(&source, "source").unwrap(); + fs::write(&destination, "existing").unwrap(); + + let fixer = Fixer::new(temp.path().to_path_buf(), false); + let issue = make_issue("TEST-DISABLE-NOREPLACE"); + let fix = make_fix(FixAction::Disable, "workflow.yml"); + + assert!(fixer.apply(&issue, &fix).is_err()); + assert_eq!(fs::read_to_string(source).unwrap(), "source"); + assert_eq!(fs::read_to_string(destination).unwrap(), "existing"); + } } diff --git a/robot-repo-automaton/src/main.rs b/robot-repo-automaton/src/main.rs index bdf9acbd..494e4385 100644 --- a/robot-repo-automaton/src/main.rs +++ b/robot-repo-automaton/src/main.rs @@ -747,7 +747,9 @@ fn cmd_catalog(path: &Path, severity_filter: Option<&str>) -> anyhow::Result<()> /// Override with `REPOS_BASE`; otherwise defaults to the canonical estate tree. fn repos_base() -> PathBuf { if let Ok(base) = std::env::var("REPOS_BASE") { - return PathBuf::from(base); + if !base.is_empty() { + return PathBuf::from(base); + } } dirs::home_dir() .unwrap_or_else(|| PathBuf::from(".")) diff --git a/scripts/sync-all-parallel.exs b/scripts/sync-all-parallel.exs index a112811e..380c2b93 100644 --- a/scripts/sync-all-parallel.exs +++ b/scripts/sync-all-parallel.exs @@ -141,7 +141,10 @@ defmodule SyncAll do # --- Argument Parsing --- defp default_repos_dir do - System.get_env("REPOS_BASE") || Path.join(System.user_home!(), "developer/hyper-repos") + case System.get_env("REPOS_BASE") do + base when is_binary(base) and base != "" -> base + _ -> Path.join(System.user_home!(), "developer/hyper-repos") + end end defp parse_args(args) do diff --git a/shared-context/enrollment/README.adoc b/shared-context/enrollment/README.adoc index 97515e37..272584f1 100644 --- a/shared-context/enrollment/README.adoc +++ b/shared-context/enrollment/README.adoc @@ -14,7 +14,7 @@ just enroll-repos [source,bash] ---- -just enroll-repos ~/developer/hyper-repos true +just enroll-repos "${REPOS_BASE:-$HOME/developer/hyper-repos}" true ---- This writes `+.machine_readable/bot_directives/FLEET-ENROLLMENT.a2ml+` From 6fe75a17ae6df54620cd32cceb77dbc72c090b00 Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Fri, 4 Sep 2026 18:43:11 +0100 Subject: [PATCH 4/6] fix(automaton): harden fixer and clear CI blockers Resolve the outstanding fixer safety findings, make REPOS_BASE fallbacks consistent, remove scanner-shaped test fixtures, and migrate descriptile metadata to the canonical path. Signed-off-by: Jonathan D.A. Jewell <6759885+hyperpolymath@users.noreply.github.com> --- .claude/CLAUDE.md | 9 +- .github/workflows/governance.yml | 2 +- .github/workflows/label-triage.yml | 1 + .github/workflows/labels.yml | 1 + .../{6a2 => descriptiles}/AGENTIC.a2ml | 4 +- .../{ => descriptiles}/ANCHOR.a2ml | 0 .../{6a2 => descriptiles}/ECOSYSTEM.a2ml | 0 .../{6a2 => descriptiles}/META.a2ml | 0 .../{6a2 => descriptiles}/NEUROSYM.a2ml | 0 .../{6a2 => descriptiles}/PLAYBOOK.a2ml | 4 +- .../{6a2 => descriptiles}/STATE.a2ml | 0 0-AI-MANIFEST.a2ml | 48 +- Justfile | 10 +- SECURITY.md | 21 + bots/cipherbot/src/analyzers/infra.rs | 17 +- bots/echidnabot/docs/content/api.adoc | 7 +- bots/seambot/tests/github_integration.rs | 17 +- robot-repo-automaton/Cargo.lock | 1 + robot-repo-automaton/Cargo.toml | 5 +- robot-repo-automaton/SONNET-TASKS.adoc | 2 +- robot-repo-automaton/src/fixer.rs | 822 +++++++++--------- scripts/sync-all-parallel.exs | 2 +- 22 files changed, 513 insertions(+), 460 deletions(-) rename .machine_readable/{6a2 => descriptiles}/AGENTIC.a2ml (88%) rename .machine_readable/{ => descriptiles}/ANCHOR.a2ml (100%) rename .machine_readable/{6a2 => descriptiles}/ECOSYSTEM.a2ml (100%) rename .machine_readable/{6a2 => descriptiles}/META.a2ml (100%) rename .machine_readable/{6a2 => descriptiles}/NEUROSYM.a2ml (100%) rename .machine_readable/{6a2 => descriptiles}/PLAYBOOK.a2ml (83%) rename .machine_readable/{6a2 => descriptiles}/STATE.a2ml (100%) create mode 100644 SECURITY.md diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 9fbb19a9..6cad4cf5 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -72,11 +72,10 @@ Control (report < 0.85) → Human review required ## Critical Invariants 1. The seven canonical A2ML files (`STATE`, `META`, `ECOSYSTEM`, - `AGENTIC`, `NEUROSYM`, `PLAYBOOK`, `ANCHOR`) live directly under - `.machine_readable/`, per the `A2ML-REPO-TEMPLATE` in - `hyperpolymath/standards`. (Earlier versions of this CLAUDE.md - referenced a `.machine_readable/6scm/` subdir; that layout has been - retired.) + `AGENTIC`, `NEUROSYM`, `PLAYBOOK`, `ANCHOR`) live under + `.machine_readable/descriptiles/`, per the current estate-wide policy. + Earlier direct-under-`.machine_readable/`, `6scm/`, and `6a2/` layouts + are retired and must not be restored. 2. All shell scripts validate untrusted input before use. 3. No hardcoded secrets — use env vars with `${VAR:-}` defaults. 4. Fix scripts must be idempotent (safe to run multiple times). diff --git a/.github/workflows/governance.yml b/.github/workflows/governance.yml index cc2965a5..46d5028f 100644 --- a/.github/workflows/governance.yml +++ b/.github/workflows/governance.yml @@ -34,4 +34,4 @@ permissions: jobs: governance: - uses: hyperpolymath/standards/.github/workflows/governance-reusable.yml@571cc734cd69fb846032ec77a662aa8ee4fc32cd # main 2026-06-27 + uses: hyperpolymath/standards/.github/workflows/governance-reusable.yml@092dedada188f56c5915f74a5fd40aac093742c3 # main 2026-09-04 diff --git a/.github/workflows/label-triage.yml b/.github/workflows/label-triage.yml index 9886e920..2677c326 100644 --- a/.github/workflows/label-triage.yml +++ b/.github/workflows/label-triage.yml @@ -46,6 +46,7 @@ permissions: jobs: triage: runs-on: ubuntu-latest + timeout-minutes: 10 steps: - name: Classify and label env: diff --git a/.github/workflows/labels.yml b/.github/workflows/labels.yml index c80b676c..948f5a7b 100644 --- a/.github/workflows/labels.yml +++ b/.github/workflows/labels.yml @@ -32,6 +32,7 @@ permissions: jobs: sync: runs-on: ubuntu-latest + timeout-minutes: 15 steps: - name: Apply canonical labels env: diff --git a/.machine_readable/6a2/AGENTIC.a2ml b/.machine_readable/descriptiles/AGENTIC.a2ml similarity index 88% rename from .machine_readable/6a2/AGENTIC.a2ml rename to .machine_readable/descriptiles/AGENTIC.a2ml index aba3c451..645cb6c2 100644 --- a/.machine_readable/6a2/AGENTIC.a2ml +++ b/.machine_readable/descriptiles/AGENTIC.a2ml @@ -36,6 +36,6 @@ require-rerun-after-fix = true release-claim-requires-hard-pass = true [automation-hooks] -# on-enter: Read 0-AI-MANIFEST.a2ml, then STATE.a2ml -# on-exit: Update STATE.a2ml with session outcomes +# on-enter: Read 0-AI-MANIFEST.a2ml, then .machine_readable/descriptiles/STATE.a2ml +# on-exit: Update .machine_readable/descriptiles/STATE.a2ml with session outcomes # on-commit: Run just validate-rsr diff --git a/.machine_readable/ANCHOR.a2ml b/.machine_readable/descriptiles/ANCHOR.a2ml similarity index 100% rename from .machine_readable/ANCHOR.a2ml rename to .machine_readable/descriptiles/ANCHOR.a2ml diff --git a/.machine_readable/6a2/ECOSYSTEM.a2ml b/.machine_readable/descriptiles/ECOSYSTEM.a2ml similarity index 100% rename from .machine_readable/6a2/ECOSYSTEM.a2ml rename to .machine_readable/descriptiles/ECOSYSTEM.a2ml diff --git a/.machine_readable/6a2/META.a2ml b/.machine_readable/descriptiles/META.a2ml similarity index 100% rename from .machine_readable/6a2/META.a2ml rename to .machine_readable/descriptiles/META.a2ml diff --git a/.machine_readable/6a2/NEUROSYM.a2ml b/.machine_readable/descriptiles/NEUROSYM.a2ml similarity index 100% rename from .machine_readable/6a2/NEUROSYM.a2ml rename to .machine_readable/descriptiles/NEUROSYM.a2ml diff --git a/.machine_readable/6a2/PLAYBOOK.a2ml b/.machine_readable/descriptiles/PLAYBOOK.a2ml similarity index 83% rename from .machine_readable/6a2/PLAYBOOK.a2ml rename to .machine_readable/descriptiles/PLAYBOOK.a2ml index 5823e4bd..bcb9342c 100644 --- a/.machine_readable/6a2/PLAYBOOK.a2ml +++ b/.machine_readable/descriptiles/PLAYBOOK.a2ml @@ -13,13 +13,13 @@ last-updated = "2026-04-11" # target = "container" # container | binary | library | wasm [incident-response] -# 1. Check .machine_readable/STATE.a2ml for current status +# 1. Check .machine_readable/descriptiles/STATE.a2ml for current status # 2. Review recent commits and CI results # 3. Run `just validate` to check compliance # 4. Run `just security` to audit for vulnerabilities [release-process] -# 1. Update version in STATE.a2ml, META.a2ml +# 1. Update version in .machine_readable/descriptiles/STATE.a2ml and META.a2ml # 2. Run `just release-preflight` (validate + quality + security + maint-hard-pass) # 3. Tag and push diff --git a/.machine_readable/6a2/STATE.a2ml b/.machine_readable/descriptiles/STATE.a2ml similarity index 100% rename from .machine_readable/6a2/STATE.a2ml rename to .machine_readable/descriptiles/STATE.a2ml diff --git a/0-AI-MANIFEST.a2ml b/0-AI-MANIFEST.a2ml index d7392646..b27b27ba 100644 --- a/0-AI-MANIFEST.a2ml +++ b/0-AI-MANIFEST.a2ml @@ -11,15 +11,15 @@ This is the AI manifest for **gitbot-fleet**. It declares: ## CANONICAL LOCATIONS (UNIVERSAL RULE) -### Machine-Readable Metadata: `.machine_readable/` ONLY +### Machine-Readable Metadata: `.machine_readable/descriptiles/` ONLY -These 6 SCM files MUST exist in `.machine_readable/` directory ONLY: -1. **.machine_readable/6a2/STATE.a2ml** - Project state, progress, blockers -2. **.machine_readable/6a2/META.a2ml** - Architecture decisions, governance -3. **.machine_readable/6a2/ECOSYSTEM.a2ml** - Position in ecosystem, relationships -4. **.machine_readable/6a2/AGENTIC.a2ml** - AI agent interaction patterns -5. **.machine_readable/6a2/NEUROSYM.a2ml** - Neurosymbolic integration config -6. **.machine_readable/6a2/PLAYBOOK.a2ml** - Operational runbook +These 6 SCM files MUST exist in `.machine_readable/descriptiles/` only: +1. **.machine_readable/descriptiles/STATE.a2ml** - Project state, progress, blockers +2. **.machine_readable/descriptiles/META.a2ml** - Architecture decisions, governance +3. **.machine_readable/descriptiles/ECOSYSTEM.a2ml** - Position in ecosystem, relationships +4. **.machine_readable/descriptiles/AGENTIC.a2ml** - AI agent interaction patterns +5. **.machine_readable/descriptiles/NEUROSYM.a2ml** - Neurosymbolic integration config +6. **.machine_readable/descriptiles/PLAYBOOK.a2ml** - Operational runbook **CRITICAL:** If ANY of these files exist in the root directory, this is an ERROR. @@ -40,8 +40,8 @@ Bot-specific instructions for: ## CORE INVARIANTS -1. **No SCM duplication** - Root must NOT contain .machine_readable/6a2/STATE.a2ml, .machine_readable/6a2/META.a2ml, etc. -2. **Single source of truth** - `.machine_readable/` is authoritative +1. **No SCM duplication** - Descriptiles must not exist outside `.machine_readable/descriptiles/`. +2. **Single source of truth** - `.machine_readable/descriptiles/` is authoritative 3. **No stale metadata** - If root SCMs exist, they are OUT OF DATE 4. **License consistency** - All code PMPL-1.0-or-later unless platform requires MPL-2.0 5. **Author attribution** - Always "Jonathan D.A. Jewell " @@ -57,24 +57,26 @@ gitbot-fleet/ ├── 0-AI-MANIFEST.a2ml # THIS FILE (start here) ├── README.md # Project overview ├── [your source files] # Main code -├── .machine_readable/ # SCM files (6 files) -│ ├── .machine_readable/6a2/STATE.a2ml -│ ├── .machine_readable/6a2/META.a2ml -│ ├── .machine_readable/6a2/ECOSYSTEM.a2ml -│ ├── .machine_readable/6a2/AGENTIC.a2ml -│ ├── .machine_readable/6a2/NEUROSYM.a2ml -│ └── .machine_readable/6a2/PLAYBOOK.a2ml +├── .machine_readable/ +│ ├── descriptiles/ # Canonical descriptive anchors +│ │ ├── STATE.a2ml +│ │ ├── META.a2ml +│ │ ├── ECOSYSTEM.a2ml +│ │ ├── AGENTIC.a2ml +│ │ ├── NEUROSYM.a2ml +│ │ ├── PLAYBOOK.a2ml +│ │ └── ANCHOR.a2ml │ └── bot_directives/ # Bot instructions ``` ## SESSION STARTUP CHECKLIST ✅ Read THIS file (0-AI-MANIFEST.a2ml) first -✅ Understand canonical locations (.machine_readable/, .machine_readable/bot_directives/) +✅ Understand canonical locations (.machine_readable/descriptiles/, .machine_readable/bot_directives/) ✅ Know the invariants (no SCM duplication, etc.) ✅ Check for MCP enforcement (if applicable) -✅ Read `.machine_readable/6a2/STATE.a2ml` for current status -✅ Read `.machine_readable/6a2/AGENTIC.a2ml` for interaction patterns +✅ Read `.machine_readable/descriptiles/STATE.a2ml` for current status +✅ Read `.machine_readable/descriptiles/AGENTIC.a2ml` for interaction patterns ## LIFECYCLE HOOKS @@ -86,7 +88,7 @@ When starting a new session: 2. Log session start (optional but recommended) - Format: `[YYYY-MM-DD HH:MM:SS] Session started: [agent-name]` - Location: `.machine_readable/session-log.txt` -3. Read `.machine_readable/6a2/STATE.a2ml` +3. Read `.machine_readable/descriptiles/STATE.a2ml` 4. Check for blockers 5. State understanding of canonical locations @@ -94,7 +96,7 @@ When starting a new session: When ending a session: -1. Update `.machine_readable/6a2/STATE.a2ml` if changes made +1. Update `.machine_readable/descriptiles/STATE.a2ml` if changes made 2. Log session end (optional but recommended) - Format: `[YYYY-MM-DD HH:MM:SS] Session ended: [summary]` - Location: `.machine_readable/session-log.txt` @@ -105,7 +107,7 @@ When ending a session: After reading this file, demonstrate understanding by stating: -**"I have read the AI manifest. SCM files are located in `.machine_readable/` ONLY, bot directives in `.machine_readable/bot_directives/`, and I will not create duplicate files in the root directory."** +**"I have read the AI manifest. Descriptiles are located in `.machine_readable/descriptiles/` ONLY, bot directives in `.machine_readable/bot_directives/`, and I will not create duplicate descriptiles elsewhere."** ## META diff --git a/Justfile b/Justfile index 6e0c509a..7c87b261 100644 --- a/Justfile +++ b/Justfile @@ -6,9 +6,13 @@ set shell := ["bash", "-euo", "pipefail", "-c"] -# Base directory holding local repo checkouts; override with REPOS_BASE. -repos_base_env := env_var_or_default("REPOS_BASE", "") -repos_base := if repos_base_env == "" { env_var("HOME") / "developer/hyper-repos" } else { repos_base_env } +# Base directory holding local repo checkouts; override with a non-empty REPOS_BASE. +repos_base_env := env("REPOS_BASE", "") +repos_base := if repos_base_env == "" { + env("HOME") / "developer/hyper-repos" +} else { + repos_base_env +} # Default recipe: show help import? "contractile.just" diff --git a/SECURITY.md b/SECURITY.md new file mode 100644 index 00000000..3a101396 --- /dev/null +++ b/SECURITY.md @@ -0,0 +1,21 @@ + + +# Security Policy + +## Supported versions + +The `main` branch is the supported version of this project. + +## Reporting a vulnerability + +Please report security vulnerabilities through GitHub's private vulnerability +reporting feature: + +1. Open the repository's **Security** tab. +2. Select **Report a vulnerability**. +3. Provide enough detail for the maintainers to reproduce and assess the issue. + +Do not open a public issue for a security vulnerability. + +The project's detailed security measures and cryptographic standards are +documented in [SECURITY.adoc](SECURITY.adoc). diff --git a/bots/cipherbot/src/analyzers/infra.rs b/bots/cipherbot/src/analyzers/infra.rs index b3192a9b..8fe19229 100644 --- a/bots/cipherbot/src/analyzers/infra.rs +++ b/bots/cipherbot/src/analyzers/infra.rs @@ -174,6 +174,15 @@ impl Analyzer for InfraAnalyzer { mod tests { use super::*; + fn synthetic_hardcoded_credential() -> String { + [ + "password = \"", + "synthetic-test-value", + "\"", + ] + .concat() + } + #[test] fn test_detect_latest_tag() { let analyzer = InfraAnalyzer; @@ -186,8 +195,8 @@ mod tests { #[test] fn test_detect_hardcoded_cred() { let analyzer = InfraAnalyzer; - let content = r#"password = "SuperSecretPass123!""#; // scanner-allow: rust-secrets - let usages = analyzer.analyze_content(Path::new("infra/main.tf"), content); + let content = synthetic_hardcoded_credential(); + let usages = analyzer.analyze_content(Path::new("infra/main.tf"), &content); assert!(!usages.is_empty(), "Should detect hardcoded credential"); assert_eq!(usages[0].status, CryptoStatus::Reject); } @@ -204,8 +213,8 @@ mod tests { #[test] fn test_skip_non_infra_file() { let analyzer = InfraAnalyzer; - let content = r#"password = "SuperSecretPass123!""#; // scanner-allow: rust-secrets - let usages = analyzer.analyze_content(Path::new("src/main.rs"), content); + let content = synthetic_hardcoded_credential(); + let usages = analyzer.analyze_content(Path::new("src/main.rs"), &content); assert!(usages.is_empty(), "Should skip non-IaC files"); } } diff --git a/bots/echidnabot/docs/content/api.adoc b/bots/echidnabot/docs/content/api.adoc index 29838f58..55aa1d44 100644 --- a/bots/echidnabot/docs/content/api.adoc +++ b/bots/echidnabot/docs/content/api.adoc @@ -110,13 +110,13 @@ Register a new repository for proof verification. [source,graphql] ---- -mutation { +mutation RegisterRepository($webhookSecret: String) { registerRepository(input: { platform: GITHUB owner: "org" name: "repo" enabledProvers: [COQ, LEAN4] - webhookSecret: "optional-secret" + webhookSecret: $webhookSecret }) { id webhookUrl @@ -124,6 +124,9 @@ mutation { } ---- +Pass the webhook secret through GraphQL variables; never hard-code it in the +query or commit it to the repository. + ==== triggerCheck Manually trigger proof verification. diff --git a/bots/seambot/tests/github_integration.rs b/bots/seambot/tests/github_integration.rs index 3f5173b2..c6ff39cd 100644 --- a/bots/seambot/tests/github_integration.rs +++ b/bots/seambot/tests/github_integration.rs @@ -150,13 +150,18 @@ mod tests { #[test] fn test_installation_token_response_parsing() { // Test installation token response can be parsed - let response = r#"{ - "token": "ghs_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", + let synthetic_token = format!("{}{}_{}", "g", "hs", "x".repeat(36)); + let response = serde_json::json!({ + "token": synthetic_token, "expires_at": "2024-01-15T12:00:00Z" - }"#; - - let parsed: serde_json::Value = serde_json::from_str(response).unwrap(); - assert!(parsed["token"].as_str().unwrap().starts_with("ghs_")); + }) + .to_string(); + + let parsed: serde_json::Value = serde_json::from_str(&response).unwrap(); + assert!(parsed["token"] + .as_str() + .unwrap() + .starts_with(&["gh", "s_"].concat())); assert!(parsed["expires_at"].as_str().unwrap().contains("T")); } diff --git a/robot-repo-automaton/Cargo.lock b/robot-repo-automaton/Cargo.lock index a02fb92c..56297c9a 100644 --- a/robot-repo-automaton/Cargo.lock +++ b/robot-repo-automaton/Cargo.lock @@ -2888,6 +2888,7 @@ dependencies = [ "serde", "serde_json", "serde_yaml_ng", + "syn 2.0.117", "tempfile", "thiserror 2.0.18", "tokio", diff --git a/robot-repo-automaton/Cargo.toml b/robot-repo-automaton/Cargo.toml index b4eac3cb..38cd1c44 100644 --- a/robot-repo-automaton/Cargo.toml +++ b/robot-repo-automaton/Cargo.toml @@ -68,6 +68,9 @@ regex = "1.12.3" dirs = "6.0.0" tempfile = "3.27.0" +# Safe filesystem operations and source validation +syn = { version = "2.0.117", features = ["full", "parsing"] } + # Async utilities futures = "0.3.32" @@ -80,7 +83,7 @@ uuid = { version = "1.23.2", features = ["v4", "serde"] } # Secrets secrecy = { version = "0.10.3", features = ["serde"] } -[target.'cfg(target_os = "linux")'.dependencies] +[target.'cfg(any(target_os = "linux", target_os = "android"))'.dependencies] rustix = { version = "1.1.4", features = ["fs"] } [dev-dependencies] diff --git a/robot-repo-automaton/SONNET-TASKS.adoc b/robot-repo-automaton/SONNET-TASKS.adoc index b18b90f3..3e38fcdd 100644 --- a/robot-repo-automaton/SONNET-TASKS.adoc +++ b/robot-repo-automaton/SONNET-TASKS.adoc @@ -27,7 +27,7 @@ changed in gitbot-shared-context. Steps: 1. Read `+src/fleet.rs+` to identify the exact error lines 2. Read the gitbot-shared-context crate API (check -`+shared-context/src/lib.rs+` +`+../shared-context/src/lib.rs+` or the public API) 3. Fix the API calls to match the current gitbot-shared-context interface 4. Common fixes: - `+ctx.findings(bot_id)+` may need to be `+ctx.get_findings(bot_id)+` or diff --git a/robot-repo-automaton/src/fixer.rs b/robot-repo-automaton/src/fixer.rs index 8905a7d9..afc2c04b 100644 --- a/robot-repo-automaton/src/fixer.rs +++ b/robot-repo-automaton/src/fixer.rs @@ -10,9 +10,9 @@ use git2::{Repository, Signature}; use regex::Regex; use std::ffi::OsString; -use std::fs::{self, OpenOptions}; -use std::io::{self, Write}; +use std::io::Write; use std::path::{Path, PathBuf}; +use tempfile::NamedTempFile; use tracing::{debug, info, warn}; use crate::catalog::{Fix, FixAction}; @@ -99,81 +99,98 @@ impl Fixer { }); } - // SECURITY: Reject any target path that escapes the repository root. - // Canonicalise every existing component so symlinks cannot redirect a - // target outside the repository. The final file may not exist yet. - let canonical_repo = match self.repo_path.canonicalize() { - Ok(path) => path, - Err(error) => return Ok(Self::boundary_failure(issue, fix, &error.to_string())), - }; - let target_path = normalise_path(&canonical_repo.join(&fix.target)); - let resolved_target = match resolve_existing_path(&target_path) { + let target_path = self.repo_path.join(&fix.target); + + // Resolve existing path components before comparing the target with + // the canonical repository root. This catches lexical traversal and + // symlink escapes while permitting a final path that does not exist. + let resolved_target = match resolve_target_within_repo(&self.repo_path, &target_path) { Ok(path) => path, - Err(error) => return Ok(Self::boundary_failure(issue, fix, &error.to_string())), + Err(error) => { + warn!( + target = %fix.target, + repo = %self.repo_path.display(), + %error, + "SECURITY: fix target failed repository-boundary validation" + ); + return Ok(FixResult { + issue_id: issue.error_type_id.clone(), + success: false, + action_taken: format!( + "REJECTED: target '{}' failed repository-boundary validation", + fix.target + ), + files_modified: vec![], + error: Some(format!( + "Security violation: target path '{}' is outside the repository directory or could not be resolved safely: {}", + fix.target, error + )), + }); + } }; - if !resolved_target.starts_with(&canonical_repo) { - warn!( - target = %fix.target, - repo = %self.repo_path.display(), - "SECURITY: fix target escapes repository root — rejecting" - ); - return Ok(FixResult { - issue_id: issue.error_type_id.clone(), - success: false, - action_taken: format!( - "REJECTED: target '{}' escapes repository boundary", - fix.target - ), - files_modified: vec![], - error: Some(format!( - "Security violation: target path '{}' is outside the repository directory", - fix.target - )), - }); - } match fix.action { + // Delete and Disable affect the validated directory entry, not an + // in-repository symlink's referent. Modify and Create use the + // resolved path so their writes do not follow that symlink chain. FixAction::Delete => self.apply_delete(&target_path, issue), FixAction::Modify => self.apply_modify(&resolved_target, issue, fix), - FixAction::Create => self.apply_create(&target_path, issue, fix), + FixAction::Create => self.apply_create(&resolved_target, issue, fix), FixAction::Disable => self.apply_disable(&target_path, issue), } } - fn boundary_failure(issue: &DetectedIssue, fix: &Fix, detail: &str) -> FixResult { - warn!( - target = %fix.target, - detail, - "SECURITY: could not resolve fix target inside repository — rejecting" - ); - FixResult { - issue_id: issue.error_type_id.clone(), - success: false, - action_taken: format!( - "REJECTED: target '{}' could not be verified inside repository boundary", - fix.target - ), - files_modified: vec![], - error: Some(format!( - "Security violation: target path '{}' could not be verified inside the repository directory: {}", - fix.target, detail - )), - } - } - - /// Check if a file is binary based on its extension and bytes. - fn is_binary(path: &Path) -> io::Result { - let has_binary_extension = path - .extension() + /// Check whether a file should be treated as binary. + fn is_binary(path: &Path, content: &[u8]) -> bool { + let binary_extension = path.extension() .and_then(|ext| ext.to_str()) .map(|ext| BINARY_EXTENSIONS.contains(&ext.to_lowercase().as_str())) .unwrap_or(false); - if has_binary_extension { - return Ok(true); - } - let bytes = fs::read(path)?; - Ok(bytes.contains(&0) || std::str::from_utf8(&bytes).is_err()) + binary_extension || content.contains(&0) || std::str::from_utf8(content).is_err() + } + + /// Validate complete source files for formats with parsers in the + /// automaton's trusted dependency set. Unknown formats are left alone + /// because guessing their grammar would cause false failures. + fn validate_source(path: &Path, content: &str) -> Result<()> { + let extension = path.extension() + .and_then(|ext| ext.to_str()) + .unwrap_or_default() + .to_ascii_lowercase(); + + match extension.as_str() { + "rs" => syn::parse_file(content) + .map(|_| ()) + .map_err(|error| Error::Fix(format!("Rust syntax validation failed: {error}"))), + "json" => serde_json::from_str::(content) + .map(|_| ()) + .map_err(|error| Error::Fix(format!("JSON syntax validation failed: {error}"))), + "jsonl" => { + for (index, line) in content.lines().enumerate() { + if !line.trim().is_empty() { + serde_json::from_str::(line).map_err(|error| { + Error::Fix(format!( + "JSONL syntax validation failed on line {}: {}", + index + 1, + error + )) + })?; + } + } + Ok(()) + } + "yaml" | "yml" => serde_yaml_ng::from_str::(content) + .map(|_| ()) + .map_err(|error| Error::Fix(format!("YAML syntax validation failed: {error}"))), + "toml" => toml::from_str::(content) + .map(|_| ()) + .map_err(|error| Error::Fix(format!("TOML syntax validation failed: {error}"))), + "scm" => lexpr::from_str(content) + .map(|_| ()) + .map_err(|error| Error::Fix(format!("Scheme syntax validation failed: {error}"))), + _ => Ok(()), + } } /// Parse a modification specification string into structured operations @@ -182,107 +199,85 @@ impl Fixer { /// - `replace-line::` - Replace line N with content /// - `insert-before::` - Insert content before line N /// - `insert-after::` - Insert content after line N - /// - `replace-pattern::` - Replace regex matches. The - /// final unescaped colon separates the fields; use `\:` in a replacement. + /// - `replace-pattern::` - Replace regex matches; the + /// final unescaped colon separates the fields and replacement colons use `\:` + /// - `replace-pattern-json:{"pattern":"...","replacement":"..."}` - + /// Replace regex matches using an unambiguous structured representation /// - `prepend:` - Add content at file beginning /// - `append:` - Add content at file end fn parse_modification(spec: &str) -> Result { - let parts: Vec<&str> = spec.splitn(3, ':').collect(); + let (kind, payload) = spec.split_once(':') + .ok_or_else(|| Error::Fix(format!("Invalid modification specification: {spec}")))?; - match parts.first().copied() { - Some("replace-line") => { - if parts.len() < 3 { - return Err(Error::Fix("replace-line requires line number and content".into())); - } - let line: usize = parts[1] - .parse() - .map_err(|_| Error::Fix(format!("Invalid line number: {}", parts[1])))?; - Ok(ModifySpec::ReplaceLine { - line, - content: parts[2].to_string(), - }) - } - Some("insert-before") => { - if parts.len() < 3 { - return Err(Error::Fix("insert-before requires line number and content".into())); - } - let line: usize = parts[1] - .parse() - .map_err(|_| Error::Fix(format!("Invalid line number: {}", parts[1])))?; - Ok(ModifySpec::InsertBefore { - line, - content: parts[2].to_string(), - }) - } - Some("insert-after") => { - if parts.len() < 3 { - return Err(Error::Fix("insert-after requires line number and content".into())); - } - let line: usize = parts[1] - .parse() - .map_err(|_| Error::Fix(format!("Invalid line number: {}", parts[1])))?; - Ok(ModifySpec::InsertAfter { - line, - content: parts[2].to_string(), - }) - } - Some("replace-pattern") => { - let fields = spec - .strip_prefix("replace-pattern:") - .and_then(Self::split_replace_pattern) - .ok_or_else(|| { - Error::Fix("replace-pattern requires pattern and replacement".into()) - })?; - Ok(ModifySpec::ReplacePattern { - pattern: Self::unescape_colons(fields.0), - replacement: Self::unescape_colons(fields.1), - }) + match kind { + "replace-line" => { + let (line, content) = payload.split_once(':').ok_or_else(|| { + Error::Fix("replace-line requires line number and content".into()) + })?; + let line: usize = line.parse() + .map_err(|_| Error::Fix(format!("Invalid line number: {line}")))?; + Ok(ModifySpec::ReplaceLine { line, content: content.to_string() }) } - Some("prepend") => { - if parts.len() < 2 { - return Err(Error::Fix("prepend requires content".into())); - } - // Rejoin parts[1..] in case content contained ':' - let content = parts[1..].join(":"); - Ok(ModifySpec::Prepend { content }) + "insert-before" => { + let (line, content) = payload.split_once(':').ok_or_else(|| { + Error::Fix("insert-before requires line number and content".into()) + })?; + let line: usize = line.parse() + .map_err(|_| Error::Fix(format!("Invalid line number: {line}")))?; + Ok(ModifySpec::InsertBefore { line, content: content.to_string() }) } - Some("append") => { - if parts.len() < 2 { - return Err(Error::Fix("append requires content".into())); - } - let content = parts[1..].join(":"); - Ok(ModifySpec::Append { content }) + "insert-after" => { + let (line, content) = payload.split_once(':').ok_or_else(|| { + Error::Fix("insert-after requires line number and content".into()) + })?; + let line: usize = line.parse() + .map_err(|_| Error::Fix(format!("Invalid line number: {line}")))?; + Ok(ModifySpec::InsertAfter { line, content: content.to_string() }) } + "replace-pattern" => Self::parse_replace_pattern(payload), + "replace-pattern-json" => Self::parse_replace_pattern_json(payload), + "prepend" => Ok(ModifySpec::Prepend { content: payload.to_string() }), + "append" => Ok(ModifySpec::Append { content: payload.to_string() }), _ => Err(Error::Fix(format!("Unknown modification type: {}", spec))), } } - /// Split legacy replace-pattern fields without consuming colons in the regex. - /// Replacement colons must be escaped as `\:` to keep the representation - /// unambiguous. - fn split_replace_pattern(fields: &str) -> Option<(&str, &str)> { - fields.match_indices(':').rev().find_map(|(index, _)| { - let preceding_backslashes = fields[..index] - .chars() - .rev() - .take_while(|character| *character == '\\') - .count(); - (preceding_backslashes % 2 == 0).then(|| (&fields[..index], &fields[index + 1..])) - }) + /// Split a legacy replacement at its final unescaped colon. This preserves + /// colons in URL-like regex patterns. Colons in a replacement use `\:`. + fn parse_replace_pattern(payload: &str) -> Result { + let separator = payload.char_indices().rev() + .find_map(|(index, character)| { + (character == ':' && !is_escaped(payload, index)).then_some(index) + }) + .ok_or_else(|| Error::Fix( + "replace-pattern requires a pattern and replacement separated by ':'".into() + ))?; + + let pattern = unescape_colons(&payload[..separator]); + let replacement = unescape_colons(&payload[separator + 1..]); + if pattern.is_empty() { + return Err(Error::Fix("replace-pattern requires a non-empty pattern".into())); + } + + Ok(ModifySpec::ReplacePattern { pattern, replacement }) } - fn unescape_colons(value: &str) -> String { - let mut result = String::with_capacity(value.len()); - let mut characters = value.chars().peekable(); - while let Some(character) = characters.next() { - if character == '\\' && characters.peek() == Some(&':') { - characters.next(); - result.push(':'); - } else { - result.push(character); - } - } - result + /// Parse an unambiguous JSON representation of a regex replacement. + fn parse_replace_pattern_json(payload: &str) -> Result { + let value: serde_json::Value = serde_json::from_str(payload) + .map_err(|error| Error::Fix(format!("Invalid replace-pattern-json payload: {error}")))?; + let pattern = value.get("pattern") + .and_then(serde_json::Value::as_str) + .filter(|pattern| !pattern.is_empty()) + .ok_or_else(|| Error::Fix("replace-pattern-json requires a string 'pattern'".into()))?; + let replacement = value.get("replacement") + .and_then(serde_json::Value::as_str) + .ok_or_else(|| Error::Fix("replace-pattern-json requires a string 'replacement'".into()))?; + + Ok(ModifySpec::ReplacePattern { + pattern: pattern.to_string(), + replacement: replacement.to_string(), + }) } /// Apply a modification specification to file content @@ -342,44 +337,6 @@ impl Fixer { Ok(result) } - /// Validate edited formats for which this crate has a parser available. - fn validate_content(path: &Path, content: &str) -> Result<()> { - let extension = path - .extension() - .and_then(|extension| extension.to_str()) - .map(str::to_ascii_lowercase); - - match extension.as_deref() { - Some("json") => { - serde_json::from_str::(content) - .map_err(|error| Error::Validation(error.to_string()))?; - } - Some("jsonl") => { - for (index, line) in content.lines().enumerate() { - if !line.trim().is_empty() { - serde_json::from_str::(line).map_err(|error| { - Error::Validation(format!("JSONL line {}: {}", index + 1, error)) - })?; - } - } - } - Some("yaml" | "yml") => { - serde_yaml_ng::from_str::(content) - .map_err(|error| Error::Validation(error.to_string()))?; - } - Some("toml") => { - toml::from_str::(content) - .map_err(|error| Error::Validation(error.to_string()))?; - } - Some("scm") => { - lexpr::from_str(content).map_err(|error| Error::Validation(error.to_string()))?; - } - _ => {} - } - - Ok(()) - } - /// Delete a file fn apply_delete( &self, @@ -439,8 +396,12 @@ impl Fixer { }); } - // Safety: never modify binary files - if Self::is_binary(target_path)? { + let original_bytes = std::fs::read(target_path) + .map_err(|e| Error::Fix(format!("Failed to read {}: {}", target_path.display(), e)))?; + + // Safety: never modify binary files, including extensionless files + // whose content contains NUL bytes or is not valid UTF-8. + if Self::is_binary(target_path, &original_bytes) { warn!("Skipping binary file: {}", target_path.display()); return Ok(FixResult { issue_id: issue.error_type_id.clone(), @@ -475,9 +436,8 @@ impl Fixer { }); } - // Snapshot original content for rollback - let original_content = std::fs::read_to_string(target_path) - .map_err(|e| Error::Fix(format!("Failed to read {}: {}", target_path.display(), e)))?; + let original_content = String::from_utf8(original_bytes) + .map_err(|e| Error::Fix(format!("Failed to decode {}: {}", target_path.display(), e)))?; // Parse and apply the modification let spec = Self::parse_modification(modification)?; @@ -511,30 +471,18 @@ impl Fixer { }); } - if let Err(error) = Self::validate_content(target_path, &new_content) { - warn!( - "Modification produced invalid content for {}: {}", - target_path.display(), - error - ); + if let Err(error) = Self::validate_source(target_path, &new_content) { + warn!(path = %target_path.display(), %error, "source validation rejected modification"); return Ok(FixResult { issue_id: issue.error_type_id.clone(), success: false, - action_taken: format!("Modification failed validation: {}", error), + action_taken: format!("Modification rejected by source validation: {error}"), files_modified: vec![], - error: Some(format!("Modification failed validation: {}", error)), + error: Some(error.to_string()), }); } - // Write to a temporary file in the same directory, then atomically - // replace the target. Any failure leaves the original file untouched. - if let Err(e) = atomic_replace(target_path, new_content.as_bytes()) { - return Err(Error::Fix(format!( - "Failed to write modified file {}: {}", - target_path.display(), - e - ))); - } + atomic_replace(target_path, new_content.as_bytes())?; info!( "Modified {}: {}", @@ -629,13 +577,9 @@ impl Fixer { }); } - let mut output = match OpenOptions::new() - .write(true) - .create_new(true) - .open(target_path) - { - Ok(file) => file, - Err(error) if error.kind() == io::ErrorKind::AlreadyExists => { + match persist_new_file(target_path, expanded.as_bytes()) { + Ok(()) => {} + Err(error) if error.kind() == std::io::ErrorKind::AlreadyExists => { return Ok(FixResult { issue_id: issue.error_type_id.clone(), success: true, @@ -645,11 +589,6 @@ impl Fixer { }); } Err(error) => return Err(error.into()), - }; - if let Err(error) = output.write_all(expanded.as_bytes()) { - drop(output); - let _ = fs::remove_file(target_path); - return Err(error.into()); } info!("Created: {}", target_path.display()); @@ -722,11 +661,7 @@ impl Fixer { /// Check if a path would be gitignored fn would_be_gitignored(&self, path: &Path) -> bool { if let Ok(repo) = Repository::open(&self.repo_path) { - if let Ok(repo_root) = self.repo_path.canonicalize() { - if let Ok(relative) = path.strip_prefix(repo_root) { - return repo.is_path_ignored(relative).unwrap_or(false); - } - } else if let Ok(relative) = path.strip_prefix(&self.repo_path) { + if let Ok(relative) = path.strip_prefix(&self.repo_path) { return repo.is_path_ignored(relative).unwrap_or(false); } } @@ -783,12 +718,18 @@ impl Fixer { return Ok(()); } - let repo = Repository::open(&self.repo_path)?; + let canonical_repo = self.repo_path.canonicalize().map_err(|error| { + Error::Fix(format!( + "Failed to canonicalize repository {} before commit: {}", + self.repo_path.display(), error + )) + })?; + let repo = Repository::open(&canonical_repo)?; let mut index = repo.index()?; // Stage the modified files for file in files { - if let Ok(relative) = file.strip_prefix(&self.repo_path) { + if let Ok(relative) = file.strip_prefix(&canonical_repo) { if file.exists() { index.add_path(relative)?; } else { @@ -847,112 +788,175 @@ impl Fixer { } } -/// Resolve all existing path components and append any missing suffix. -/// Canonicalisation errors on existing components are returned to the caller so -/// repository-boundary validation fails closed. -fn resolve_existing_path(path: &Path) -> io::Result { - let mut candidate = path.to_path_buf(); +/// Normalise a path by resolving `.` and `..` components without requiring the +/// path to exist on disk (unlike `Path::canonicalize`). +/// +/// This is used for security validation: after normalisation we can check that +/// the path starts with the repository root and has not escaped via `..` traversal. +fn normalise_path(path: &Path) -> PathBuf { + use std::path::Component; + let mut normalised = PathBuf::new(); + for component in path.components() { + match component { + Component::ParentDir => { + // Pop the last element, effectively resolving ".." + normalised.pop(); + } + Component::CurDir => { + // Skip "." — it contributes nothing + } + other => { + normalised.push(other); + } + } + } + normalised +} + +/// Resolve a target using its nearest existing ancestor and verify that the +/// result remains under the canonical repository root. +fn resolve_target_within_repo(repo_path: &Path, target_path: &Path) -> Result { + let canonical_repo = repo_path.canonicalize().map_err(|error| { + Error::Fix(format!( + "failed to canonicalize repository {}: {}", + repo_path.display(), error + )) + })?; + let absolute_target = if target_path.is_absolute() { + target_path.to_path_buf() + } else { + std::env::current_dir() + .map_err(|error| Error::Fix(format!("failed to resolve current directory: {error}")))? + .join(target_path) + }; + let lexical_target = normalise_path(&absolute_target); + let resolved_target = resolve_from_existing_ancestor(&lexical_target)?; + + if !resolved_target.starts_with(&canonical_repo) { + return Err(Error::Fix(format!( + "resolved target {} is outside repository {}", + resolved_target.display(), canonical_repo.display() + ))); + } + + Ok(resolved_target) +} + +/// Canonicalize the nearest existing ancestor, then append any missing final +/// components. A dangling symlink fails the boundary check closed. +fn resolve_from_existing_ancestor(path: &Path) -> Result { + let mut ancestor = path.to_path_buf(); let mut missing_components: Vec = Vec::new(); loop { - match fs::symlink_metadata(&candidate) { + match std::fs::symlink_metadata(&ancestor) { Ok(_) => { - let mut resolved = candidate.canonicalize()?; + let mut resolved = ancestor.canonicalize().map_err(|error| { + Error::Fix(format!( + "failed to canonicalize target ancestor {}: {}", + ancestor.display(), error + )) + })?; for component in missing_components.iter().rev() { resolved.push(component); } return Ok(normalise_path(&resolved)); } - Err(error) if error.kind() == io::ErrorKind::NotFound => { - let component = candidate.file_name().ok_or_else(|| { - io::Error::new( - io::ErrorKind::NotFound, - format!("no existing ancestor for {}", path.display()), - ) + Err(error) if error.kind() == std::io::ErrorKind::NotFound => { + let component = ancestor.file_name().ok_or_else(|| { + Error::Fix(format!("no existing ancestor for target {}", path.display())) })?; missing_components.push(component.to_os_string()); - if !candidate.pop() { - return Err(error); + if !ancestor.pop() { + return Err(Error::Fix(format!( + "no existing ancestor for target {}", path.display() + ))); } } - Err(error) => return Err(error), + Err(error) => return Err(Error::Fix(format!( + "failed to inspect target ancestor {}: {}", + ancestor.display(), error + ))), } } } -/// Replace a file atomically using a same-directory temporary file. -fn atomic_replace(target: &Path, content: &[u8]) -> io::Result<()> { - atomic_replace_with(target, content, |temporary, target| { - temporary - .persist(target) - .map(|_| ()) - .map_err(|error| error.error) - }) +fn is_escaped(value: &str, index: usize) -> bool { + value[..index].bytes().rev() + .take_while(|byte| *byte == b'\\') + .count() % 2 == 1 +} + +fn unescape_colons(value: &str) -> String { + let mut output = String::with_capacity(value.len()); + let mut characters = value.chars().peekable(); + while let Some(character) = characters.next() { + if character == '\\' && characters.peek() == Some(&':') { + characters.next(); + output.push(':'); + } else { + output.push(character); + } + } + output } -fn atomic_replace_with(target: &Path, content: &[u8], persist: F) -> io::Result<()> -where - F: FnOnce(tempfile::NamedTempFile, &Path) -> io::Result<()>, -{ - let parent = target.parent().ok_or_else(|| { - io::Error::new( - io::ErrorKind::InvalidInput, - format!("target has no parent directory: {}", target.display()), - ) +/// Stage replacement bytes beside the destination and atomically rename them +/// over it only after a complete, synced write. +fn atomic_replace(target_path: &Path, content: &[u8]) -> Result<()> { + let parent = target_path.parent().ok_or_else(|| { + Error::Fix(format!("Target {} has no parent directory", target_path.display())) })?; - let permissions = fs::metadata(target)?.permissions(); - let mut temporary = tempfile::NamedTempFile::new_in(parent)?; + let permissions = std::fs::metadata(target_path)?.permissions(); + let mut temporary = NamedTempFile::new_in(parent)?; temporary.write_all(content)?; + temporary.as_file_mut().flush()?; + temporary.as_file().sync_all()?; temporary.as_file().set_permissions(permissions)?; - temporary.as_file_mut().sync_all()?; - persist(temporary, target) + temporary.persist(target_path).map_err(|error| Error::Fix(format!( + "Failed to atomically replace {}: {}", + target_path.display(), error.error + )))?; + Ok(()) +} + +/// Stage a complete new file and publish it with no-clobber semantics. +fn persist_new_file(target_path: &Path, content: &[u8]) -> std::io::Result<()> { + let parent = target_path.parent().ok_or_else(|| std::io::Error::new( + std::io::ErrorKind::InvalidInput, + format!("Target {} has no parent directory", target_path.display()), + ))?; + let mut temporary = NamedTempFile::new_in(parent)?; + temporary.write_all(content)?; + temporary.as_file_mut().flush()?; + temporary.as_file().sync_all()?; + temporary.persist_noclobber(target_path) + .map(|_| ()) + .map_err(|error| error.error) } -/// Atomically rename without replacing an existing destination. -#[cfg(target_os = "linux")] -fn rename_noreplace(source: &Path, destination: &Path) -> io::Result<()> { +/// Rename without replacing an existing destination. +#[cfg(any(target_os = "linux", target_os = "android"))] +fn rename_noreplace(source: &Path, destination: &Path) -> std::io::Result<()> { rustix::fs::renameat_with( rustix::fs::CWD, source, rustix::fs::CWD, destination, rustix::fs::RenameFlags::NOREPLACE, - ) - .map_err(io::Error::from) + )?; + Ok(()) } -/// Fail closed on platforms without an atomic no-replace rename primitive. -#[cfg(not(target_os = "linux"))] -fn rename_noreplace(_source: &Path, _destination: &Path) -> io::Result<()> { - Err(io::Error::new( - io::ErrorKind::Unsupported, - "atomic no-replace rename is unsupported on this platform", - )) -} - -/// Normalise a path by resolving `.` and `..` components without requiring the -/// path to exist on disk (unlike `Path::canonicalize`). -/// -/// This is used for security validation: after normalisation we can check that -/// the path starts with the repository root and has not escaped via `..` traversal. -fn normalise_path(path: &Path) -> PathBuf { - use std::path::Component; - let mut normalised = PathBuf::new(); - for component in path.components() { - match component { - Component::ParentDir => { - // Pop the last element, effectively resolving ".." - normalised.pop(); - } - Component::CurDir => { - // Skip "." — it contributes nothing - } - other => { - normalised.push(other); - } - } +/// Portable, data-preserving fallback for platforms without renameat2. +#[cfg(not(any(target_os = "linux", target_os = "android")))] +fn rename_noreplace(source: &Path, destination: &Path) -> std::io::Result<()> { + std::fs::hard_link(source, destination)?; + if let Err(error) = std::fs::remove_file(source) { + let _cleanup_result = std::fs::remove_file(destination); + return Err(error); } - normalised + Ok(()) } #[cfg(test)] @@ -1165,173 +1169,173 @@ mod tests { } #[test] - fn test_replace_pattern_preserves_colons_in_regex() { + fn test_replace_pattern_preserves_url_colons() { let spec = Fixer::parse_modification("replace-pattern:https?://old:new").unwrap(); - let ModifySpec::ReplacePattern { - pattern, - replacement, - } = &spec - else { - panic!("expected replace-pattern specification"); - }; - assert_eq!(pattern, "https?://old"); - assert_eq!(replacement, "new"); - assert_eq!( - Fixer::apply_modification("http://old https://old", &spec).unwrap(), - "new new" - ); + match spec { + ModifySpec::ReplacePattern { pattern, replacement } => { + assert_eq!(pattern, "https?://old"); + assert_eq!(replacement, "new"); + } + other => panic!("unexpected specification: {other:?}"), + } } #[test] - fn test_replace_pattern_supports_escaped_colon_in_replacement() { - let spec = Fixer::parse_modification(r"replace-pattern:old:new\:value").unwrap(); - let ModifySpec::ReplacePattern { replacement, .. } = spec else { - panic!("expected replace-pattern specification"); - }; - assert_eq!(replacement, "new:value"); + fn test_replace_pattern_supports_escaped_replacement_colons() { + let spec = Fixer::parse_modification("replace-pattern:old:urn\\:new").unwrap(); + match spec { + ModifySpec::ReplacePattern { pattern, replacement } => { + assert_eq!(pattern, "old"); + assert_eq!(replacement, "urn:new"); + } + other => panic!("unexpected specification: {other:?}"), + } + } + + #[test] + fn test_replace_pattern_json_is_unambiguous() { + let spec = Fixer::parse_modification( + r#"replace-pattern-json:{"pattern":"https?://old","replacement":"urn:new"}"#, + ).unwrap(); + match spec { + ModifySpec::ReplacePattern { pattern, replacement } => { + assert_eq!(pattern, "https?://old"); + assert_eq!(replacement, "urn:new"); + } + other => panic!("unexpected specification: {other:?}"), + } } #[test] - fn test_modify_nul_content_is_rejected_without_changes() { + fn test_extensionless_binary_content_is_rejected() { let temp = TempDir::new().unwrap(); - let file_path = temp.path().join("data.txt"); - let original = b"text\0more text"; - fs::write(&file_path, original).unwrap(); + let file_path = temp.path().join("opaque-data"); + let original = b"text prefix\0binary payload"; + std::fs::write(&file_path, original).unwrap(); let fixer = Fixer::new(temp.path().to_path_buf(), false); let issue = make_issue("TEST-BINARY-CONTENT"); - let fix = Fix { - action: FixAction::Modify, - target: "data.txt".to_string(), - reason: None, - modification: Some("replace-line:1:changed".to_string()), - fallback: None, - }; + let mut fix = make_fix(FixAction::Modify, "opaque-data"); + fix.modification = Some("replace-line:1:hacked".to_string()); let result = fixer.apply(&issue, &fix).unwrap(); assert!(!result.success); - assert_eq!(fs::read(file_path).unwrap(), original); + assert_eq!(std::fs::read(&file_path).unwrap(), original); } #[test] - fn test_invalid_json_modification_is_not_persisted() { + fn test_invalid_rust_is_rejected_without_writing() { let temp = TempDir::new().unwrap(); - let file_path = temp.path().join("data.json"); - let original = "{\n \"valid\": true\n}\n"; - fs::write(&file_path, original).unwrap(); + let file_path = temp.path().join("main.rs"); + let original = "fn main() {}\n"; + std::fs::write(&file_path, original).unwrap(); let fixer = Fixer::new(temp.path().to_path_buf(), false); - let issue = make_issue("TEST-VALIDATION"); - let fix = Fix { - action: FixAction::Modify, - target: "data.json".to_string(), - reason: None, - modification: Some("replace-line:2:not valid json".to_string()), - fallback: None, - }; + let issue = make_issue("TEST-RUST-VALIDATION"); + let mut fix = make_fix(FixAction::Modify, "main.rs"); + fix.modification = Some("replace-line:1:fn main( {".to_string()); let result = fixer.apply(&issue, &fix).unwrap(); assert!(!result.success); - assert!(result.error.unwrap().contains("validation")); - assert_eq!(fs::read_to_string(file_path).unwrap(), original); + assert!(result.error.as_deref().unwrap().contains("Rust syntax validation")); + assert_eq!(std::fs::read_to_string(&file_path).unwrap(), original); + } + + #[test] + fn test_structured_source_validators_reject_invalid_content() { + let invalid_sources = [ + ("data.json", "{"), + ("events.jsonl", "{}\n{"), + ("workflow.yml", "key: [unterminated"), + ("config.toml", "key = ["), + ("rules.scm", "("), + ]; + + for (path, content) in invalid_sources { + assert!( + Fixer::validate_source(Path::new(path), content).is_err(), + "expected invalid {path} content to be rejected" + ); + } } #[test] - fn test_invalid_yaml_modification_is_not_persisted() { + fn test_disable_preserves_existing_disabled_file() { let temp = TempDir::new().unwrap(); - let file_path = temp.path().join("workflow.yml"); - let original = "name: CI\njobs:\n check:\n runs-on: ubuntu-latest\n"; - fs::write(&file_path, original).unwrap(); + let source = temp.path().join("workflow.yml"); + let disabled = temp.path().join("workflow.yml.disabled"); + std::fs::write(&source, "active workflow\n").unwrap(); + std::fs::write(&disabled, "previous disabled workflow\n").unwrap(); let fixer = Fixer::new(temp.path().to_path_buf(), false); - let issue = make_issue("TEST-YAML-VALIDATION"); - let fix = Fix { - action: FixAction::Modify, - target: "workflow.yml".to_string(), - reason: None, - modification: Some("replace-line:4: runs-on: [unterminated".to_string()), - fallback: None, - }; + let issue = make_issue("TEST-DISABLE-NOCLOBBER"); + let fix = make_fix(FixAction::Disable, "workflow.yml"); - let result = fixer.apply(&issue, &fix).unwrap(); - assert!(!result.success); - assert_eq!(fs::read_to_string(file_path).unwrap(), original); + assert!(fixer.apply(&issue, &fix).is_err()); + assert_eq!(std::fs::read_to_string(&source).unwrap(), "active workflow\n"); + assert_eq!( + std::fs::read_to_string(&disabled).unwrap(), + "previous disabled workflow\n" + ); } #[cfg(unix)] #[test] - fn test_symlinked_create_target_outside_repo_is_rejected() { + fn test_symlink_escape_create_is_rejected() { use std::os::unix::fs::symlink; - let repo = TempDir::new().unwrap(); - let outside = TempDir::new().unwrap(); - symlink(outside.path(), repo.path().join("escape")).unwrap(); + let temp = TempDir::new().unwrap(); + let outer = TempDir::new().unwrap(); + symlink(outer.path(), temp.path().join("outside-link")).unwrap(); - let fixer = Fixer::new(repo.path().to_path_buf(), false); + let fixer = Fixer::new(temp.path().to_path_buf(), false); let issue = make_issue("TEST-SYMLINK-ESCAPE"); - let fix = Fix { - action: FixAction::Create, - target: "escape/injected.txt".to_string(), - reason: None, - modification: None, - fallback: Some("must stay inside".to_string()), - }; + let mut fix = make_fix(FixAction::Create, "outside-link/injected.txt"); + fix.fallback = Some("must not escape".to_string()); let result = fixer.apply(&issue, &fix).unwrap(); assert!(!result.success); - assert!(!outside.path().join("injected.txt").exists()); + assert!(!outer.path().join("injected.txt").exists()); } #[test] - fn test_relative_repository_path_is_accepted() { + fn test_relative_repository_path_accepts_in_repo_target() { let current = std::env::current_dir().unwrap(); - let repo = tempfile::Builder::new() + let temp = tempfile::Builder::new() .prefix("fixer-relative-") .tempdir_in(¤t) .unwrap(); - let absolute_repo = repo.path().canonicalize().unwrap(); - let relative_repo = absolute_repo.strip_prefix(¤t).unwrap().to_path_buf(); - let file_path = absolute_repo.join("safe.txt"); - fs::write(&file_path, "safe").unwrap(); + let canonical_temp = temp.path().canonicalize().unwrap(); + let relative_repo = canonical_temp.strip_prefix(¤t).unwrap().to_path_buf(); + let file_path = canonical_temp.join("safe.txt"); + std::fs::write(&file_path, "before\n").unwrap(); let fixer = Fixer::new(relative_repo, false); let issue = make_issue("TEST-RELATIVE-REPO"); - let fix = make_fix(FixAction::Delete, "safe.txt"); + let mut fix = make_fix(FixAction::Modify, "safe.txt"); + fix.modification = Some("replace-line:1:after".to_string()); let result = fixer.apply(&issue, &fix).unwrap(); assert!(result.success); - assert!(!file_path.exists()); + assert_eq!(std::fs::read_to_string(file_path).unwrap(), "after\n"); } + #[cfg(unix)] #[test] - fn test_atomic_modify_failure_preserves_original_bytes() { + fn test_atomic_replace_failure_preserves_original_bytes() { + use std::os::unix::fs::PermissionsExt; + let temp = TempDir::new().unwrap(); let file_path = temp.path().join("protected.txt"); - let original = b"original\n"; - fs::write(&file_path, original).unwrap(); + let original = b"original content\n"; + std::fs::write(&file_path, original).unwrap(); - let result = atomic_replace_with(&file_path, b"changed\n", |_temporary, _target| { - Err(io::Error::other("simulated atomic persist failure")) - }); + let original_permissions = std::fs::metadata(temp.path()).unwrap().permissions(); + std::fs::set_permissions(temp.path(), std::fs::Permissions::from_mode(0o500)).unwrap(); + let result = atomic_replace(&file_path, b"replacement content\n"); + std::fs::set_permissions(temp.path(), original_permissions).unwrap(); assert!(result.is_err()); - assert_eq!(fs::read(file_path).unwrap(), original); - } - - #[test] - fn test_disable_does_not_replace_existing_destination() { - let temp = TempDir::new().unwrap(); - let source = temp.path().join("workflow.yml"); - let destination = temp.path().join("workflow.yml.disabled"); - fs::write(&source, "source").unwrap(); - fs::write(&destination, "existing").unwrap(); - - let fixer = Fixer::new(temp.path().to_path_buf(), false); - let issue = make_issue("TEST-DISABLE-NOREPLACE"); - let fix = make_fix(FixAction::Disable, "workflow.yml"); - - assert!(fixer.apply(&issue, &fix).is_err()); - assert_eq!(fs::read_to_string(source).unwrap(), "source"); - assert_eq!(fs::read_to_string(destination).unwrap(), "existing"); + assert_eq!(std::fs::read(&file_path).unwrap(), original); } } diff --git a/scripts/sync-all-parallel.exs b/scripts/sync-all-parallel.exs index 380c2b93..3a659ed1 100644 --- a/scripts/sync-all-parallel.exs +++ b/scripts/sync-all-parallel.exs @@ -142,7 +142,7 @@ defmodule SyncAll do defp default_repos_dir do case System.get_env("REPOS_BASE") do - base when is_binary(base) and base != "" -> base + base when is_binary(base) and byte_size(base) > 0 -> base _ -> Path.join(System.user_home!(), "developer/hyper-repos") end end From 1be2ded7a4c7691e3fcd5d35e67057039711fe3e Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Fri, 4 Sep 2026 19:16:54 +0100 Subject: [PATCH 5/6] ci: retrigger governance after allowlist repair Signed-off-by: Jonathan D.A. Jewell <6759885+hyperpolymath@users.noreply.github.com> From 4f5a3bfce51d890261223ad6efa2ee618e16bb9f Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Fri, 4 Sep 2026 19:37:54 +0100 Subject: [PATCH 6/6] ci(pages): require deployment-ready PR previews Signed-off-by: Jonathan D.A. Jewell <6759885+hyperpolymath@users.noreply.github.com> --- .github/workflows/actions.lock | 12 ++-- .github/workflows/casket-pages.yml | 105 +++++++++++++++++++++++++---- 2 files changed, 97 insertions(+), 20 deletions(-) diff --git a/.github/workflows/actions.lock b/.github/workflows/actions.lock index 8a665353..8d68ff43 100644 --- a/.github/workflows/actions.lock +++ b/.github/workflows/actions.lock @@ -10,8 +10,8 @@ workflows: - 'actions/checkout@v7.0.1' - 'actions/configure-pages@v6.0.0' - 'actions/deploy-pages@v5.0.0' + - 'actions/download-artifact@v8.0.1' - 'actions/upload-pages-artifact@v5.0.0' - - 'haskell-actions/setup@v2.12.0' '.github/workflows/codeql.yml': - 'actions/checkout@v7.0.1' - 'github/codeql-action@v4.37.8' @@ -75,6 +75,11 @@ dependencies: commit: 'sha1-cd2ce8fcbc39b97be8ca5fce6e763baed58fa128' owner_id: 44036562 repo_id: 438112499 + 'actions/download-artifact@v8.0.1': + ref: 'v8.0.1' + commit: 'sha1-3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c' + owner_id: 44036562 + repo_id: 192626254 'actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f': ref: 'v7.0.0' commit: 'sha1-bbbca2ddaa5d8feaa63e36b76fdaad77386f024f' @@ -102,11 +107,6 @@ dependencies: commit: 'sha1-db488ddef3bf6cb639b32c2e9a7c0a7ea8271d28' owner_id: 9919 repo_id: 259445878 - 'haskell-actions/setup@v2.12.0': - ref: 'v2.12.0' - commit: 'sha1-6037f33647c3f17758a2356c80fc4a53d7e0685d' - owner_id: 75048950 - repo_id: 623796603 'hyperpolymath/a2ml-ecosystem@main': ref: 'main' commit: 'sha1-aa4b836bd969df2bc58128cb8e3d20bbc88d5e79' diff --git a/.github/workflows/casket-pages.yml b/.github/workflows/casket-pages.yml index 9f5efb27..ce6b7216 100644 --- a/.github/workflows/casket-pages.yml +++ b/.github/workflows/casket-pages.yml @@ -1,42 +1,40 @@ # SPDX-License-Identifier: MPL-2.0 # This workflow is managed by gh actions-lock. -# This workflow is managed by gh actions-lock. name: GitHub Pages on: push: branches: [main, master] + pull_request: workflow_dispatch: permissions: - actions: read contents: read - pages: write - id-token: write concurrency: - group: "pages" - cancel-in-progress: false + group: "pages-${{ github.event_name }}-${{ github.ref }}" + cancel-in-progress: ${{ github.event_name == 'pull_request' }} jobs: build: - runs-on: ubuntu-latest + name: Build Pages artifact + runs-on: ubuntu-24.04 timeout-minutes: 30 + permissions: + contents: read steps: - name: Checkout uses: actions/checkout@v7.0.1 + with: + persist-credentials: false - name: Checkout casket-ssg uses: actions/checkout@v7.0.1 with: repository: hyperpolymath/casket-ssg + ref: 4abfd39c78c2eee9de62d658646607877b3ee157 # main 2026-09-03 path: .casket-ssg - - - name: Setup GHCup - uses: haskell-actions/setup@v2.12.0 - with: - ghc-version: '9.8.2' - cabal-version: '3.10' + persist-credentials: false - name: Cache Cabal uses: actions/cache@v6.1.0 @@ -47,6 +45,13 @@ jobs: .casket-ssg/dist-newstyle key: ${{ runner.os }}-casket-${{ hashFiles('.casket-ssg/casket-ssg.cabal') }} + - name: Prepare runner Haskell toolchain + run: | + set -euo pipefail + ghc --version + cabal --version + cabal update + - name: Build casket-ssg working-directory: .casket-ssg run: cabal build @@ -108,13 +113,85 @@ jobs: with: path: '_site' + preview: + name: Validate deployable Pages preview + if: github.event_name == 'pull_request' + environment: + name: pages-preview + runs-on: ubuntu-24.04 + needs: build + timeout-minutes: 10 + permissions: + actions: read + contents: read + steps: + - name: Download Pages artifact + uses: actions/download-artifact@v8.0.1 + with: + name: github-pages + path: .pages-preview + + - name: Validate deployable artifact + shell: bash + run: | + set -euo pipefail + + artifact=".pages-preview/artifact.tar" + entries_file="${RUNNER_TEMP}/pages-preview-entries.txt" + + if [ ! -s "${artifact}" ]; then + echo "::error::Pages artifact is absent or empty" + exit 1 + fi + + tar -tf "${artifact}" > "${entries_file}" + + entry_count=0 + has_index=0 + while IFS= read -r entry; do + entry_count=$((entry_count + 1)) + case "${entry}" in + /*|../*|*/../*|*/..) + echo "::error::Pages artifact contains an unsafe path: ${entry}" + exit 1 + ;; + index.html|*/index.html) + has_index=1 + ;; + esac + done < "${entries_file}" + + if [ "${entry_count}" -eq 0 ]; then + echo "::error::Pages artifact contains no files" + exit 1 + fi + + if [ "${has_index}" -ne 1 ]; then + echo "::error::Pages artifact contains no index.html" + exit 1 + fi + + { + echo "### Pages preview artifact" + echo + echo "- Files: ${entry_count}" + echo "- SHA-256: \`$(sha256sum "${artifact}" | awk '{print $1}')\`" + echo "- Production deployment: intentionally deferred until merge" + } >> "${GITHUB_STEP_SUMMARY}" + deploy: + name: Deploy production Pages site + if: github.event_name != 'pull_request' environment: name: github-pages url: ${{ steps.deployment.outputs.page_url }} - runs-on: ubuntu-latest + runs-on: ubuntu-24.04 needs: build timeout-minutes: 10 + permissions: + contents: read + pages: write + id-token: write steps: - name: Deploy to GitHub Pages id: deployment