Part 6: Sequential Patch Edits (description_edits) & Whole-Document Quick-Action Defense in MCP save_work_item

Series Context: Part 6 of 10 in GitLab Core Engineering: Architecture, MCP & CI/CD Deep Dives.

Featured MR: gitlab-org/gitlab!264862 (Merged into master) · Work Item: #635910

Why Sending Full Markdown Descriptions Over MCP Does Not Scale

When an AI agent fixes a typo or updates a single status checkbox in a GitLab issue or epic containing tables and implementation checklists, forcing it to resend the entire description string via save_work_item creates two real problems:

1. Token waste & truncation risk: Large GitLab issues run thousands of tokens. Re-generating the whole body risks subtly rewriting unrelated paragraphs.
2. Concurrent edit clobbering: Full-body overwrites silently erase edits another engineer made to a different section moments earlier.

In !264862, we added an update-only description_edits array to save_work_item:

{
  "project_id": "gitlab-org/gitlab",
  "work_item_iid": 635910,
  "description_edits": [
    { "old_str": "- [ ] Verify CI", "new_str": "- [x] Verify CI" },
    { "old_str": "Status: In review", "new_str": "Status: Merged" }
  ]
}

Implementing surgical text patching inside a Markdown engine that executes /quick_actions turns out to have fascinating Ruby and security subtleties.

Strict Uniqueness, Ordering & Ruby's sub Trap

Look at apply_single_description_edit inside app/services/mcp/tools/work_items/update_work_item_tool.rb:

def apply_single_description_edit(content, edit)
  edit_hash = edit.is_a?(Hash) ? edit.with_indifferent_access : {}
  unless edit_hash.key?(:old_str) && edit_hash.key?(:new_str)
    raise ArgumentError, 'Provide both old_str and new_str for each edit in description_edits'
  end

  old_str = edit_hash[:old_str].to_s
  new_str = edit_hash[:new_str].to_s

  raise ArgumentError, 'old_str cannot be empty in description_edits' if old_str.empty?

  occurrences = content.scan(old_str).length
  if occurrences == 0
    raise ArgumentError, "old_str was not found in description: #{old_str.truncate(60).inspect}"
  elsif occurrences > 1
    msg = [
      "old_str is ambiguous in description (#{occurrences} matches found);",
      "provide more surrounding context: #{old_str.truncate(60).inspect}"
    ].join(' ')
    raise ArgumentError, msg
  end

  content.sub(old_str) { new_str }
end

Three design choices here protect callers and maintainers alike:

1. Sequential reduce pipeline:

   new_description = edits.reduce(current_description) do |content, edit|
     apply_single_description_edit(content, edit)
   end

Edits apply left-to-right against the output of the previous edit. If edit #2 touches text updated by edit #1, it works deterministically—and if edit #1 introduces ambiguity, edit #2 fails loudly before mutating anything.

1. Exact-once match enforcement (occurrences == 1): If old_str appears twice (occurrences > 1), guessing first vs all silently corrupts documents. Failing with "provide more surrounding context" gives an AI agent the exact hint needed to expand old_str on the next call. Also notice content.scan(old_str) treats a String old_str literally (no regex interpretation!).
2. Ruby's sub(old_str) { new_str } block form: If you write content.sub(old_str, new_str), Ruby interprets \&, \1, \+, and \\ inside new_str as regex backreferences! Writing content.sub(old_str) { new_str } passes new_str verbatim, even when new_str contains shell backslashes or code snippets.

We also guarded the boundaries cleanly:

• Rejected description_edits inside CreateWorkItemTool::UPDATE_ONLY_PARAMS (you cannot patch a record being created).
• Enforced mutual exclusivity with description: "Provide either description or description_edits, not both".
• Re-checked total character length against Concerns::Constants::MARKDOWN_TEXT_MAX_LENGTH (1_048_576) after all sequential expansions complete.

Security Deep Dive: Why Quick-Action Validation Must Diff the Whole Document

GitLab MCP tools forbid injecting slash quick actions (/close, /spend, /award, /label) inside descriptions so prompt-injected issues cannot silently execute commands.

