Skip to content

Add autoban remediation command#2302

Merged
Patrick-Erichsen merged 1 commit into
mainfrom
pe/autoban-remediation
May 18, 2026
Merged

Add autoban remediation command#2302
Patrick-Erichsen merged 1 commit into
mainfrom
pe/autoban-remediation

Conversation

@Patrick-Erichsen
Copy link
Copy Markdown
Contributor

Summary

  • add admin POST /api/v1/users/remediate-autobans for malware autoban remediation
  • add clawhub-mod users remediate-autobans with dry-run default, apply mode, targeting, limit, since, JSON, and reason options
  • restore timestamp-matched skills/packages only when current trigger verdicts are resolved non-malicious, with safety guards for pending scans, still-malicious content, stale batches, and conflicting CLI flags

Tests

  • bun run test packages/clawhub-mod/src/commands/moderation.test.ts convex/httpApiV1.handlers.test.ts convex/autobanRemediation.test.ts
  • bun run lint -- convex/users.ts convex/packages.ts convex/skills.ts convex/autobanRemediation.test.ts packages/schema/src/schemas.ts packages/clawhub/src/schema/schemas.ts packages/clawhub-mod/src/commands/moderation.ts packages/clawhub-mod/src/commands/moderation.test.ts
  • bunx tsc --noEmit && bunx tsc -p packages/schema/tsconfig.json --noEmit && bunx tsc -p packages/clawhub/tsconfig.json --noEmit && bun run --cwd packages/clawhub-mod typecheck
  • AUTOREVIEW_AUTO_TESTS=0 .agents/skills/autoreview/scripts/autoreview --mode local

@Patrick-Erichsen Patrick-Erichsen requested a review from a team as a code owner May 18, 2026 23:09
@vercel
Copy link
Copy Markdown
Contributor

vercel Bot commented May 18, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
clawhub Ready Ready Preview, Comment May 18, 2026 11:09pm

@clawsweeper
Copy link
Copy Markdown

clawsweeper Bot commented May 18, 2026

Codex review: needs real behavior proof before merge.

Workflow note: Future ClawSweeper reviews update this same comment in place.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

Summary
This PR adds an admin autoban remediation HTTP endpoint and clawhub-mod users remediate-autobans command for dry-running or applying malware autoban unban/restoration decisions from current scanner verdicts.

Reproducibility: yes. Source inspection shows cmdRemediateAutobans discards inputAllowed and posts the write request whenever --apply is set, unlike existing ban/unban commands that require confirmation or --yes.

PR rating
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🦐 gold shrimp
Summary: The implementation has useful coverage, but missing real behavior proof and the apply-mode safety issue make it not quality-ready yet.

Rank-up moves:

  • Add confirmation or --yes protection for --apply and cover non-interactive behavior in tests.
  • Attach redacted terminal output, logs, or a recording showing dry-run and one targeted apply against a real setup.
  • Update the security-moderation spec with the intended autoban remediation policy.
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

PR egg
🎁 Pass real behavior proof to wake the egg and unlock a hatchable treat.

Where did the egg go?
  • The egg game starts only after the PR passes the real-behavior proof check.
  • Before that, no creature, rarity, or ASCII portrait is rolled. The treat waits for real proof.
  • This is still just collectible flavor: proof affects review readiness, not creature quality.

Real behavior proof
Needs real behavior proof before merge: The PR body lists tests, lint, typechecks, and autoreview only; it needs redacted terminal output, logs, or a recording showing the real CLI/API dry-run and targeted apply behavior before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge
Why this matters: - Merging as-is would let an admin run --apply for the default remediation batch without the interactive confirmation or --yes guard used by existing ban/unban commands.

  • The PR does not yet include after-fix real behavior proof for the live CLI/API flow; unit tests do not prove an operator can safely dry-run and apply against a real setup.
  • The new autoban remediation policy changes a security-sensitive moderation flow and should be captured in the internal security-moderation spec before landing.

Maintainer options:

  1. Gate apply mode before merge (recommended)
    Add the same confirmation or --yes protection used by existing moderation write commands, update focused tests/specs, and attach redacted real CLI/API proof for dry-run plus targeted apply.
  2. Accept explicit apply as policy
    Maintainers could decide that admin-only --apply is sufficient, but that intentionally owns the accidental mass-remediation risk.

Next step before merge
Contributor action and maintainer security review are needed because the PR lacks real behavior proof and changes a sensitive admin remediation flow.

Security
Needs attention: The diff adds an admin remediation path that can unban accounts and restore content, but the CLI apply mode lacks an explicit confirmation gate.

Review findings

  • [P2] Require confirmation before applying remediation — packages/clawhub-mod/src/commands/moderation.ts:207
Review details

Best possible solution:

Land this after --apply has an explicit confirmation/--yes gate, regression coverage and spec intent are updated, and the contributor adds redacted live CLI/API proof for dry-run and targeted apply.

Do we have a high-confidence way to reproduce the issue?

Yes. Source inspection shows cmdRemediateAutobans discards inputAllowed and posts the write request whenever --apply is set, unlike existing ban/unban commands that require confirmation or --yes.

Is this the best way to solve the issue?

No. The backend approach may be workable, but the CLI write path needs the existing confirmation pattern and live proof before this is the safest maintainable solution.

Label justifications:

  • P2: This is a normal-priority admin/security improvement with limited operator blast radius, but it should not merge without the safety gate and proof.
  • merge-risk: 🚨 security-boundary: The PR adds an admin path that can unban accounts and restore previously hidden packages/skills, so a weak operator gate can affect moderation boundaries.

Full review comments:

  • [P2] Require confirmation before applying remediation — packages/clawhub-mod/src/commands/moderation.ts:207
    cmdRemediateAutobans accepts --apply and immediately posts the write payload while discarding inputAllowed. Unlike the existing ban/unban commands, a terminal typo or non-interactive invocation can remediate the default batch without an explicit confirmation or --yes gate, so please add that guard before write mode and cover it in tests.
    Confidence: 0.84

Overall correctness: patch is incorrect
Overall confidence: 0.82

Security concerns:

  • [medium] Apply mode can restore accounts without confirmation — packages/clawhub-mod/src/commands/moderation.ts:207
    The new CLI command can apply remediation for a batch as soon as --apply is present, while existing moderation write commands require interactive confirmation or --yes; this increases the chance of accidental account/content restoration by an admin operator.
    Confidence: 0.84

What I checked:

  • PR diff adds mass remediation command: The patch adds cmdRemediateAutobans, which supports untargeted --apply remediation and sends the write payload after discarding inputAllowed. (packages/clawhub-mod/src/commands/moderation.ts:187, 1c09dd4ebbd5)
  • Existing moderation commands require confirmation: Current cmdBanUser and cmdUnbanUser use inputAllowed, interactive prompts, and --yes before write operations, which this new apply path does not mirror. (packages/clawhub-mod/src/commands/moderation.ts:44, 1a00013c21b1)
  • Backend change touches security-sensitive account restoration: The patch adds an internal mutation that clears deletedAt/banReason, logs user.unban, and restores owned skills and packages for selected autoban candidates. (convex/users.ts:831, 1c09dd4ebbd5)
  • Current spec makes autobans security-sensitive: The moderation spec says malicious scanner paths schedule account-level autoban/token revocation and that manual unban leaves revoked tokens revoked, so remediation changes should keep a conservative operator gate. (specs/security-moderation.md:70, 2aa2a449e5fc)
  • Proof is test-only: The PR body lists unit tests, lint, typechecks, and autoreview, but no redacted terminal output, logs, recording, or live API/CLI run showing the new remediation behavior after the change. (1c09dd4ebbd5)

Likely related people:

  • Peter Steinberger: Current main history/blame shows the autoban mutation, moderation spec, schema, and existing moderator CLI command surface were introduced in commit 1a00013. (role: feature-history owner; confidence: high; commits: 1a00013c21b1; files: convex/users.ts, convex/skills.ts, packages/clawhub-mod/src/commands/moderation.ts)
  • Patrick-Erichsen: Current main history shows recent scan timestamp work in the same moderation/scanner area, and this PR also builds on that surface. (role: recent area contributor; confidence: medium; commits: 7cf16ebb28aa; files: convex/skills.ts)

Codex review notes: model gpt-5.5, reasoning high; reviewed against 2aa2a449e5fc.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. labels May 18, 2026
@Patrick-Erichsen Patrick-Erichsen merged commit 6814af9 into main May 18, 2026
29 of 30 checks passed
@Patrick-Erichsen Patrick-Erichsen deleted the pe/autoban-remediation branch May 18, 2026 23:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. P2 Normal backlog priority with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant