Fix security, correctness, and docs issues in file-writing feature

README.md

@@ -27,19 +27,17 @@ When `write_files=true`, Grok parses code blocks for filename annotations and wr
 ### Supported Patterns
 
 **Fenced code blocks with path in the language tag:**
-```markdown
-```typescript:src/auth/login.ts
-export function login() { ... }
-```
-```
+
+    ```typescript:src/auth/login.ts
+    export function login() { ... }
+    ```
 
 **Fenced code blocks with `// FILE:` marker:**
-```markdown
-```typescript
-// FILE: src/auth/login.ts
-export function login() { ... }
-```
-```
+
+    ```typescript
+    // FILE: src/auth/login.ts
+    export function login() { ... }
+    ```
 
 ### Example
 
@@ -193,16 +191,16 @@ ls ~/.openclaw/skills/grok-refactor/grok_bridge.py
 
 Grok Swarm resolves your API key in this order (highest to lowest priority):
 
-1. **Environment variables** — `OPENROUTER_API_KEY` or `XAI_API_KEY`
-2. **Local config file** — `~/.config/grok-swarm/config.json` with `{"api_key": "..."}`
-3. **OpenClaw auth profiles** — `~/.openclaw/agents/coder/agent/auth-profiles.json`
+1. **Environment variables** — `OPENROUTER_API_KEY` or `OPENCLAW_OPENROUTER_DEFAULT_KEY`
+2. **OpenClaw auth profiles** — searched in order:
+   - `~/.openclaw/agents/coder/agent/auth-profiles.json`
+   - `~/.openclaw/agents/main/agent/auth-profiles.json`
+   - `~/.openclaw/auth-profiles.json`
+   - `~/.config/openclaw/auth-profiles.json`
 
 ```bash
-# If you set an env var, it takes precedence over config files:
-export OPENROUTER_API_KEY="sk-or-v1-xxx"   # This overrides ~/.config/grok-swarm/config.json!
-
-# To use the local config file instead, unset the env var:
-unset OPENROUTER_API_KEY
+# Set via environment variable (highest priority):
+export OPENROUTER_API_KEY="sk-or-v1-xxx"
 ```
 
 **Get a key at:** https://openrouter.ai/keys

src/bridge/grok_bridge.py

@@ -132,6 +132,26 @@ def load_tools(tools_path):
     return tools
 
 
+def _safe_dest(output_path, file_path):
+    """
+    Resolve ``file_path`` relative to ``output_path`` and verify the result
+    stays inside ``output_path``.  Returns the resolved Path or raises
+    ValueError for unsafe paths (absolute, containing ``..``, etc.).
+    """
+    raw = Path(file_path)
+    if raw.is_absolute():
+        raise ValueError(f"Absolute paths are not allowed: {file_path!r}")
+    if ".." in raw.parts:
+        raise ValueError(f"Path traversal not allowed: {file_path!r}")
+    dest = (output_path / raw).resolve()
+    resolved_root = output_path.resolve()
+    try:
+        dest.relative_to(resolved_root)
+    except ValueError:
+        raise ValueError(f"Path escapes output directory: {file_path!r}")
+    return dest
+
+
 def parse_and_write_files(response_text, output_dir):
     """
     Scan response for fenced code blocks with filename annotations and write to disk.
@@ -143,7 +163,8 @@ def parse_and_write_files(response_text, output_dir):
       ...
       ```
 
-    Returns list of (relative_path, byte_count) tuples written.
+    Returns list of (relative_path, byte_count) tuples written, where
+    byte_count is the number of UTF-8 bytes written.
     """
     written = []
     output_path = Path(output_dir)
@@ -153,30 +174,33 @@ def parse_and_write_files(response_text, output_dir):
     # Pattern for // FILE: or # FILE: markers
     file_marker_pattern = re.compile(r'^\s*(?://|#)\s*FILE:\s*(.+?)\s*$', re.MULTILINE)
 
+    def _write_file(file_path, content):
+        """Validate path, write content, and record result. Returns True on success."""
+        try:
+            dest = _safe_dest(output_path, file_path)
+        except ValueError as exc:
+            print(f"WARNING: Skipping unsafe path — {exc}", file=sys.stderr)
+            return False
+        dest.parent.mkdir(parents=True, exist_ok=True)
+        encoded = content.strip().encode("utf-8", errors="replace")
+        dest.write_bytes(encoded)
+        written.append((file_path, len(encoded)))
+        return True
+
     # Split into code blocks by ``` fences
     parts = re.split(r'```', response_text)
 
     for part in parts:
         # Check for lang:path at start (language tag contains the path)
         lang_match = lang_path_pattern.match(part)
         if lang_match:
-            file_path = lang_match.group(2)
-            content = part[lang_match.end():]
-            dest = output_path / file_path
-            dest.parent.mkdir(parents=True, exist_ok=True)
-            byte_count = dest.write_text(content.strip(), errors='replace')
-            written.append((file_path, byte_count))
+            _write_file(lang_match.group(2), part[lang_match.end():])
             continue
 
         # Check for // FILE: or # FILE: marker within the block
         marker_match = file_marker_pattern.search(part)
         if marker_match:
-            file_path = marker_match.group(1).strip()
-            content = part[marker_match.end():]
-            dest = output_path / file_path
-            dest.parent.mkdir(parents=True, exist_ok=True)
-            byte_count = dest.write_text(content.strip(), errors='replace')
-            written.append((file_path, byte_count))
+            _write_file(marker_match.group(1).strip(), part[marker_match.end():])
 
     return written
 
@@ -325,20 +349,23 @@ def main():
     if args.output:
         Path(args.output).write_text(result)
         print(f"Written to: {args.output}", file=sys.stderr)
-    else:
-        if args.write_files:
-            written = parse_and_write_files(result, args.output_dir)
-            if written:
-                total_bytes = sum(b for _, b in written)
-                print(f"Wrote {len(written)} files to {args.output_dir}")
-                for rel_path, byte_count in written:
-                    print(f"  {rel_path} ({byte_count:,} bytes)")
-                print(f"Total: {total_bytes:,} bytes", file=sys.stderr)
-            else:
-                print("No annotated files found in response", file=sys.stderr)
-                print(result)
+
+    if args.write_files:
+        written = parse_and_write_files(result, args.output_dir)
+        if written:
+            total_bytes = sum(b for _, b in written)
+            print(f"Wrote {len(written)} files to {args.output_dir}")
+            for rel_path, byte_count in written:
+                print(f"  {rel_path} ({byte_count:,} bytes)")
+            print(f"Total: {total_bytes:,} bytes")
         else:
-            print(result)
+            print(
+                "No annotated files found in model response to write to disk.\n"
+                "Re-run without --write-files to see the full response.",
+                file=sys.stderr,
+            )
+    elif not args.output:
+        print(result)
 
 
 if __name__ == "__main__":