If we had naively checked validate_no_quick_actions!(edit[:new_str]) on each replacement snippet alone, two injection attacks would have bypassed it completely:

1. Deletion smuggling: Existing description:

   Please don't /close this issue

Attacker sends { "old_str": "Please don't ", "new_str": "" }.

Neither old_str nor new_str starts with /! Yet after deletion, the line begins with /close this issue—executing a quick action!

1. Newline smuggling: Same existing line: "Please don't /close this issue". Attacker sends { "old_str": "don't ", "new_str": "don't\n" }. Still no / anywhere in new_str! Yet injecting a newline pushes /close to column 0 of the next line.

Meanwhile, legitimate issues often already contain lines starting with / (like /etc/gitlab.rb), so blindly rejecting any new_description containing a leading / would freeze future unrelated edits to the issue!

Look at how validate_description_edits_quick_actions! threads that needle cleanly:

def validate_description_edits_quick_actions!(current_description, new_description)
  original_actions = current_description.lines.grep(Concerns::ContentValidation::QUICK_ACTION_PATTERN).map(&:strip)
  new_actions = new_description.lines.grep(Concerns::ContentValidation::QUICK_ACTION_PATTERN).map(&:strip)

  return if (new_actions - original_actions).empty?

  raise ArgumentError, 'Quick actions (commands starting with /) are not allowed in description'
end

By comparing the multiset of leading-slash lines after all edits against the original document, both deletion smuggling and newline smuggling fail immediately, while untouched lines like /etc/gitlab.rb remain editable.

Bonus Cleanup: Eliminating Duplicate Work Item Queries Across UrlParser & BaseTool

Notice apply_description_edits needed the WorkItem record (to read item.description), whereas UpdateWorkItemTool previously only called resolve_work_item_id (returning a Global ID string gid://gitlab/WorkItem/123).

Instead of running a second query (::WorkItem.find_by_id), we refactored UrlParser and BaseTool:

• Extracted UrlParser#find_work_item_from_url(url) returning the WorkItem model directly, and had resolve_work_item_from_url(url) delegate .to_global_id.to_s.
• Renamed BaseTool#resolve_work_item_from_params -> find_work_item_from_params, memoized @resolved_work_item in BaseTool#resolve_work_item, and had resolve_work_item_id return resolve_work_item.to_global_id.to_s.
• Verified with expect(::WorkItem).not_to receive(:find_by_id) in base_tool_spec.rb that resolving both the description and Global ID performs zero extra database lookups.

Takeaways for GitLab Engineers

1. Always use block form String#sub(target) { replacement } when applying user- or LLM-supplied patches—non-block sub(a, b) corrupts \& and \1 sequences.
2. Validate line-anchored invariants (like /quick_actions) against the post-patch document diff, never isolated edit chunks. Deletions and \n insertions can shift existing substrings into column 0.
3. Share model resolution cleanly: Return the ActiveRecord instance once (resolve_work_item) and derive .to_global_id.to_s from it rather than issuing duplicate DB queries.

Series Navigation

• Part 1: Taming EE::Project & Extracting Ai::Catalog (!269060)
• Part 2: Decoupling EE::Group Routing via Ai::DuoSettings::Paths (!269059)
• Part 3: Escaping Non-ASCII Git Refs in Workspaces — AppSec CWE-78 & CWE-116 (!262103)
• Part 4: Exposing Grape Routes to MCP — list_pipeline_schedules & Auth Traps (!264856)
• Part 5: Vacuous RSpec Mocks & Live Upstream Flag Rebases (!269061)
• Part 6 (This Article): Sequential description_edits & Quick-Action Defense in save_work_item (!264862)
• Part 7: Self-Correcting MCP Errors & JSON Schema Fixes (!264852, !264853, !264864)
• Part 8: Consolidating SavePipelineService & Active Polling Hints (!264858, !264863)
• Part 9: Surviving Dangerfile, Commit Trailers & Predictive CI Pipelines
• Part 10: Security Cleanups, Organization empty? & Review Etiquette (!261178, !261030, !264859)