Skip to content

Commit 4051fc8

Browse files
author
Brian Krafft
committed
feat(7.2): SkillGuard quality gates + blocklist hardening - SkillGuard wraps ReviewGate+Store for pre-persist review - Wire guarded_evolve/save into all 3 evolution strategies - Add 7 blocklist categories: os.exec*, os.spawn*, breakpoint, shutil, os.remove, os.chmod - 20 new tests (2040 total passed)
1 parent 9428ad3 commit 4051fc8

7 files changed

Lines changed: 590 additions & 7 deletions

File tree

RUNBOOK.yaml

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -608,15 +608,20 @@ epics:
608608
- id: '7.1'
609609
phase: P7
610610
title: Eight-eyes integration
611-
status: pending
612-
branch: null
613-
pr: null
614-
review_round: 0
611+
status: done
612+
branch: epic/7.1-review-gate
613+
pr: 54
614+
review_round: 3
615615
description: "Built-in /8eyes review for skill evolution \xE2\u20AC\u201D every\
616616
\ evolved skill gets multi-agent review before activation.\n"
617617
acceptance:
618618
- Skill evolution triggers /8eyes review automatically
619619
- Evolved skills quarantined until review passes
620+
notes: |
621+
ReviewGate with 5-layer security: path traversal, extension allowlist,
622+
size limits, HIGH+CRITICAL AST blocking, lineage.validate().
623+
63 tests (45 adversarial), 2020 total passed.
624+
3 review rounds, 12 agent-reviews (impl, sec, GPT-5.4, Opus 4.6).
620625
- id: '7.2'
621626
phase: P7
622627
title: SkillGuard quality gates

openspace/security/blocklist.yml

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,3 +175,73 @@ patterns:
175175
- globals
176176
- locals
177177
- vars
178+
179+
# === Added in Epic 7.2 — SkillGuard hardening ===
180+
181+
- name: os_exec_family
182+
description: "os.exec*() replaces the current process — full RCE"
183+
severity: CRITICAL
184+
ast_type: Call
185+
targets:
186+
- os.execv
187+
- os.execve
188+
- os.execvp
189+
- os.execvpe
190+
- os.execl
191+
- os.execlp
192+
- os.execlpe
193+
194+
- name: os_spawn_family
195+
description: "os.spawn*() creates new processes — command execution"
196+
severity: CRITICAL
197+
ast_type: Call
198+
targets:
199+
- os.spawnl
200+
- os.spawnle
201+
- os.spawnlp
202+
- os.spawnlpe
203+
- os.spawnv
204+
- os.spawnve
205+
- os.spawnvp
206+
- os.spawnvpe
207+
208+
- name: breakpoint_debugger
209+
description: "breakpoint() drops to interactive debugger — arbitrary code"
210+
severity: HIGH
211+
ast_type: Call
212+
targets:
213+
- breakpoint
214+
215+
- name: shutil_destructive
216+
description: "shutil.rmtree/move/copy can destroy or exfiltrate filesystem data"
217+
severity: HIGH
218+
ast_type: Call
219+
targets:
220+
- shutil.rmtree
221+
- shutil.move
222+
- shutil.copytree
223+
224+
- name: shutil_import
225+
description: "Importing shutil enables destructive filesystem operations"
226+
severity: HIGH
227+
ast_type: Import
228+
targets:
229+
- shutil.*
230+
231+
- name: os_remove_family
232+
description: "os.remove/unlink/rmdir can delete files and directories"
233+
severity: HIGH
234+
ast_type: Call
235+
targets:
236+
- os.remove
237+
- os.unlink
238+
- os.rmdir
239+
- os.removedirs
240+
241+
- name: os_chmod
242+
description: "os.chmod() can weaken file permissions"
243+
severity: HIGH
244+
ast_type: Call
245+
targets:
246+
- os.chmod
247+
- os.chown

openspace/skill_engine/evolution/strategies.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ async def evolve_fix(evolver, ctx: EvolutionContext) -> Optional[SkillRecord]:
118118
critical_tools=list(parent.critical_tools),
119119
)
120120

121-
await evolver._store.evolve_skill(new_record, [parent.skill_id])
121+
await evolver._guard.guarded_evolve(new_record, [parent.skill_id])
122122

123123
# Stamp the new skill_id into the sidecar file so next discover()
124124
write_skill_id(parent_dir, new_id)
@@ -252,7 +252,7 @@ async def evolve_derived(evolver, ctx: EvolutionContext) -> Optional[SkillRecord
252252
critical_tools=sorted(all_critical),
253253
)
254254

255-
await evolver._store.evolve_skill(new_record, parent_ids)
255+
await evolver._guard.guarded_evolve(new_record, parent_ids)
256256

257257
# Stamp skill_id sidecar so discover() uses this ID on restart
258258
write_skill_id(target_dir, new_id)
@@ -360,7 +360,7 @@ async def evolve_captured(evolver, ctx: EvolutionContext) -> Optional[SkillRecor
360360
),
361361
)
362362

363-
await evolver._store.save_record(new_record)
363+
await evolver._guard.guarded_save(new_record)
364364

365365
# Stamp skill_id sidecar so discover() uses this ID on restart
366366
write_skill_id(target_dir, new_id)

openspace/skill_engine/evolver.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@
6363
process_tool_degradation as _process_tool_degradation_impl,
6464
)
6565
from .patch import SkillEditResult
66+
from .skill_guard import SkillGuard
6667
from .store import SkillStore
6768
from .types import (
6869
EvolutionSuggestion,
@@ -123,6 +124,7 @@ def __init__(
123124
max_concurrent: int = 3,
124125
) -> None:
125126
self._store = store
127+
self._guard = SkillGuard(store=store)
126128
self._registry = registry
127129
self._llm_client = llm_client
128130
self._model = model
Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
"""SkillGuard — quality gates for skill evolution.
2+
3+
Wraps ReviewGate + SkillStore into a single guarded API that ensures
4+
every skill mutation is reviewed BEFORE persistence. Unsafe skills are
5+
never written to the store.
6+
7+
Three guarded entry points:
8+
- guarded_evolve() — for FIXED/DERIVED evolutions (store.evolve_skill)
9+
- guarded_save() — for CAPTURED skills (store.save_record)
10+
- guarded_reactivate() — re-review before reactivating quarantined skills
11+
12+
Usage::
13+
14+
guard = SkillGuard(store=store)
15+
result = await guard.guarded_evolve(new_record, parent_ids)
16+
if not result.passed:
17+
logger.error("Skill blocked: %s", result)
18+
"""
19+
20+
from __future__ import annotations
21+
22+
import logging
23+
from typing import List, Optional
24+
25+
from openspace.skill_engine.review_gate import ReviewGate, ReviewResult, CheckResult
26+
27+
logger = logging.getLogger(__name__)
28+
29+
30+
class SkillGuard:
31+
"""Pre-persist quality gate for all skill mutations.
32+
33+
Every mutation path (evolve, save, reactivate) runs through
34+
ReviewGate BEFORE the store write. If review fails, the write
35+
never happens — the skill is never activated.
36+
"""
37+
38+
def __init__(self, store, gate: Optional[ReviewGate] = None) -> None:
39+
self._store = store
40+
self._gate = gate or ReviewGate()
41+
42+
async def guarded_evolve(
43+
self,
44+
new_record,
45+
parent_skill_ids: List[str],
46+
) -> ReviewResult:
47+
"""Review, then persist an evolved skill (FIXED or DERIVED).
48+
49+
Returns the ReviewResult. If passed, the skill is persisted and
50+
activated. If failed, the skill is NOT persisted at all.
51+
"""
52+
result = self._gate.review(new_record)
53+
54+
if not result.passed:
55+
failed = [c for c in result.checks if c.verdict == "fail"]
56+
logger.warning(
57+
"SkillGuard BLOCKED evolve of %s — failed: %s",
58+
new_record.skill_id,
59+
", ".join(c.name for c in failed),
60+
)
61+
for c in failed:
62+
logger.warning(" [%s] %s", c.name, c.detail)
63+
return result
64+
65+
await self._store.evolve_skill(new_record, parent_skill_ids)
66+
logger.info(
67+
"SkillGuard APPROVED evolve of %s (gen=%d)",
68+
new_record.skill_id,
69+
new_record.lineage.generation,
70+
)
71+
return result
72+
73+
async def guarded_save(self, record) -> ReviewResult:
74+
"""Review, then save a new captured skill.
75+
76+
Returns the ReviewResult. If passed, the skill is saved.
77+
If failed, the skill is NOT saved.
78+
"""
79+
result = self._gate.review(record)
80+
81+
if not result.passed:
82+
failed = [c for c in result.checks if c.verdict == "fail"]
83+
logger.warning(
84+
"SkillGuard BLOCKED save of %s — failed: %s",
85+
record.skill_id,
86+
", ".join(c.name for c in failed),
87+
)
88+
return result
89+
90+
await self._store.save_record(record)
91+
logger.info("SkillGuard APPROVED save of %s", record.skill_id)
92+
return result
93+
94+
async def guarded_reactivate(self, skill_id: str) -> ReviewResult:
95+
"""Re-review a quarantined skill before reactivation.
96+
97+
Fetches the record, runs review, and only reactivates if it passes.
98+
"""
99+
record = await self._store.get_record(skill_id)
100+
if record is None:
101+
return ReviewResult.from_checks([
102+
CheckResult(
103+
name="lookup",
104+
verdict="fail",
105+
detail=f"Skill {skill_id} not found in store",
106+
),
107+
])
108+
109+
result = self._gate.review(record)
110+
111+
if not result.passed:
112+
logger.warning(
113+
"SkillGuard BLOCKED reactivation of %s — still fails review",
114+
skill_id,
115+
)
116+
return result
117+
118+
await self._store.reactivate_record(skill_id)
119+
logger.info("SkillGuard APPROVED reactivation of %s", skill_id)
120+
return result

tests/test_evolution_strategies.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
evolve_derived,
2323
evolve_fix,
2424
)
25+
from openspace.skill_engine.review_gate import CheckResult, ReviewResult
26+
from openspace.skill_engine.skill_guard import SkillGuard
2527

2628

2729
# ---------------------------------------------------------------------------
@@ -94,6 +96,12 @@ def __init__(self):
9496
self._store = MagicMock()
9597
self._store.evolve_skill = AsyncMock()
9698
self._store.save_record = AsyncMock()
99+
# SkillGuard wraps the mock store with an always-pass gate
100+
_pass_gate = MagicMock()
101+
_pass_gate.review = MagicMock(return_value=ReviewResult.from_checks([
102+
CheckResult(name="test-gate", verdict="pass", detail="ok"),
103+
]))
104+
self._guard = SkillGuard(store=self._store, gate=_pass_gate)
97105
self._registry = MagicMock()
98106
self._registry._skill_dirs = [Path("/skills")]
99107
self._registry.update_skill = MagicMock()

0 commit comments

Comments
 (0)