Add Picoclaw guardian + posture-review skills at v0.0.1 with wiki docs#208
Conversation
| export function stableStringify(value, space = 2) { | ||
| return JSON.stringify(sortDeep(value), null, space); | ||
| } | ||
|
|
||
| function sortDeep(value) { | ||
| if (Array.isArray(value)) return value.map(sortDeep); | ||
| if (!value || typeof value !== "object") return value; | ||
| const out = {}; | ||
| for (const key of Object.keys(value).sort()) out[key] = sortDeep(value[key]); | ||
| return out; |
There was a problem hiding this comment.
stableStringify/sortDeep is duplicated across Picoclaw and skills/hermes-attestation-guardian, so should we reuse the existing Hermes helper via a shared utils export instead of copying the canonicalizer into each skill?
Finding type: Code Dedup and Conventions | Severity: 🟢 Low
Want Baz to fix this for you? Activate Fixer
| - `"openclaw"` - OpenClaw/Clawdbot/MoltBot only | ||
| - `"nanoclaw"` - NanoClaw only | ||
| - `["openclaw", "nanoclaw"]` - Both platforms | ||
| - `"hermes"` - Hermes only | ||
| - `"picoclaw"` - Picoclaw only | ||
| - `["openclaw", "nanoclaw", "hermes", "picoclaw"]` - All core platforms |
There was a problem hiding this comment.
README advertises picoclaw as a core platform but CorePlatformSlug, isCorePlatformSlug(), and AdvisoryPlatformFilter don't include it, so picoclaw falls through to "other" in FeedSetup.tsx — should we update CORE_PLATFORM_SLUGS and related types/badge mapping to include picoclaw, or remove it from the docs until the contract is ready?
Finding types: Breaking Changes | Severity: 🟠 Medium
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents:
Resolve the inconsistency between the README and the runtime taxonomy for `picoclaw`: -
Either update the shared core-platform contract — `CORE_PLATFORM_SLUGS`,
`CorePlatformSlug`, `isCorePlatformSlug()`, `AdvisoryPlatformFilter`, and any related
badge/tab mapping in `pages/FeedSetup.tsx` — to include `picoclaw` so it renders as a
first-class tab/filter option. - Or, if that runtime change is out of scope, remove
`picoclaw` from the README's core platform list (lines 231-232) and document it under an
"Other"/non-core category until the UI supports it. Verify against the current code
before applying, and ensure the UI badge descriptor for `picoclaw` matches whichever
path is chosen.
c3bd4c0 to
df020b4
Compare
| @@ -0,0 +1,371 @@ | |||
| #!/usr/bin/env bash | |||
There was a problem hiding this comment.
This regression test under test/ is named picoclaw_security_guardian_sandbox_regression.sh instead of using the required *.test.mjs suffix per AGENTS.md, should we rename it to match?
Finding type: AI Coding Guidelines | Severity: 🟢 Low
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents:
Before applying, verify this suggestion against the current code. In
skills/picoclaw-security-guardian/test/picoclaw_security_guardian_sandbox_regression.sh
around lines 1-1, the sandbox regression script is added as a .sh file named
picoclaw_security_guardian_sandbox_regression.sh, but AGENTS.md requires skill-local
tests to use the *.test.mjs suffix. Refactor by either renaming the test to end with
.test.mjs (and port the runner to Node/ESM, preserving the current bash logic) or, if
you must keep bash, add a new wrapper file named e.g.
picoclaw_security_guardian_sandbox_regression.test.mjs that invokes this script and then
remove/adjust the old .sh so the test runner only sees the required suffix. Also update
any CI/test discovery configuration or documentation that references the old filename so
it runs under the new *.test.mjs convention.
| export function runPicoclawSelfPenTest(profile, options = {}) { | ||
| const findings=[]; const rt=profile?.posture?.runtime || {}; | ||
| if (rt.ui?.public_web_ui) add(findings,"critical","PUBLIC_WEB_UI_EXPOSED","Web UI appears bound publicly","public_web_ui=true or equivalent detected","Bind to localhost or enforce password auth + CIDR allowlist before exposure."); | ||
| if (rt.ui?.auth_disabled) add(findings,"critical","WEB_UI_AUTH_DISABLED","Web UI auth appears disabled","auth_disabled=true or empty password marker detected","Require password/session auth for any gateway controller UI."); | ||
| if (rt.tools?.unrestricted_workspace) add(findings,"critical","WORKSPACE_UNRESTRICTED","Tool workspace restriction appears disabled","restrict_to_workspace=false or sandbox=false marker detected","Enable workspace confinement and deny symlink/absolute-path escapes."); | ||
| if (rt.risky_toggles?.allow_unsigned_mode) add(findings,"critical","UNSIGNED_MODE_ALLOWED","Unsigned or insecure verification mode appears enabled","allow_unsigned/skip_signature marker detected","Disable unsigned mode except short audited break-glass windows."); | ||
| if (rt.mcp?.enabled) add(findings,"high","MCP_REVIEW_REQUIRED","MCP servers enabled","mcp marker detected","Review each MCP server as a separate trust boundary with least privilege and secrets isolation."); | ||
| if (rt.tools?.enabled) add(findings,"medium","TOOLING_REVIEW_REQUIRED","Agent tools appear enabled","tools/code_execution/shell/filesystem marker detected","Require per-tool allowlists and operator approval for dangerous tools."); | ||
| if (rt.scheduler?.enabled) add(findings,"medium","SCHEDULER_REVIEW_REQUIRED","Scheduler/persistence features appear enabled","cron/schedule marker detected","Inventory jobs and alert on new persistent actions."); | ||
| if ((rt.secrets?.config_secret_markers || 0) > 0) add(findings,"high","PLAINTEXT_SECRET_MARKERS","Config contains secret-like markers",`${rt.secrets.config_secret_markers} marker(s) found`,`Move secrets to supported encrypted/secure storage and redact logs/exports.`); | ||
| const enabledGateways = Object.entries(rt.gateways || {}).filter(([,v])=>!!v).map(([k])=>k); | ||
| if (enabledGateways.length > 1) add(findings,"medium","MULTI_CHANNEL_AUTH_REVIEW","Multiple chat gateways appear enabled",enabledGateways.join(", "),"Pin immutable user IDs per channel and reject group/forwarded-message ambiguity."); |
There was a problem hiding this comment.
runPicoclawSelfPenTest duplicates the same runtime-toggle checks from skills/picoclaw-security-guardian/lib/drift.mjs, so should we extract/reuse the signal-threshold logic (e.g. via mergeConfigSignals) to feed both drift comparison and self-pen tests from one shared source?
Finding type: Code Dedup and Conventions | Severity: 🟢 Low
Want Baz to fix this for you? Activate Fixer
| const SEVERITY_ORDER = ["critical", "high", "medium", "low", "info"]; | ||
|
|
||
| function bump(summary, sev) { summary[sev] = (summary[sev] || 0) + 1; } | ||
| function bool(value) { return !!value; } | ||
| function add(findings, summary, severity, code, path, message, details = undefined) { |
There was a problem hiding this comment.
diffPicoclawProfiles duplicates most of SEVERITY_ORDER/compare and the diff boilerplate from lib/diff.mjs, so changes would need updating in two places—should we extract those helpers into a shared module or parameterize diff.mjs and reuse it in both skills?
Finding type: Code Dedup and Conventions | Severity: 🟢 Low
Want Baz to fix this for you? Activate Fixer
| { value: 'openclaw', label: 'OpenClaw', active: 'bg-clawd-accent/20 text-clawd-accent border-2 border-clawd-accent', inactive: 'bg-clawd-800 text-gray-400 border border-clawd-700 hover:border-clawd-accent/50' }, | ||
| { value: 'nanoclaw', label: 'NanoClaw', active: 'bg-clawd-secondary/20 text-clawd-secondary border-2 border-clawd-secondary', inactive: 'bg-clawd-800 text-gray-400 border border-clawd-700 hover:border-clawd-secondary/50' }, | ||
| { value: 'hermes', label: 'Hermes', active: 'bg-emerald-500/20 text-emerald-300 border-2 border-emerald-400', inactive: 'bg-clawd-800 text-gray-400 border border-clawd-700 hover:border-emerald-400/50' }, | ||
| { value: 'picoclaw', label: 'Picoclaw', active: 'bg-cyan-500/20 text-cyan-300 border-2 border-cyan-400', inactive: 'bg-clawd-800 text-gray-400 border border-clawd-700 hover:border-cyan-400/50' }, | ||
| { value: 'other', label: 'Other', active: 'bg-clawd-600/40 text-gray-100 border-2 border-clawd-500', inactive: 'bg-clawd-800 text-gray-400 border border-clawd-700 hover:border-clawd-500/50' }, |
There was a problem hiding this comment.
PLATFORM_TABS duplicates core platform definitions already in types.ts::CORE_PLATFORM_SLUGS and utils/advisoryPlatforms.ts::PLATFORM_DESCRIPTOR_BY_SLUG, so adding a platform means updating multiple places or it gets omitted—can we derive the tab metadata from the shared descriptor/slug source?
Finding type: Code Dedup and Conventions | Severity: 🟢 Low
Want Baz to fix this for you? Activate Fixer
| export function runPicoclawSelfPenTest(profile, _options = {}) { | ||
| const findings=[]; const rt=profile?.posture?.runtime || {}; | ||
| if (rt.ui?.public_web_ui) add(findings,"critical","PUBLIC_WEB_UI_EXPOSED","Web UI appears bound publicly","public_web_ui=true or equivalent detected","Bind to localhost or enforce password auth + CIDR allowlist before exposure."); | ||
| if (rt.ui?.auth_disabled) add(findings,"critical","WEB_UI_AUTH_DISABLED","Web UI auth appears disabled","auth_disabled=true or empty password marker detected","Require password/session auth for any gateway controller UI."); | ||
| if (rt.tools?.unrestricted_workspace) add(findings,"critical","WORKSPACE_UNRESTRICTED","Tool workspace restriction appears disabled","restrict_to_workspace=false or sandbox=false marker detected","Enable workspace confinement and deny symlink/absolute-path escapes."); | ||
| if (rt.risky_toggles?.allow_unsigned_mode) add(findings,"critical","UNSIGNED_MODE_ALLOWED","Unsigned or insecure verification mode appears enabled","allow_unsigned/skip_signature marker detected","Disable unsigned mode except short audited break-glass windows."); | ||
| if (rt.mcp?.enabled) add(findings,"high","MCP_REVIEW_REQUIRED","MCP servers enabled","mcp marker detected","Review each MCP server as a separate trust boundary with least privilege and secrets isolation."); | ||
| if (rt.tools?.enabled) add(findings,"medium","TOOLING_REVIEW_REQUIRED","Agent tools appear enabled","tools/code_execution/shell/filesystem marker detected","Require per-tool allowlists and operator approval for dangerous tools."); | ||
| if (rt.scheduler?.enabled) add(findings,"medium","SCHEDULER_REVIEW_REQUIRED","Scheduler/persistence features appear enabled","cron/schedule marker detected","Inventory jobs and alert on new persistent actions."); | ||
| if ((rt.secrets?.config_secret_markers || 0) > 0) add(findings,"high","PLAINTEXT_SECRET_MARKERS","Config contains secret-like markers",`${rt.secrets.config_secret_markers} marker(s) found`,`Move secrets to supported encrypted/secure storage and redact logs/exports.`); | ||
| const enabledGateways = Object.entries(rt.gateways || {}).filter(([,v])=>!!v).map(([k])=>k); | ||
| if (enabledGateways.length > 1) add(findings,"medium","MULTI_CHANNEL_AUTH_REVIEW","Multiple chat gateways appear enabled",enabledGateways.join(", "),"Pin immutable user IDs per channel and reject group/forwarded-message ambiguity."); |
There was a problem hiding this comment.
runPicoclawSelfPenTest duplicates the runtime-flag discovery/severity metadata from skills/picoclaw-security-guardian/lib/drift.mjs, so can we consolidate both to read from a shared data-driven runtime-flag helper/source of truth?
Finding type: Code Dedup and Conventions | Severity: 🟢 Low
Want Baz to fix this for you? Activate Fixer
User description
Opener Type
Summary
Changes Made
Related Issues
Type of Change
Testing
Checklist
Generated description
Below is a concise technical summary of the changes proposed in this PR:
Deliver Picoclaw security guardian and
picoclaw-self-pen-testingpackages with deterministic profile/drift/supply-chain libraries, CLI scripts, tests, and a Picoclaw sandbox regression harness so posture, drift, and advisory workflows stay guarded. Update the feed pipeline, UI filters, and README/wiki docs to treatpicoclawas a first-class platform in the security hardening feed.Modified files (24)
Latest Contributors(0)
picoclawtargets so the shared feed and CLI experience surface the new platform consistently.Modified files (11)
Latest Contributors(2)