Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions docs/audits/PHASE-1-CONSOLIDATED-TRIAGE.md
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,8 @@ If you do all 8: you have layered defenses against D-1 / D-2 / D-3 / D-4 / D-7 u
| ID | Audit | Title | Wave |
|---|---|---|---|
| **D-1** | Security | Skill registry lazy-load = RCE for anyone who can drop a `.py` file | **W1 β€” CLOSED ([#42](https://github.com/AVADSA25/codec/pull/42), `48ec5d5`)** |
| **D-2** | Security | `/api/forge` fetches arbitrary URL β†’ LLM β†’ writes skill, no review gate | **W1 β€” CLOSED (PR-1B)** |
| **D-3** | Security | `/api/save_skill` writes directly to skills/ with only substring check | **W1 β€” CLOSED (PR-1B)** |
| **D-2** | Security | `/api/forge` fetches arbitrary URL β†’ LLM β†’ writes skill, no review gate | **W1 β€” CLOSED ([#43](https://github.com/AVADSA25/codec/pull/43), `ff16664`)** |
| **D-3** | Security | `/api/save_skill` writes directly to skills/ with only substring check | **W1 β€” CLOSED ([#43](https://github.com/AVADSA25/codec/pull/43), `ff16664`)** |
| **D-4** | Security | `file_write` skill (MCP-exposed) can write to `~/.codec/skills/` | **W1** |
| **D-5** | Security | `permission_gate` accepts path-traversal via `fnmatch` (no realpath) | **W1** |
| C-1 | Reliability | `codec.py` daemon ignores SIGINT/SIGTERM; leaks sox + tkinter on every restart | W4 |
Expand Down
4 changes: 2 additions & 2 deletions docs/audits/PHASE-1-SECURITY.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ No exploits were executed. All bypasses were analyzed against source as text-onl
### D-2 β€” `/api/forge` endpoint fetches arbitrary URL β†’ LLM β†’ writes skill, no review gate [CRITICAL]
**Location:** `routes/skills.py:71-201` (`forge_skill`)

> **Closed by PR-1B** (branch `fix/pr1b-remove-save-skill-and-forge`). The `/api/forge` endpoint and its handler `forge_skill` were removed entirely from `routes/skills.py`. Skill creation now routes exclusively through `/api/skill/review` β†’ `/api/skill/approve` (the human-review-and-approve flow). The URL-fetch capability is intentionally dropped per Mickael decision Q1 β€” users wanting to import code from a URL paste the source into the editor and go through the review-and-approve flow. The Skill Forge modal + toolbar button + JS handlers in `codec_vibe.html` were removed alongside. 9 tests in `tests/test_skill_routes.py` verify the endpoint returns 404 and that the replacement flow remains functional. AGENTS.md Β§7 documents the contributor / operator workflow.
> **Closed by PR-1B** ([#43](https://github.com/AVADSA25/codec/pull/43), merged as `ff16664`). The `/api/forge` endpoint and its handler `forge_skill` were removed entirely from `routes/skills.py`. Skill creation now routes exclusively through `/api/skill/review` β†’ `/api/skill/approve` (the human-review-and-approve flow). The URL-fetch capability is intentionally dropped per Mickael decision Q1 β€” users wanting to import code from a URL paste the source into the editor and go through the review-and-approve flow. The Skill Forge modal + toolbar button + JS handlers in `codec_vibe.html` were removed alongside. 9 tests in `tests/test_skill_routes.py` verify the endpoint returns 404 and that the replacement flow remains functional. AGENTS.md Β§7 documents the contributor / operator workflow.
**CWE / OWASP:** CWE-94 Code Injection / CWE-918 SSRF / OWASP A10 SSRF / Agentic A01 Memory Poisoning, A02 Tool Misuse
**Description:** Endpoint accepts `code` field; if it starts with `http://` or `https://`, fetches the URL via `requests.get(code, timeout=15)` with no localhost / internal IP filtering, no domain allowlist, no MIME check. The fetched body is then passed to the LLM. The LLM's output is filtered through a 10-pattern substring blocklist (line 170-173: `os.system(`, `subprocess.`, `eval(`, `exec(`, `__import__`, `importlib`, `shutil.rmtree`, `open('/etc`, `open('/dev`, `ctypes`) β€” bypassable in dozens of ways: split string concatenation (`'os.sys' + 'tem('`), getattr indirection (`getattr(__builtins__, chr(95)*2+'import'+chr(95)*2)`), or simply double-quote variants of the `open('/...` patterns (the blocker only matches single-quote `open('/etc`).
After substring check + AST compile (which only catches syntax errors), the code is **written directly to `<repo>/skills/<name>.py` AND `~/.codec/skills/<name>.py`** (line 186-190). No `/api/skill/review` staging.
Expand All @@ -84,7 +84,7 @@ After substring check + AST compile (which only catches syntax errors), the code
### D-3 β€” `/api/save_skill` writes directly to `skills/` with only substring check [CRITICAL]
**Location:** `routes/skills.py:14-28` (`save_skill`)

> **Closed by PR-1B** (branch `fix/pr1b-remove-save-skill-and-forge`). The `/api/save_skill` endpoint and its handler `save_skill` were removed entirely from `routes/skills.py`. Skill creation now routes exclusively through `/api/skill/review` β†’ `/api/skill/approve` (the human-review-and-approve flow), which already runs `is_dangerous_skill_code` at the approve gate. The "Skill" toolbar button in `codec_vibe.html` and the `doSkill()` JS function were removed alongside. Defense-in-depth with PR-1A: even if a malicious file reaches `~/.codec/skills/` via some other path, `SkillRegistry.load` refuses it at load time. Same 9 tests in `tests/test_skill_routes.py` cover D-2 + D-3.
> **Closed by PR-1B** ([#43](https://github.com/AVADSA25/codec/pull/43), merged as `ff16664`). The `/api/save_skill` endpoint and its handler `save_skill` were removed entirely from `routes/skills.py`. Skill creation now routes exclusively through `/api/skill/review` β†’ `/api/skill/approve` (the human-review-and-approve flow), which already runs `is_dangerous_skill_code` at the approve gate. The "Skill" toolbar button in `codec_vibe.html` and the `doSkill()` JS function were removed alongside. Defense-in-depth with PR-1A: even if a malicious file reaches `~/.codec/skills/` via some other path, `SkillRegistry.load` refuses it at load time. Same 9 tests in `tests/test_skill_routes.py` cover D-2 + D-3.
**CWE / OWASP:** CWE-94 Code Injection / OWASP A04 Insecure Design
**Description:** `/api/save_skill` writes the request body's `content` field directly to `<skills_dir>/<filename>` after only:
1. Checking that "SKILL_DESCRIPTION" and "def run(" are in the content (presence check, not security).
Expand Down
Loading