fix(skills): harden auto-learn pipeline and enable skills guard scan - #89
Merged
Conversation
Makes the skill auto-learn system safe to use by default: 1. Local-scan floor: ScanSuggestionBody, scanSkill, and applyGuardToSkills now always run the local rule scan via guard.ScanContentWithScope - previously 'skills scan disabled' (the default everywhere) meant zero scanning, so a learned skill body containing injection patterns saved and loaded unchecked. 2. NeedsReview is now enforced at trigger time: lazy matchers are built from the non-NeedsReview skill set, so flagged/tainted skills can no longer be trigger-injected on a single keyword match. Previously NeedsReview only pinned skills out of auto-load, leaving per-turn injection wide open. 3. Project-dir skills (./.odek/skills) are distrusted like ./odek.json: forced NeedsReview with a 'project' source, so a cloned repo can no longer auto-inject a SKILL.md. 'odek skill promote' now falls back to the project dir so these skills can be reviewed and promoted. 4. Taint coverage: web_search, vision, and delegate_tasks now taint the session (like browser/http_batch), so skills learned from such sessions are declined by auto-save. shell is deliberately excluded (primary work tool - tainting it would taint nearly every session). Auto-load skill context and skill_load tool output are now wrapped with the nonce'd untrusted-content boundary. 5. The 'skills' guard scan scope now defaults to enabled (config + docker configs + docs), adding the PiGuard semantic second opinion on top of the local floor at save and load time. Tests: 12+ new/updated tests across skills, memory, guard, config, and CLI packages; full suite, -race on skills, and repo-wide golangci-lint all clean.
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
odek | 918d76b | Commit Preview URL Branch Preview URL |
Jul 22 2026, 10:44 AM |
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
1 participant
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.
Follow-up to the auto-learn security review. Makes the skill self-improvement system safe to use by default, with PiGuard as the semantic second opinion on top of structural fixes. All findings were verified against the code before fixing.
The problem (verified)
ScanSuggestionBody,scanSkill,applyGuardToSkills) early-returned when the scope was off — and it defaulted off everywhere — so not even the free local rule scan ran. A learned skill body containing injection patterns saved and loaded unchecked.NeedsReviewdidn't gate trigger matching. It only pinned skills out of auto-load; lazy matchers had no provenance filter and fire on a single keyword hit. So a flagged/tainted skill was still injected into context on near-every turn (learned triggers derive from LLM/body keywords likerun/test/build). The PiGuard flag was nearly toothless../.odek/skills/was scanned first with priority,auto_load: truehonored, no provenance — a cloned malicious repo could auto-inject a SKILL.md into the system prompt. Inconsistent with how./odek.jsonis treated everywhere else.web_search/vision/delegate_tasksdidn't taint the session, so skills learned from injected search results were auto-saved as "clean". Auto-load skill context andskill_loadoutput were not wrapped as untrusted.The fixes
guard.ScanContentWithScope: local rule scan always, PiGuard when the scope is enabled.NeedsReviewenforced at trigger time — lazy matchers are built from the non-NeedsReview set; flagged skills stay listable/promotable but cannot be trigger-injected until promoted.NeedsReview+projectsource (same model as./odek.json).odek skill promotenow falls back to the project dir so these skills are promotable after review (requires--forcesince they carry a source — deliberate).web_search/vision/delegate_tasksnow taint (shelldeliberately excluded — it's the primary work tool; comment documents why). Auto-load skill context andskill_loadoutput are wrapped with the nonce'd untrusted boundary.scan.skillsdefaults to enabled —DefaultScanConfig, both docker configs, anddocs/CONFIG.md. PiGuard now scans skill bodies at learn-loop save time (one call per saved skill, off the hot path) and at reload (one batch per skill-set change).Verification
go test ./...— all green;-raceoninternal/skills— green; repo-widegolangci-lint— 0 issues.AGENTS.mdanddocs/SECURITY.mdsynced to the new behavior.Operator note: existing project-dir skills will appear as
NeedsReviewafter this change and won't trigger untilodek skill promote <name> --force. Skills in~/.odek/skillsare unaffected.