Skip to content

docs(skills/darwinian-evolver): add pickle security note to SKILL.md - #28564

Open
Bihruze wants to merge 1 commit into
NousResearch:mainfrom
Bihruze:patch-1
Open

docs(skills/darwinian-evolver): add pickle security note to SKILL.md#28564
Bihruze wants to merge 1 commit into
NousResearch:mainfrom
Bihruze:patch-1

Conversation

@Bihruze

@Bihruze Bihruze commented May 19, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Small follow-up to #26760. Adds an explicit security note to SKILL.md about the pickle deserialization risk for snapshots produced by darwinian_evolver.

The Pitfalls section in the merged version mentions nested-pickle snapshots as a structural quirk, but doesn't flag the underlying arbitrary-code-execution risk. This PR makes that explicit so users know not to load snapshots from untrusted sources.

Raised by @123mikeyd in Discord — credit to them for the catch.

Related Issue

Follow-up to #26760 (no separate issue).

Type of Change

  • 📝 Documentation update
  • 🔒 Security fix

Changes Made

  • optional-skills/research/darwinian-evolver/SKILL.md: added a new ## Security section warning users about pickle deserialization risk and recommending they only load snapshots from trusted sources.

How to Test

Docs-only change, no runtime behavior affected.

  1. Open optional-skills/research/darwinian-evolver/SKILL.md
  2. Verify the new ## Security section renders correctly and the warning is clear.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (docs(scope):)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix
  • I've run pytest tests/ -q and all tests pass — N/A (docs-only)
  • I've added tests for my changes — N/A (docs-only)
  • I've tested on my platform: docs-only, no platform-specific code

Documentation & Housekeeping

  • I've updated relevant documentation (this PR is the documentation update)
  • I've updated cli-config.yaml.example if I added/changed config keys — N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A
  • I've considered cross-platform impact — N/A (docs-only)
  • I've updated tool descriptions/schemas if I changed tool behavior — N/A

Screenshots / Logs

N/A — docs-only change.

Adds explicit security warning about pickle deserialization risk for snapshots. Raised by @123mikeyd in Discord.
@alt-glitch alt-glitch added type/docs Documentation improvements type/security Security vulnerability or hardening P3 Low — cosmetic, nice to have tool/skills Skills system (list, view, manage) labels May 19, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the focused security docs follow-up. The premise still holds on current main: SKILL.md mentions pickle snapshots at optional-skills/research/darwinian-evolver/SKILL.md:84 and nested pickles at optional-skills/research/darwinian-evolver/SKILL.md:169-171, but it does not explicitly warn about pickle RCE.

Problems

  • Current main has since added a runtime acknowledgement gate in optional-skills/research/darwinian-evolver/scripts/show_snapshot.py:29-35 and refuses to unpickle without it at optional-skills/research/darwinian-evolver/scripts/show_snapshot.py:42-48. The SKILL.md quick-start command at optional-skills/research/darwinian-evolver/SKILL.md:109-111 still omits --i-trust-this-file, so applying this docs warning alone would leave the example stale.

Suggested changes

  • Keep the new ## Security section, and also update the show_snapshot.py invocation in SKILL.md to include --i-trust-this-file so the warning and the runnable example line up with current main.

Automated hermes-sweeper review.

```
## Security

⚠️ **Pickle deserialization risk.** Snapshots produced by `darwinian_evolver` are Python pickle files. Loading a pickle from an untrusted source can execute arbitrary code on your machine.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This warning is accurate. If this is salvaged onto current main, please also update the earlier show_snapshot.py example to pass --i-trust-this-file, because the script now refuses to unpickle without that acknowledgement.

@teknium1 teknium1 added the sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users label Jul 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P3 Low — cosmetic, nice to have sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users tool/skills Skills system (list, view, manage) type/docs Documentation improvements type/security Security vulnerability or hardening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants