Skip to content

feat: add docker-destructive-guardrails skill - #17

Merged
dgageot merged 3 commits into
mainfrom
feat/docker-destructive-guardrails
Sep 22, 2026
Merged

dgageot merged 3 commits into
mainfrom
feat/docker-destructive-guardrails

Conversation

@glours

@glours glours commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

What I did
AI agents need explicit guardrails so they never run an irreversible Docker command without the user's consent. Adds a confirm-before- destroy policy: docker-compose-patterns documents its own destructive commands, and a new docker-destructive-guardrails skill covers the generic Docker CLI, audited against real --help output instead of picked ad hoc.

Container commands get a two-tier model so cleaning up an agent's own disposable test container doesn't need the same friction as deleting a volume. All four skills cross-reference each other for routing.
Related issue
N/A

Checklist

  • My commits are signed off (DCO).
  • task validate and task eval pass locally.

(not mandatory) A picture of a cute animal, if possible in relation to what you did
image

@melmennaoui

melmennaoui commented Aug 21, 2026

Copy link
Copy Markdown

Review: docker-destructive-guardrails

The skill content and eval coverage are solid — the two-tier container model is well-reasoned, the flag semantics are audited against real --help output, and the ownership boundary between standalone and Compose-managed volumes is clean. A few mechanics issues inline, but the main thing I want to flag for team discussion is the interface with docker-agent's built-in safety classifier, since there are a few places where the skill makes promises the runtime cannot keep.


Interface gaps with docker-agent's safety mechanism

docker-agent has a layered safety system (pkg/safety + pkg/runtime/toolexec): every shell command is classified as safe, destructive, or unknown via a regex pattern taxonomy (safety_patterns.json), and the (mode × label) table in toolexec/permissions.go maps that onto Allow / Ask / Deny before execution. The skill operates upstream of that — Gordon reads it before deciding what command to emit — but the runtime gating fires after, independently. A few tensions result:

1. Tier 1 auto-proceed is blocked by the runtime in Balanced/Strict mode

The skill says:

When every condition holds, the agent removes the container, states what it did, and proceeds — no blocking confirmation is required first.

But docker rm <id> is classified MEDIUM blast-radius ClassDestructive in safety_patterns.json. In Balanced mode applyMode() returns OutcomeAsk for anything that isn't ClassSafe, so the runtime will prompt the user regardless of what the skill says about Tier 1 being low-friction. The Tier 1 no-confirmation guarantee effectively only holds in Autonomous mode — the one mode where the runtime approves everything. The skill should document this, or we need to align with the safety team on an override path for Tier 1 cases.

2. Autonomous mode bypasses Tier 2's hard guarantees

The skill is emphatic that docker kill is always Tier 2 and that user reassurance cannot downgrade it. In Autonomous mode applyMode() returns OutcomeAllow for everything, so none of the Tier 2 friction the skill promises is enforced by the runtime. Users running Gordon in Autonomous mode get weaker-than-documented guarantees. The skill should call this out explicitly.

3. docker buildx rm is missing from safety_patterns.json

The skill documents docker buildx rm as a flat-rule command, but the classifier has no pattern for it. The functional outcome in Balanced mode is still OutcomeAsk (ClassUnknown → Ask), but the confirmation UI will show "unknown" blast radius and no category, rather than something meaningful. These two should agree.

4. docker kill and docker context rm blast-radius conflicts

safety_patterns.json assigns both LOW blast radius. The skill treats both as requiring the same confirmation friction as volume deletion. A user sees a "low" badge in the confirmation UI while the skill's reasoning explains why these commands are high-stakes. The classifier needs a pass to align with the skill's intent.


Smaller issues

  • No eval check for allow_implicit_invocation: true in agents/openai.yaml — this is the field that makes the skill trigger implicitly, and a careless edit that disables it would break the skill silently with no eval failure catching it.
  • Cross-skill index header says "every command below requires explicit user confirmation" — technically wrong for the Tier 1 docker rm row.
  • skill.yaml do_not_use_when is missing the external: true carve-out documented in SKILL.md (treat external volumes as standalone case). Agents routing via skill.yaml alone get a slightly looser boundary.
  • docker stop is classified destructive (LOW) in the runtime but the skill is silent on it. Gordon in Balanced mode will be prompted to confirm docker stop with no skill-level guidance.

Suggested next step

Before merging, I'd suggest a short sync with the docker-agent safety team to agree on:

  1. Whether Tier 1 needs a classifier-level exception path (e.g. a ClassSafe override for agent-created stopped containers), or whether the skill doc simply needs to acknowledge the runtime-mode dependency.
  2. Whether the Autonomous-mode bypass of Tier 2 guarantees is acceptable as-is (document it) or needs a ForceAsk permissions rule to enforce Tier 2 unconditionally.
  3. A patch pass to align safety_patterns.json with the skill's blast-radius judgements for docker kill, docker context rm, and docker buildx rm.

The behavioral policy itself is in good shape — this is about making sure the two layers agree on what they're each enforcing.

cc. @aheritier @dgageot @trungutt @hamza-jeddad

@glours
glours requested a review from trungutt August 28, 2026 09:43
@trungutt

Copy link
Copy Markdown

docker image prune -a scope looks slightly off in three places — it's "not referenced by any container" (running or stopped), not "not referenced by a running container". A stopped container also protects its image from pruning.

Sources:

  • Official docs, docker image prune: "If -a is specified, also remove all images not referenced by any container."https://docs.docker.com/reference/cli/docker/image/prune/
  • CLI warning shown at docker image prune -a runtime: "This will remove all images without at least one container associated to them."

Locations to update:

  • skills/docker-destructive-guardrails/SKILL.md:46docker image prune -a — deletes every image not referenced by a running container
  • skills/docker-destructive-guardrails/references/docker-cli-destructive-commands.md:43system prune -a widens image deletion to "any image not referenced by a running container"
  • skills/docker-destructive-guardrails/references/docker-cli-destructive-commands.md:79 — image prune -a: "deleted if nothing is currently running from it"

The dialogue in checks/verification.md:94 inherits the same phrasing and would be worth updating too.

This one matters more than the usual doc nit because it's the specific rule the skill is trying to protect the user from getting wrong: an agent following the current phrasing could conclude a stopped container's image is safely prunable, which is the exact "surprising deletion" outcome this skill exists to prevent.

Suggested phrasing: "any image not referenced by any container (running or stopped)" — or, if you want to preserve the "system prune -a is deceptively broad" framing, note that system prune -a first prunes stopped containers, then prunes images now unreferenced, so previously-only-stopped-container-referenced images do get removed by the combined pass even though image prune -a alone would not touch them until those containers are gone.

@aheritier
aheritier marked this pull request as draft September 21, 2026 15:30
AI agents need explicit guardrails so they never run an irreversible
Docker command without the user's consent. Adds a confirm-before-
destroy policy: docker-compose-patterns documents its own destructive
commands, and a new docker-destructive-guardrails skill covers the
generic Docker CLI, audited against real `--help` output instead of
picked ad hoc.

Container commands get a two-tier model so cleaning up an agent's own
disposable test container doesn't need the same friction as deleting
a volume. All four skills cross-reference each other for routing.

Signed-off-by: Guillaume Lours <glours@users.noreply.github.com>
The image prune -a description said images are protected by a
running container, but Docker considers any container — running or
stopped — a reference. That gap could lead an agent to conclude a
stopped container's image is safely prunable, the exact surprising
deletion this skill exists to prevent.

Also document docker stop, which the runtime's safety classifier
flags as destructive but the skill was silent on; tighten the
cross-skill index header and the compose external-volume carve-out;
and add an eval check guarding allow_implicit_invocation.

Signed-off-by: Guillaume Lours <glours@users.noreply.github.com>
@glours
glours force-pushed the feat/docker-destructive-guardrails branch from ba7e366 to 018416e Compare September 22, 2026 08:43
@glours
glours marked this pull request as ready for review September 22, 2026 08:43
@glours
glours requested review from a team as code owners September 22, 2026 08:43
@glours
glours requested a review from ndeloof September 22, 2026 08:43

@docker-agent docker-agent Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Assessment: 🟡 NEEDS ATTENTION

Partial review: some changed content was unavailable or omitted from the initial context. This is not an approval of the entire change set.

Adds Docker destructive-command guardrails and cross-skill routing.


`docker kill` is always Tier 2 (see the reference for why no stopped-container exception exists for it). Also Tier 2: `docker rm -f` on a *running* container, any unscoped sweep regardless of container state (e.g. `docker rm -f $(docker ps -aq)`, "remove/kill all containers"), a container the agent didn't create and has no context on, and any action taken on the agent's own initiative rather than an explicit user ask. These carry the same confirmation bar as every flat-rule command in this skill: state exactly what will be lost and get explicit confirmation before running anything — no exception carved out.

### `docker stop` — reversible, outside the tier model

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[medium] Tier 1 incorrectly includes the host-wide docker container prune sweep.

docker container prune cannot meet the stated Tier 1 conditions because it always removes every stopped container on the host, rather than one specifically identified container. Yet this paragraph explicitly lists it as Tier 1, while Tier 2 says any unscoped sweep requires confirmation. For example, when an agent cleans up its own test container while another stopped container exists, following the Tier 1 listing can remove both without confirmation. Remove docker container prune from Tier 1 so it follows the existing Tier 2 rule for unscoped sweeps.

Suggested change
### `docker stop` — reversible, outside the tier model
Applies only to `docker rm <name>` on an already-stopped container and `docker rm -f <name>` on a container the agent itself created and started earlier in the same session purely for testing or debugging. All of the following must hold: the container is already stopped, or was created/started by the agent itself this session for testing/debugging; there's no known unpersisted state at risk; and the action targets one specific, identified container rather than an unscoped sweep. When every condition holds, the agent removes the container, states what it did, and proceeds — no blocking confirmation is required first.
Confidence Score
🟢 strong 100/100

dgageot
dgageot previously approved these changes Sep 22, 2026
@dgageot

dgageot commented Sep 22, 2026

Copy link
Copy Markdown
Member

@glours can you look at the reviewer's feedback?

@glours

glours commented Sep 22, 2026

Copy link
Copy Markdown
Collaborator Author

@melmennaoui the mechanical fixes (image prune -a scope, docker stop, cross-skill index header, skill.yaml carve-out, allow_implicit_invocation eval check) are in.

The remaining open item is the four-point interface gap with docker-agent's runtime classifier (Tier 1 blocked in Balanced/Strict, Tier 2 bypassed in Autonomous, docker buildx rm missing from safety_patterns.json, docker kill/docker context rm blast-radius mismatch). None of these are fixable from this repo.

Before we pick a direction here, we'd like the your team's input on your three suggested discussion points:

  1. Classifier-level exception path for Tier 1, or just document the runtime-mode dependency in the skill?
  2. Is the Autonomous-mode bypass of Tier 2 acceptable as documented behavior, or does it need a ForceAsk rule?
  3. Timeline for aligning safety_patterns.json blast-radius/coverage with docker kill, docker context rm, and docker buildx rm.

cc @aheritier @dgageot @trungutt @hamza-jeddad, how do you want to run this sync?

Tier 1 and Tier 2 both matched the same case — an agent's own
still-running test container removed without an explicit user ask —
with no stated precedence, so the text could justify either outcome.

The cross-skill index also still described docker-sandboxes-lifecycle's
sbx guardrails as an unmerged, pending PR, so an agent routing off it
would treat existing guidance as nonexistent.

Also close eval-coverage gaps for three documented commands, add the
skill's required CODEOWNERS entry, and trim an overstated frontmatter
claim.

Signed-off-by: Guillaume Lours <glours@users.noreply.github.com>
@dgageot
dgageot merged commit 7852ce2 into main Sep 22, 2026
3 checks passed
@dgageot
dgageot deleted the feat/docker-destructive-guardrails branch September 22, 2026 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants