13 KiB
Adversarial Verifier Verdict — Self-Evolution Arc (steals #2 + #3 v1)
Date: 2026-07-10 · Auditor: Fable adversarial verifier (read-only) · Branch: feat/steals-2-3
Under audit: d07fb03b (Wave 2), ce9f67ea (Wave 1), plan docs/plans/SELF-EVOLUTION-ARC-2026-07-10.md
VERDICT: REJECTED → APPROVED after re-audit of a4bcfe28 (see "Re-audit" at end)
(Original first-pass verdict below, retained for the record.)
One HIGH finding blocks approval. The core trust boundary (no autonomous disk/skill/memory write when disabled or unapproved) holds and is well-tested — the rejection is about the quality of the human approval the whole design leans on, plus follow-ups. The fix is small and frontend-only; this is a cheap bounce, not a redesign.
What is CORRECT and safe (verified, not assumed)
- Default OFF.
self-evolution.jsonabsent/corrupt/array/partial ⇒ merged ontoDEFAULT_CONFIGwithenabled:false;tick()returns 0 before enumerating (idle-watcher.ts:120-131,214). Tested: absent, corrupt, disabled, partial-merge all assert disabled/no-op. - VITEST guard + onClose stop wired (
index.ts:2239-2244). Single-flighttickingguard tested. - Watcher → reviewer wiring is fail-safe.
runReviewTurnalways sendsproposeHeld:trueand noautonomy⇒ chat defaultsautonomyLevel:'normal'(chat.ts:564,chat-client.ts:76-82). - create_skill is gated at normal (
confirmation.ts:16-26ALWAYS_CONFIRM) ⇒ it reaches theproposeHeldTurnintercept (chat.ts:1171-1189) and is held, never executed inline. The intercept runs before the grant-store and auto-approve checks, so a saved "Always allow" cannot leak a headless write. Any other gated tool in a review turn iscancel:truedenied. - Reviewer's effective tool pool = reads + create_skill only.
applyPersonaToolFilterallowlist =tools[] ∪ ALWAYS_AVAILABLE_TOOLS, thendisallowedToolsstrips the re-adds.save_memory,delete_skill,install_capability,acquire_capability,add_task,correct_knowledge,spawn_agent,bash, file/git writes,execute_step,compose_workfloware all explicitly disallowed and stripped — locked bysession-reviewer-persona.test.ts:39-56. Connectors carry dynamic names in neithertools[]norALWAYS_AVAILABLE, so the allowlist already excludes them. The non-create_skill survivors are all reads (jailed to workspacefiles/), harmless to run inline. - User manually selecting
session-reviewerin normal chat is safe: noproposeHeld, so create_skill takes the normal interactive approval path — the user approves their own write. Not surfaced inpersona-tier.ts(no ref) so it is not offered in the PersonaSwitcher/onboarding. - Held execution is defended twice:
enqueueHeldActionscans args + refuses critical/non-proposable;executeHeldActionre-scans + re-checks critical at execute time + atomic idempotent claim gate; runs the REAL create_skill tool through the sanctioned, backup-protectedwriteSkill(skill-write-service.ts:168-200,invalidNameblocks path traversal,stashBackupbefore overwrite). - Anti-nag gate namespaces dedupeKeys (
prompt_optimization:<ws>,monthly_assessment,self-evolution:<session>) — no cross-source collision; corrupt/array store recovers to "notify"; LRU-capped at 200. The "Action awaiting your approval" notification is emitted without a dedupeKey, so a real proposal notification is never suppressed. Tests are thorough and real. - Memory-write lint correctly routes symptoms to
recordCapabilityGap+ cancels; conservative empty/non-string ⇒ allow; sentence-scoped to avoid cross-sentence combination.
FINDINGS
[HIGH] F1 — ApprovalsApp shows the human NOTHING about a held create_skill's content
apps/web/src/components/os/apps/ApprovalsApp.tsx:62-74 (summarizeInput) + :244-251.
summarizeInput only surfaces to/recipient, path/file_path/target_workspace_id, command,
query. A create_skill held action's args are {name, content} — none of those fields match,
so inputSummary is the empty string. The card then renders:
toolName:create_skillAutomation:line:describeToolUse('create_skill') = "Creating skill: <name>..."(chat-helpers.ts:158-159), CSS-truncated- the skill body (
input.content) is never shown, and there is no expand/inspect affordance.
/api/approval/pending DOES return the full input (approval.ts:82-92, safeParseArgs), so the
data is present — the FE simply doesn't render it.
Why HIGH. The entire safety story of steal #3 v1 is "review-before-apply — every apply goes
through ApprovalsApp." A skill is a persistence primitive: once approved it is written to disk and
injected into every future agent turn's prompt. The reviewer is fed an attacker-influenceable
session transcript (see F-note below), and its one write is exactly this. A human clicking
"Approve & run" sees only a skill name and a truncated verb phrase — they cannot see the instructions
they are persisting. That converts the human control from "review" into "approve-blind" for the one
vehicle this arc adds. (write_file held actions at least show the target path via
summarizeInput; create_skill shows neither location nor content.)
Mitigations that keep this out of CRITICAL: default OFF; enqueueHeldAction injection-scans the
args (overt payloads refused); redactSkillContent on write; human approval still required (no
autonomous write). But a benign-looking-but-adversarial skill body evades the injection scanner.
Required fix (FE-only, small): in ApprovalsApp render the held create_skill input.name and a
content preview with an expander (or a "view full skill" panel) so the approver sees the exact bytes
that writeSkill will persist. Optionally extend summarizeInput to surface input.name for
create_skill. Add a test asserting a held create_skill card exposes its content.
[MEDIUM] F2 — memory-write lint false-positives lose legitimate preference/dependence memories
packages/agent/src/memory-write-lint.ts:34-61,93-106. The classifier fires on (capability noun AND
failure verb in the same sentence), and FAILURE_VERB includes bare cannot|can't. Realistic
LEGIT memories are misclassified as capability_symptom and cancelled:
"User's main tool is Figma; they cannot stand Sketch."→tool+cannot⇒ blocked (it's a preference)."User can't work without their Jira integration."→integration+can't⇒ blocked (it's a dependence, positive)."Our API is down for maintenance this weekend."→API+is down⇒ blocked (a legit business fact).
The module's own docstring promises "User preferences / opinions about tools MUST pass" — these
violate that stated constraint, and CLAUDE.md §coding-style values not losing data. Because
save_memory is the sink (cognify bypasses the hook per the declared v1 gap), the loss is bounded to
explicit agent save_memory calls, and the agent receives a cancel reason it could act on — hence
MEDIUM, not HIGH. Fix/follow-up: require the failure to predicate on the capability (not the
user) — e.g. exclude when the subject of the failure verb is a person/pronoun, or drop bare
cannot/can't from the generic-noun path and keep it only for the connection-failure path.
[MEDIUM] F3 — the proposeHeldTurn intercept branch itself is not directly tested
The plan promised an "end-to-end route test with mocked loopback (reviewer proposes → held action
exists → approve → skill written with backup)." The two halves exist —
session-reviewer-persona.test.ts (tool filter) and held-action-executor.test.ts:153-165
(create_skill held → executes via sanctioned path) — but the security-critical chat.ts:1171-1189
branch (convert a live gated create_skill into enqueueHeldAction, and cancel:true-deny every
other gated tool during a review turn) has no direct test. The branch is simple and composes
tested pieces, so this is a coverage gap, not a known break. Follow-up: add a route test with
proposeHeld:true asserting (a) a create_skill call becomes a held row and (b) a second gated tool
(e.g. write_file) is denied with no side effect.
[LOW] F4 — meta lazy-backfill elsewhere can trigger a spurious re-review
idle-watcher.ts:169-177 keys the fired-set on sessionId:mtimeMs. The watcher itself is pure, but
if any other subsystem calls readSessionMeta (which lazily backfills a title/summary and writes
the file — the documented side effect the watcher avoids) on an already-reviewed idle session, its
mtime advances, the fired key changes, and the session re-fires one review with no new user
content. Bounded by maxReviewsPerDay. Cost/nuisance only. Follow-up: key on last-message content
hash or line count instead of mtime, or persist the fired-set.
[LOW] F5 — daily cap + fired-set are RAM-only (declared)
dayCount/firedKeys reset on restart (idle-watcher.ts:87-89), so a crash-loop could exceed the
intended 5/day and a restart could re-fire recently-reviewed sessions. Declared as an accepted v1
deviation; acceptable given default-OFF and small cap. Note only.
[LOW] F6 — NotificationGate load→save is non-atomic
notification-gate.ts:86-91 reads the whole store then writes it; concurrent emits can race and lose
a fingerprint update. Worst case is one duplicate/missed suppression — never a security effect. Note only.
Declared deviations — assessment
- Transcript embedded in the review message (vs "reviewer has read tools + workspace binding").
Sound:
read_fileis jailed to workspacefiles/(resolveSafe) so the reviewer cannot open the session JSONL; embedding is the reliable path. The embedded transcript passes throughscanForInjection(message,'user_input')atchat.ts:617— a ≥0.7 payload blocks the whole review (fail-safe), a weaker one can steer the reviewer but its only write is the held, human-approved create_skill. Acceptable, and it is what makes F1 the load-bearing control. - RAM daily cap / fired-set — acceptable (F5).
- Persona 22→23 — session-reviewer added, excluded from onboarding + PersonaSwitcher; create_skill still gates even if selected. Acceptable.
Path to APPROVED
Fix F1 (render held create_skill content in ApprovalsApp + a test). F2/F3 are strongly recommended before shipping self-evolution to users but can be listed follow-ups. F4-F6 are notes.
Re-audit — fix commit a4bcfe28 (2026-07-10)
FINAL VERDICT: APPROVED (zero CRITICAL, zero HIGH)
Re-read the full diff of a4bcfe28 and ran every affected suite myself from the worktree.
F1 (HIGH) — CLOSED
ApprovalsApp.tsx: new SkillPreview renders the skill name up front + an
expander showing the exact bytes writeSkill will persist, for any held action
carrying {name, content} (:251-253 gate, :289-291 render). Content is rendered
as text inside <pre>{content}</pre> — React-escaped, no dangerouslySetInnerHTML,
so no XSS from an attacker-shaped skill body. Not shown for send_email (no
{name,content}). The approver can no longer approve blind. Locked by
ApprovalsApp.test.tsx (3/3): name visible, content collapsed-by-default then
expandable to exact bytes then collapsible, absent for send_email, generic for any
{name,content}.
F2 (MEDIUM) — CLOSED
memory-write-lint.ts: bare cannot/can't/could not/unable to now fires ONLY when
immediately followed by a capability verb (connect|authenticate|access|load|run|…),
and is down carries a (?!\s+for\s+maintenance) lookahead. Verified against the
three cited false-positives — all now allow: "cannot stand Sketch",
"can't work without their Jira integration", "our API is down for maintenance".
Real symptoms still caught ("failed to authenticate", "keeps timing out", "unavailable",
"cannot connect"). Residual is a few exotic-verb false-negatives — the safe
direction (memory kept). 35/35 lint tests pass.
F3 (MEDIUM) — CLOSED
Intercept extracted to held-action-executor.ts::decideReviewTurnTool(); chat.ts:1172
rewired to call it; enqueueHeldAction/isProposableTool imports removed with no
dangling references (grep clean). Extraction is behavior-preserving (proposable→held+step,
non-proposable→deny step, always cancel:true). Now unit-tested at the exact break point
(held-action-executor.test.ts +3): create_skill → held row created & NOT written inline;
bash (non-proposable) → denied, no row; injection-tripping create_skill → refused, no row.
Regression check — the previously-sound boundary is intact
The persona tool-filter, proposeHeld-before-grant-store ordering, gated-create_skill→held
path, and default-OFF watcher are all unchanged by this commit. No new autonomous-write path.
Verification I ran (worktree root, not the main checkout)
- vitest: memory-write-lint 35/35, idle-watcher 17/17, notification-gate 14/14, session-reviewer-persona 2/2, held-action-executor 17/17, ApprovalsApp 3/3.
- tsc
--noEmit: packages/agent 0, packages/server 0, apps/web 0.
Remaining residuals (declared LOW — do not block)
F4 (mtime-keyed fired-set → possible spurious re-review), F5 (RAM daily-cap/fired-set reset on restart), F6 (non-atomic NotificationGate save). Acceptable for v1; track as follow-ups.