fix(security): D-2 + D-3 — remove /api/save_skill and /api/forge#43
Merged
Conversation
… D-3)
Closes the two write-path criticals from Phase 1 Audit D (see
docs/audits/PHASE-1-SECURITY.md):
D-2 /api/forge — URL fetch → LLM → write skill, no review gate
D-3 /api/save_skill — direct write with only a substring blocker
Both endpoints are removed entirely. 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 — anyone wanting to
import skill code from a URL pastes the source into the editor and
goes through the review-and-approve flow like any other skill.
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 via the manifest + AST gate.
Live proof of D-2: the initial RED-phase test_post_forge_returns_404
actually called the live LLM, fetched example.com, AND wrote a real
fetch_example_domain_html.py to skills/ before the endpoint was
removed. The endpoint was a working SSRF + RCE-write primitive on
this machine right up to the moment it was deleted.
Changes:
Code:
- routes/skills.py:
* delete async def save_skill (D-3 handler)
* delete async def forge_skill (D-2 handler)
* module docstring updated to reflect the review-and-approve-only
contract; unused imports (DASHBOARD_DIR, CONFIG_PATH) dropped;
import style PEP8'd
- codec_vibe.html:
* remove the entire Forge Modal (HTML markup + overlay + tabs)
* remove .fm-* CSS block
* remove Skill + Forge toolbar buttons (kept Test button — it
calls the independent /api/run_code, not the removed endpoints)
* remove JS: doSkill, doForge, submitForge, closeForgeModal,
setForgeMode, _forgeMode, fmOverlay event handlers
* update Vibe system prompt — no more mention of Skill Forge
- scripts/feature_audit.py:
* features 10 and 12 now assert 404 (proof of removal) instead
of "endpoint responsive"
Docs / UI references:
- AGENTS.md §7: new "Skill creation flow — review-and-approve only"
subsection above the Skill load-time safety gate subsection.
Documents D-2 + D-3 removal trail.
- codec_cortex.html: CODEC Vibe panel rewritten — "Skill Forge"
removed from subtitle, features, description.
- codec_chat.html: Vibe blurb updated.
- README.md: 5 mentions updated (table row, section heading,
"Writes its own plugins" comparison row, IDE comparison row,
Project Structure description).
- FEATURES.md: Vibe feature list renumbered — Skill Forge items
removed; the human-review approval workflow stays as item 17.
- setup_codec.py:457: Vibe install description.
- docs/API.md: /api/forge section removed; /api/skill/review +
/api/skill/approve are now the documented contract.
Tests:
- NEW tests/test_skill_routes.py — 9 tests covering removal +
replacement:
* test_save_skill_handler_removed
* test_forge_skill_handler_removed
* test_no_route_decorator_strings_for_removed_endpoints
* test_replacement_handlers_present
* test_post_save_skill_returns_404
* test_post_forge_returns_404
* test_post_skill_review_still_accepts_valid_body
* test_skill_review_rejects_empty_body
* test_skill_approve_writes_only_after_review (full review →
approve flow exercised via FastAPI TestClient, asserts
no-disk-write at review step and disk-write at approve step)
- tests/test_dashboard.py:
* test_forge_requires_input replaced with
test_forge_endpoint_removed (asserts 404)
* added test_save_skill_endpoint_removed
- tests/test_full_product_audit.py:
* test_forge_endpoint replaced with test_forge_endpoint_removed
- tests/test_critical_fixes.py:
* TestSkillForgeBlocklist class removed entirely. The substring
blocker it tested is gone with the endpoints. Dangerous-pattern
coverage now lives in tests/test_skill_registry.py (load-time
AST gate) and routes/skills.py:skill_approve (write-time AST).
* Note: this class was already failing on main per
docs/PHASE1-STEP1-PREMERGE-AUDIT.md row #4 because it referenced
codec_dashboard.save_skill which never existed on main; the
deletion is a positive net for the test suite.
- tests/test_security.py:
* test_save_skill_validates_content removed. Same root cause as
above — referenced codec_dashboard.save_skill, was already
failing on main per the pre-merge audit row #20.
Audit closure:
- docs/audits/PHASE-1-SECURITY.md:
* D-2 closure footnote appended
* D-3 closure footnote appended
- docs/audits/PHASE-1-CONSOLIDATED-TRIAGE.md:
* D-2 row status: W1 → W1 — CLOSED (PR-1B)
* D-3 row status: W1 → W1 — CLOSED (PR-1B)
Verification:
- pytest tests/test_skill_routes.py — 9 passed (RED watched,
then GREEN after endpoint removal)
- pytest tests/test_skill_registry.py — 13 passed (PR-1A tests
still green)
- pytest tests/test_skill_contracts.py — 1 passed
- pytest tests/test_oauth_provider.py + test_retry.py — passed
- python3 tests/test_skill_imports.py — 76 skills parsed, 0 errors
- python3 tools/generate_skill_manifest.py --check — manifest current
- ruff check routes/skills.py tests/test_skill_routes.py — clean
- Full regression against PR-1A baseline (stash-and-rerun): same
12 pre-existing failures, no new regressions. Net failures count
drops by 2 (the deleted broken tests).
Out of scope (later PRs):
- D-4 file_write block-roots expansion (PR-1C)
- D-5 permission_gate realpath + path-blocklist (PR-1D)
- D-17 positive-allowlist for is_dangerous_skill_code (optional PR-1E)
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
PR-1A (#42) merged to main as squash commit 48ec5d5. Update the D-1 closure footnote in docs/audits/PHASE-1-SECURITY.md and the D-1 row in docs/audits/PHASE-1-CONSOLIDATED-TRIAGE.md to reference the PR number + commit hash, replacing the branch-name placeholder. This commit lands on the PR-1B branch (fix/pr1b-remove-save-skill-and-forge) so the citation travels with PR-1B's next merge. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
CI failure root cause for PR-1A (#42) and PR-1B (#43): the smoke job on Ubuntu runners failed at test collection because importing codec_skill_registry → codec_config → `from pynput import keyboard` raised ImportError on a headless display: ImportError: this platform is not supported: ('failed to acquire X connection: Bad display name ""') pynput needs a real display (X11 / AppKit / win32). GitHub Actions Linux runners are headless. Other modules import codec_config for its constants and is_dangerous_skill_code — none of them need the keyboard subsystem. Fix: wrap the pynput import in try/except so codec_config is import- safe in headless contexts. The two `getattr(keyboard.Key, ...)` call sites in `_resolve_key` now early-return None when keyboard is None (the resulting KEY_TOGGLE / KEY_VOICE / KEY_TEXT module constants are None on headless systems — only matters if the live keyboard listener actually runs, which it doesn't in CI). This was triggered by the new PR-1A test `tests/test_skill_registry.py` which CI now runs per the new `Skill registry load-time AST gate tests (D-1)` step. Before PR-1A, no CI-gated test imported codec_config, so the pynput crash was latent. Production paths (codec.py daemon, dashboard, etc.) all run on macOS where pynput imports fine. Verified locally: - pytest tests/test_skill_routes.py tests/test_skill_registry.py tests/test_skill_contracts.py tests/test_oauth_provider.py tests/test_retry.py → 28 passed - python3 tests/test_skill_imports.py → 76 skills parsed, 0 errors - python3 tools/generate_skill_manifest.py --check → ok - Headless simulation: monkey-patch builtins.__import__ to raise on pynput, then `from codec_config import is_dangerous_skill_code` → succeeds, returns (True, 'Dangerous import: subprocess') correctly. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
6286b17 to
2590f02
Compare
AVADSA25
added a commit
that referenced
this pull request
May 17, 2026
PR-1B (#43) merged to main as squash commit ff16664. Update the closure footnotes in docs/audits/PHASE-1-SECURITY.md (D-2 + D-3) and the corresponding rows in docs/audits/PHASE-1-CONSOLIDATED-TRIAGE.md, replacing the branch-name placeholders with PR number + commit hash. Mirrors the citation style applied to D-1 in the PR-1A closure trail. Co-authored-by: Mickael Farina <farina.mickael@gmail.com> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
8 tasks
AVADSA25
pushed a commit
that referenced
this pull request
May 17, 2026
…5+D-14+D-16)
Closes three Phase 1 Audit D findings in one cohesive change to the
Step 9 agent runtime:
D-5 CRITICAL permission_gate accepts path-traversal via fnmatch
D-14 MEDIUM _PATH_BLOCKLIST_SUBSTRINGS misses ~/.codec/skills + plugins
D-16 MEDIUM blocklist substring match is anchorless
All three are auto-extract / runtime paths that an LLM-drafted plan
could use to chain into D-1 RCE. Each layer closes independently.
codec_agent_runner.py — permission_gate refactor (D-5):
- New helper _path_allowed(action_path, grants) → (bool, reason):
* reject `..` segments outright (closes the dotdot bypass that
fnmatch glob-matched against grants like ~/Documents/**)
* os.path.realpath both sides — symlink-out-of-grant rejected
* fnmatch replaced with action_real.startswith(grant_real + os.sep)
Trade-off: a grant like ~/Documents/*.md now accepts any file
under realpath(~/Documents/) — safety > granularity per audit.
- New helper _emit_gate_blocked(action_path, reason, agent_id):
emits `permission_gate_blocked` audit event before raising
PermissionViolation. source=codec-agent-runner, outcome=error,
level=warning, extra={requested_path, resolved_path, reason,
agent_id}. Forensic visibility per D-5 closure §3.
- Removed unused `import fnmatch` (no other callers in the module).
codec_agent_plan.py — blocklist extension + segment-aware (D-14+D-16):
- _PATH_BLOCKLIST_SUBSTRINGS extended with 8 new entries (D-14):
/.codec/skills /.codec/plugins
/.codec/oauth_state.json /.codec/audit.log
/.codec/agents /.codec/agent_global_grants.json
/.codec/config.json /.codec/memory.db
- New helper _path_segments_match(path, pattern) does segment-aware
matching (D-16): splits both `path` (after expanduser+normpath) and
`pattern` on `/`, requires consecutive subsequence of segments.
`~/Documents/notes_ssh/foo.md` no longer matches `/.ssh` (segment
is `notes_ssh`, not `.ssh`), but `~/.ssh/config` still does.
- New helper _is_path_blocklisted(path) walks the full blocklist.
- extract_user_paths now calls _is_path_blocklisted instead of the
raw substring `any(b in raw for b in _PATH_BLOCKLIST_SUBSTRINGS)`.
Tests:
- tests/test_agent_runner.py +5 new (143 LOC):
test_permission_gate_rejects_path_traversal_dotdot
test_permission_gate_rejects_read_path_traversal
test_permission_gate_rejects_symlink_outside_grant
test_permission_gate_accepts_realpath_within_grant
test_permission_gate_emits_blocked_audit_event
- tests/test_agent_plan.py +10 new (80 LOC, 7 parametrized + 3 misc):
test_extract_user_paths_blocks_sensitive_codec_dirs (×7)
test_extract_user_paths_segment_aware_not_false_positive
test_extract_user_paths_still_blocks_real_ssh_path
test_extract_user_paths_allows_legitimate_user_paths
Verification:
- pytest tests/test_agent_runner.py tests/test_agent_plan.py
tests/test_file_write.py tests/test_skill_routes.py
tests/test_skill_registry.py tests/test_skill_contracts.py
→ 148 passed (full Wave 1 regression + new)
- python3 tests/test_skill_imports.py → 76 parsed, 0 errors
- python3 tools/generate_skill_manifest.py --check → ok
- ruff check codec_agent_runner.py codec_agent_plan.py
→ 5 errors in codec_agent_runner.py + 22 in codec_agent_plan.py,
all pre-existing on main (F401 unused imports `field`, `time`,
E402 inline imports for regex, F541 f-string-without-placeholder,
F821 List/Tuple undefined). The one new error introduced —
`fnmatch` no longer used in codec_agent_runner.py — has been
cleaned up (import removed).
Docs:
- AGENTS.md §7 new "Agent permission gate + path blocklist
(Phase 1 Wave 1, PR-1D — closes D-5 + D-14 + D-16)" subsection
documenting the four-layer defense (PR-1A + PR-1C + PR-1D blocklist
+ PR-1D runtime gate) against the D-1 RCE chain.
- docs/audits/PHASE-1-SECURITY.md: closure footnotes on D-5, D-14,
D-16.
- docs/audits/PHASE-1-CONSOLIDATED-TRIAGE.md: D-5 row flipped to
W1 — CLOSED (PR-1D). D-14 and D-16 are MEDIUM (not in the
CRITICAL findings table) but their D-section footnotes carry the
closure trail.
Wave 1 status after this PR merges:
- D-1 ✅ closed (PR-1A, #42, 48ec5d5)
- D-2 ✅ closed (PR-1B, #43, ff16664)
- D-3 ✅ closed (PR-1B, #43, ff16664)
- D-4 ✅ closed (PR-1C, #45, 0065d90)
- D-5 ✅ closed (this PR)
- D-14 ✅ closed (this PR — bonus)
- D-16 ✅ closed (this PR — bonus)
All five CRITICAL skill-loading + write-path findings are closed.
Sample audit line emitted on a blocked traversal attempt:
{"ts": "2026-05-17T...Z",
"schema": 1,
"event": "permission_gate_blocked",
"source": "codec-agent-runner",
"outcome": "error",
"level": "warning",
"message": "permission_gate refused '~/Documents/../../etc/passwd': path_traversal",
"extra": {
"requested_path": "~/Documents/../../etc/passwd",
"resolved_path": "/etc/passwd",
"reason": "path_traversal",
"agent_id": ""
}}
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
AVADSA25
added a commit
that referenced
this pull request
May 17, 2026
…5+D-14+D-16) (#47) Closes three Phase 1 Audit D findings in one cohesive change to the Step 9 agent runtime: D-5 CRITICAL permission_gate accepts path-traversal via fnmatch D-14 MEDIUM _PATH_BLOCKLIST_SUBSTRINGS misses ~/.codec/skills + plugins D-16 MEDIUM blocklist substring match is anchorless All three are auto-extract / runtime paths that an LLM-drafted plan could use to chain into D-1 RCE. Each layer closes independently. codec_agent_runner.py — permission_gate refactor (D-5): - New helper _path_allowed(action_path, grants) → (bool, reason): * reject `..` segments outright (closes the dotdot bypass that fnmatch glob-matched against grants like ~/Documents/**) * os.path.realpath both sides — symlink-out-of-grant rejected * fnmatch replaced with action_real.startswith(grant_real + os.sep) Trade-off: a grant like ~/Documents/*.md now accepts any file under realpath(~/Documents/) — safety > granularity per audit. - New helper _emit_gate_blocked(action_path, reason, agent_id): emits `permission_gate_blocked` audit event before raising PermissionViolation. source=codec-agent-runner, outcome=error, level=warning, extra={requested_path, resolved_path, reason, agent_id}. Forensic visibility per D-5 closure §3. - Removed unused `import fnmatch` (no other callers in the module). codec_agent_plan.py — blocklist extension + segment-aware (D-14+D-16): - _PATH_BLOCKLIST_SUBSTRINGS extended with 8 new entries (D-14): /.codec/skills /.codec/plugins /.codec/oauth_state.json /.codec/audit.log /.codec/agents /.codec/agent_global_grants.json /.codec/config.json /.codec/memory.db - New helper _path_segments_match(path, pattern) does segment-aware matching (D-16): splits both `path` (after expanduser+normpath) and `pattern` on `/`, requires consecutive subsequence of segments. `~/Documents/notes_ssh/foo.md` no longer matches `/.ssh` (segment is `notes_ssh`, not `.ssh`), but `~/.ssh/config` still does. - New helper _is_path_blocklisted(path) walks the full blocklist. - extract_user_paths now calls _is_path_blocklisted instead of the raw substring `any(b in raw for b in _PATH_BLOCKLIST_SUBSTRINGS)`. Tests: - tests/test_agent_runner.py +5 new (143 LOC): test_permission_gate_rejects_path_traversal_dotdot test_permission_gate_rejects_read_path_traversal test_permission_gate_rejects_symlink_outside_grant test_permission_gate_accepts_realpath_within_grant test_permission_gate_emits_blocked_audit_event - tests/test_agent_plan.py +10 new (80 LOC, 7 parametrized + 3 misc): test_extract_user_paths_blocks_sensitive_codec_dirs (×7) test_extract_user_paths_segment_aware_not_false_positive test_extract_user_paths_still_blocks_real_ssh_path test_extract_user_paths_allows_legitimate_user_paths Verification: - pytest tests/test_agent_runner.py tests/test_agent_plan.py tests/test_file_write.py tests/test_skill_routes.py tests/test_skill_registry.py tests/test_skill_contracts.py → 148 passed (full Wave 1 regression + new) - python3 tests/test_skill_imports.py → 76 parsed, 0 errors - python3 tools/generate_skill_manifest.py --check → ok - ruff check codec_agent_runner.py codec_agent_plan.py → 5 errors in codec_agent_runner.py + 22 in codec_agent_plan.py, all pre-existing on main (F401 unused imports `field`, `time`, E402 inline imports for regex, F541 f-string-without-placeholder, F821 List/Tuple undefined). The one new error introduced — `fnmatch` no longer used in codec_agent_runner.py — has been cleaned up (import removed). Docs: - AGENTS.md §7 new "Agent permission gate + path blocklist (Phase 1 Wave 1, PR-1D — closes D-5 + D-14 + D-16)" subsection documenting the four-layer defense (PR-1A + PR-1C + PR-1D blocklist + PR-1D runtime gate) against the D-1 RCE chain. - docs/audits/PHASE-1-SECURITY.md: closure footnotes on D-5, D-14, D-16. - docs/audits/PHASE-1-CONSOLIDATED-TRIAGE.md: D-5 row flipped to W1 — CLOSED (PR-1D). D-14 and D-16 are MEDIUM (not in the CRITICAL findings table) but their D-section footnotes carry the closure trail. Wave 1 status after this PR merges: - D-1 ✅ closed (PR-1A, #42, 48ec5d5) - D-2 ✅ closed (PR-1B, #43, ff16664) - D-3 ✅ closed (PR-1B, #43, ff16664) - D-4 ✅ closed (PR-1C, #45, 0065d90) - D-5 ✅ closed (this PR) - D-14 ✅ closed (this PR — bonus) - D-16 ✅ closed (this PR — bonus) All five CRITICAL skill-loading + write-path findings are closed. Sample audit line emitted on a blocked traversal attempt: {"ts": "2026-05-17T...Z", "schema": 1, "event": "permission_gate_blocked", "source": "codec-agent-runner", "outcome": "error", "level": "warning", "message": "permission_gate refused '~/Documents/../../etc/passwd': path_traversal", "extra": { "requested_path": "~/Documents/../../etc/passwd", "resolved_path": "/etc/passwd", "reason": "path_traversal", "agent_id": "" }} Co-authored-by: Mickael Farina <farina.mickael@gmail.com> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
AVADSA25
added a commit
that referenced
this pull request
May 17, 2026
#48) PR-1D (#47) merged to main as squash commit fd2b460. Update the closure footnotes for D-5, D-14, and D-16 in docs/audits/PHASE-1-SECURITY.md plus the D-5 row in docs/audits/PHASE-1-CONSOLIDATED-TRIAGE.md, replacing the branch-name placeholders with PR number + commit hash. Mirrors the citation style applied to: D-1 PR-1A #42 → 48ec5d5 D-2/3 PR-1B #43 → ff16664 D-4 PR-1C #45 → 0065d90 D-5 PR-1D #47 → fd2b460 (this commit) After this lands, Wave 1 is fully closed with complete citation trails. All five CRITICAL skill-loading + write-path findings (D-1, D-2, D-3, D-4, D-5) plus the two bonus mediums (D-14, D-16) carry merge commit hashes in their audit-doc footnotes. Co-authored-by: Mickael Farina <farina.mickael@gmail.com> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Phase 1 Wave 1 — PR-1B. Closes D-2 (CRITICAL) and D-3 (CRITICAL) — see docs/audits/PHASE-1-SECURITY.md findings D-2 + D-3 and the consolidated triage.
/api/save_skill(D-3)<skills_dir>/<name>.pyafter a 10-substring blocker/api/forge(D-2)<repo>/skills/AND~/.codec/skills/)The replacement (pre-existing, unchanged):
POST /api/skill/review→ user reviews →POST /api/skill/approve. The approve step runsis_dangerous_skill_code(AST) as the write-time gate. Defense in depth pairs with PR-1A: even if a malicious file reaches~/.codec/skills/via some other path,SkillRegistry.loadrefuses it at load time via the manifest + AST gate.Live proof of D-2 being exploitable
The TDD RED-phase test_post_forge_returns_404 actually hit the live endpoint on this machine before removal — and it succeeded. It fetched example.com, called the local LLM, AND wrote a real skill file
skills/fetch_example_domain_html.pyto disk. Cleaned up + endpoint deleted in the same PR. That artifact's existence in test output is the strongest single proof of the vulnerability being a working SSRF + RCE-write primitive on day-zero (not theoretical).Files changed (17)
Code
routes/skills.py— deletesave_skill+forge_skillhandlers; clean up unused imports; module docstring rewritten to reflect review-and-approve-only contract.codec_vibe.html— remove the entire Skill Forge modal (HTML + CSS + JS): markup,.fm-*CSS, "Skill" + "Forge" toolbar buttons (kept "Test" — it calls independent/api/run_code), 5 JS functions (doSkill,doForge,submitForge,closeForgeModal,setForgeMode),_forgeModeglobal, modal event handlers. Vibe system prompt updated.scripts/feature_audit.py— features 10 + 12 now assert404(removal proof) instead of "endpoint responsive".Docs / UI
AGENTS.md§7 — new "Skill creation flow — review-and-approve only" subsection above the existing PR-1A load-time gate subsection.codec_cortex.html— CODEC Vibe panel rewritten (no Skill Forge in subtitle / features / description).codec_chat.html— Vibe blurb updated.README.md— 5 Skill Forge mentions updated (table row, section heading, comparison rows, project-structure line).FEATURES.md— Vibe feature list renumbered (Skill Forge items removed; human-review workflow stays as item 17).setup_codec.py:457— Vibe install description.docs/API.md—/api/forgesection removed;/api/skill/review+/api/skill/approveare now the documented contract.Tests
tests/test_skill_routes.py— 9 tests: 4 static-source checks (handlers removed, route strings gone, replacements present), 5 liveTestClientchecks (404 on removed endpoints, 200 on review with valid body, 400 on empty body, full review→approve flow writes after approval only).tests/test_dashboard.py— replacedtest_forge_requires_inputwithtest_forge_endpoint_removed; addedtest_save_skill_endpoint_removed.tests/test_full_product_audit.py— replacedtest_forge_endpointwithtest_forge_endpoint_removed.tests/test_critical_fixes.py— deletedTestSkillForgeBlocklistclass (was already failing on main perdocs/PHASE1-STEP1-PREMERGE-AUDIT.mdrow feat(hooks): plugin lifecycle hook system (Phase 1 Step 2) #4).tests/test_security.py— deletedtest_save_skill_validates_content(was already failing on main per row fix(phase3-step9): consolidated review fast-follow (I2 + I4 + M1 + M2 + M4) #20).Audit closure
docs/audits/PHASE-1-SECURITY.md— D-2 + D-3 closure footnotes.docs/audits/PHASE-1-CONSOLIDATED-TRIAGE.md— D-2 + D-3 rows flippedW1→W1 — CLOSED (PR-1B).Verification
pytest tests/test_skill_routes.pypytest tests/test_skill_registry.pypytest tests/test_skill_contracts.py test_oauth_provider.py test_retry.pypython3 tests/test_skill_imports.pypython3 tools/generate_skill_manifest.py --checkruff check routes/skills.py tests/test_skill_routes.pycodec_dashboard.save_skillare gone)What this PR does NOT include
/api/skill/reviewis NOT extended to support URL-fetch (intentionally dropped per Q1).is_dangerous_skill_codeis NOT modified (its AST contract was already correct).file_writeskill block-roots are NOT changed (that's PR-1C for D-4).permission_gateis NOT changed (that's PR-1D for D-5).Note on the prior PR
This PR's branch is based off PR-1A's branch (
fix/pr1a-skill-load-ast-check), so the PR-1A commits appear in this PR's history until PR-1A merges. Reviewers should either (a) wait for PR-1A to merge and then this PR rebases cleanly onto main, or (b) review only the PR-1B-specific commits (the diff against PR-1A's HEAD).Test plan
pytest tests/test_skill_routes.py -v— 9 passedpytest tests/test_skill_registry.py tests/test_skill_contracts.py tests/test_oauth_provider.py tests/test_retry.py— all passpython3 tests/test_skill_imports.py— 76 parsed, 0 errorspython3 tools/generate_skill_manifest.py --check— cleanruff check routes/skills.py tests/test_skill_routes.py— cleancodec_vibe.html(verified by grep — nofm-*,forge_skill,doSkill,doForge,submitForge,closeForgeModal,setForgeMode,fmOverlayremaining)curl -X POST http://127.0.0.1:8090/api/save_skill -d '{"filename":"x.py","content":"..."}' → 404curl -X POST http://127.0.0.1:8090/api/forge -d '{"code":"http://example.com"}' → 404🤖 Generated with Claude Code