Skip to content

maxDefusePasses documents a termination argument that is false #4054

Description

@dwin-gharibi

Description

pkg/attachment bounds its replace-until-stable loop with maxDefusePasses = 8, justified by
this comment:

// maxDefusePasses bounds the replace-until-stable loop. Each pass strictly
// shortens the body (a match is always longer than nothing and is replaced by a
// constant), so this converges quickly; the bound only exists so a pathological
// input cannot spin.
const maxDefusePasses = 8

The premise is false. delimiterPlaceholder is 41 bytes; </document-x> is 13. Replacing the
latter with the former grows the body. Passes do not shorten it.

A second comment on defuseDelimiters is wrong in the opposite direction:

// Replacement repeats until the output is stable, because one pass can leave a
// delimiter-shaped residue behind: `</TAG</TAG>>` collapses to `[…removed]>` only
// after the second pass.

envelopeTagRe runs to the first >, so </TAG</TAG>> is a single match and collapses in
one pass, not two.

This is a documentation defect, not a behaviour defect. The loop does terminate and no
delimiter escapes — that was verified before filing (see below). But the reasoning is what
someone will rely on when tuning the constant or changing the placeholder, and it is wrong.

Expected Behavior

The comment states the argument that actually guarantees termination.

Actual Behavior

It states an argument that is measurably false, and a worked example whose pass count is wrong.

Steps to Reproduce

Measure a single pass over </document-x> with the envelope's own tag:

pass 1: len 13 -> 42   (GREW)

And over the residue shape </document-x</document-x>>:

pass 1: len 26 -> 43   (GREW)   — and it is already stable, i.e. one pass, not two

Docker Agent version

No response

OS & terminal

No response

Model used

No response

Error output

case 0 pass 1: len 13 -> 42  (GREW)
case 1 pass 1: len 26 -> 43  (GREW)
case 2 pass 1: len 117 -> 50  (shrank)


Only inputs with many delimiters shrink; the common single-delimiter case grows.

Screenshots

No response

Additional context

No break-out is possible today. Eighteen adversarial bodies — deep nesting, split brackets,
mixed case, trailing attributes, self-closing forms, whitespace padding, prefix-extending tags,
a body pre-seeded with the placeholder — were run through defuseDelimiters. Every one came back
with zero live delimiters, and every one converged in a single pass. The escaping is sound; this
issue is strictly about the stated reasoning.

The real termination argument is: delimiterPlaceholder contains neither < nor >, so it can
never form part of a new match. Every pass therefore strictly reduces the number of
delimiter-shaped tokens left in the body, and that count is a non-negative integer.

Why this is worth fixing rather than ignoring: the loop returns whatever it has if the bound
is ever hit — silently, with no error. So if someone later gives the placeholder angle brackets
(a plausible cosmetic change, e.g. <removed>), the real invariant breaks, the loop can exhaust
its bound, and a live delimiter is returned inside the envelope. The current comment gives no
hint that the placeholder's character set is load-bearing.

Metadata

Metadata

Assignees

No one assigned

    Labels

    area/docsDocumentation changes

    Type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions