Skip to content

[fix] Sieve RFC 5703 still unusable end-to-end after #548: unregistered foreverypart/replace capabilities, inverted capability enforcement, folded-boundary replace no-op #556

Description

@jfexyz

Describe the bug

Node.js version: N/A — verified by executing the current master (428eba35) sieve helpers directly in a sandbox with real dependencies (peggy, re2, mailsplit, mailparser, libmime, mongoose); symptom originally observed on the hosted forwardemail.net service.

OS version: N/A — Sieve script authored via the forwardemail.net dashboard for a domain alias.

Description: Follow-up to #548 (fixed by 8665211). That fix corrected the RFC 5703 engine logic, but the feature still cannot work end-to-end for real mail. Executing the script below through the real delivery path — createSieveIntegration({...}).processMessage({...}), which is what helpers/parse-payload.js:1223-1230 invokes for inbound mail — surfaces four further bugs, two of which are independently blocking.

  1. (Blocking) foreverypart, replace, extracttext, and enclose are not recognized capability strings anywhere, so a spec-compliant script that declares them in require is rejected outright with Unsupported capability: foreverypart.
  2. (RFC compliance) Capability enforcement happens only at require-time; the commands themselves are dispatched with no capability guard. Combined with bug 1 this is inverted — a compliant script is rejected, a non-compliant one runs.
  3. (Blocking for real-world mail) applyReplaceActions()'s boundary-detection regex cannot match a folded boundary= parameter, so replace silently no-ops on any message whose top-level Content-Type is folded — which is what Microsoft Exchange Online emits, because its boundary strings are long.
  4. (Minor) parseMimeTree()'s header parser does not unfold RFC 5322 continuation lines, producing garbage header keys.

Plus a code-hygiene cluster around enabledExtensions (details at the end) that is not causing live misbehavior but is a latent trap.

I have failing tests for all of these in the linked PR. As with #548, these are reproductions, not fixes — I'm not confident enough in engine.js/integration.js internals to fix them safely.

1. foreverypart/replace/extracttext/enclose are not valid capability strings

engine.js defines two capability sets: DEFAULT_CAPABILITIES (lines 83-91) and EXTENDED_CAPABILITIES (lines 94-120). helpers/sieve/filter-handler.js defines a third, SUPPORTED_CAPABILITIES (lines 35-58), which is the one that actually matters for live mail (see "blast radius" below). The strings foreverypart, replace, extracttext, and enclose appear in none of the three — even though 8665211 added working implementations of exactly these commands (executeForeverypart at engine.js:1915, executeExtracttext at :1993, executeReplace at :2131, executeEnclose at :2159) and your documentation claims support for these capabilities: app/views/faq/index.md:2850 advertises mime as "✅ Full — foreverypart, break, extracttext, replace, enclose commands; :mime, :type, :subtype, :contenttype, :param, :anychild tags", a claim repeated across every translated FAQ (index-de.md, index-fr.md, index-zh.md, and the rest).

processRequires() (engine.js:292-308) throws on any capability not found, so this script:

require ["foreverypart", "mime", "replace"];

fails immediately. Running my real script through processMessage() gives:

{
  "action": "keep",
  "scriptExecuted": true,
  "errors": [{ "type": "execution_error", "message": "Unsupported capability: foreverypart" }],
  "headerChanges": [],
  "modifiedRaw": null
}

The script is rejected at the first capability check, before any condition is evaluated — so the entire #548 fix can never manifest for a script that declares its extensions per RFC 5703.

Note that 8665211's own tests only ever require "mime", never require "foreverypart", which is why this gap isn't visible in the existing suite.

2. Capability checks are enforced at require-time only, never at execution-time

processRequires() is the sole place capabilities are enforced. The dispatch for foreverypart and for replace (engine.js:523-524) has no hasCapability() guard. Grepping the engine's runtime capability state (enabledCapabilities) shows it is consulted in exactly two places, both for the variables extension (engine.js:1592, :1995).

Consequence: simply omitting foreverypart/replace from require makes the script execute with errors: []. RFC 5703 §3 and §5 require the extension be declared before use, so this is backwards on both ends — a compliant script is rejected (bug 1), while a non-compliant one is accepted and executed.

I want to flag this as a trap rather than a workaround: on folded-boundary mail (bug 3), the require-less version reports modifiedRaw !== null and successfully stamps the addheader marker onto the message, while the image it was supposed to redact is still byte-for-byte present. It converts a loud, safe failure into a silent one.

3. applyReplaceActions() boundary regex fails on folded Content-Type headers

helpers/sieve/integration.js:1044-1046:

const ctMatch = rawStr.match(
  /content-type:[^\r\n]*boundary="?([^"\s;]+)"?/i
);

[^\r\n]* cannot cross a line break. RFC 5322 §2.2.3 allows header field bodies to be folded onto continuation lines, and Exchange Online routinely does so for multipart/related because its boundary strings are long:

Content-Type: multipart/related;
	boundary="_004_AS8P194MB1974EXAMPLEBOUNDARY_";
	type="multipart/alternative"

Against that, the regex returns null, and applyReplaceActions() returns the raw message unchanged — replace becomes a silent no-op. Verified by calling applyReplaceActions() directly: with a folded boundary the output is byte-identical to the input (Buffer.compare(raw, result) === 0), while the identical message with an unfolded boundary is correctly rewritten. Verified against a real Exchange Online message as well as the synthetic fixture in the linked PR.

This is independent of bugs 1 and 2: even with capabilities resolved, replace still does nothing for this very common class of message.

4. parseMimeTree() does not unfold header continuation lines

helpers/sieve/integration.js:799-806 splits chunk.getHeaders() on /\r?\n/ and treats each physical line as a complete header, calling line.indexOf(':') per line. A folded continuation line therefore becomes its own bogus header. For a part with:

Content-Disposition: inline; filename="x.png";
	creation-date="Mon, 21 Jul 2026 10:00:00 GMT"

the parsed headers come out as:

{
  "content-type": "image/png",
  "content-disposition": "inline; filename=\"x.png\";",
  "creation-date=\"mon, 21 jul 2026 10": "00:00 GMT\"",
  "content-id": "<img1>"
}

— the colon inside the timestamp is misread as the header separator. In practice the content-disposition value survives well enough for :type extraction (the inline token precedes the fold), so this is real but not itself blocking; reporting it because it is adjacent, cheap to fix, and will corrupt any test that reads a folded parameter.

Investigated and ruled out — please don't chase this

An earlier draft of this report theorized a much larger blast radius: SieveIntegration's constructor (integration.js:159-160) passes { extensions: this.config.enabledExtensions } while SieveEngine's constructor (engine.js:137-141) reads only options.capabilities, which would appear to silently disable vacation, variables, editheader, mime and everything else beyond the 6-item DEFAULT_CAPABILITIES for all live scripts.

That conclusion is wrong, and I want to be explicit about it so nobody wastes time on it. Two independent reasons:

  1. hasCapability() (engine.js:185-189) unconditionally ORs in EXTENDED_CAPABILITIES, so those extensions are available regardless of what the constructor received.
  2. That engine instance is dead code. processMessage() constructs a fresh SieveFilterHandler (integration.js:235-241) and executes scripts through its engine (filter-handler.js:84-90 via createEngine()), which passes capabilities: correctly. this.engine is assigned at integration.js:159 and never read again. I confirmed this empirically by monkey-patching si.engine.execute to throw before calling processMessage() — it never fired.

The key mismatch is nonetheless real as written, and worth fixing as a latent trap: anyone who later wires this.engine up will get silently ignored configuration. Related and also harmless today: enabledExtensions is a no-op end-to-end — its only real consumer is SieveIntegration.validateScript() (integration.js:854-857), which is never called anywhere in the repo, and the script-save controllers each build their own SieveSecurityValidator without allowedExtensions, so validateExtensions() (security.js:266-289) short-circuits to valid.

Finally, DEFAULT_CONFIG.enabledExtensions still carries comments (integration.js:110, :112-115) saying foreverypart/replace/enclose/extracttext are "NOT IMPLEMENTED" and that mime is "partial: ... foreverypart/extracttext not yet". git blame dates those to c76b41f (2026-07-05), 17 days before 8665211 — they are stale post-fix, and they contradict the commit message of 8665211. Worth updating alongside bug 1, since they're the natural place a maintainer would look to decide which capability strings should be registered.

Actual behavior

The script validates and saves without error, is marked active, and never modifies delivered mail. Executed through processMessage() it fails with Unsupported capability: foreverypart and makes no changes at all. With the extensions omitted from require it executes, adds the diagnostic header, and still does not remove the image, because the boundary regex never matched.

Expected behavior

A script declaring require ["foreverypart", "mime", "replace"] should be accepted and executed; using those commands without declaring them should be an error; and a matched replace should rewrite the MIME part regardless of whether the top-level Content-Type header is folded.

Code to reproduce

require ["fileinto", "foreverypart", "mime", "replace", "relational", "comparator-i;ascii-numeric", "editheader"];

if anyof (
    address :domain :is "from" ["example.com"]
) {
    foreverypart {
        if allof (
            header :mime :type :is "Content-Disposition" "inline",
            header :mime :type :is "Content-Type" "image",
            exists :mime "Content-ID",
            size :under 100K
        ) {
            replace "[image removed by filter]";
            addheader "X-Signature-Filter" "by Forward Email Sieve";
        }
    }
}

Against a multipart/related message from the matching sender with an inline image/png part carrying Content-Disposition: inline and a Content-ID, whose top-level Content-Type folds the boundary= parameter onto a continuation line (as Exchange Online emits).

  1. Save and activate the script on an alias, send a matching message → Unsupported capability: foreverypart, nothing modified (bugs 1/2).
  2. Remove "foreverypart" and "replace" from the require line and repeat → script executes, X-Signature-Filter is added, image is not removed (bugs 2/3).

Failing tests for all of the above are in the linked PR, written against the existing node:test harness in test/sieve/ with synthetic fixtures — all reproduce at the unit level. Two go into test/sieve/engine.js and test/sieve/filter-handler.js; the three integration-layer ones go into a new test/sieve/integration-unit.js, since the existing test/sieve/integration.js is an AVA suite requiring the full MX/IMAP/SQLite harness (compare test/sieve/mx-integration.js, which is the existing node:test unit counterpart). Baseline is 322/322 passing; with these added it is 328 tests, 323 passing, 5 failing — the 5 new ones, plus an intentionally-passing unfolded control.

Questions for maintainers

  1. For bug 1, should foreverypart/replace/extracttext/enclose be added to EXTENDED_CAPABILITIES in engine.js, to SUPPORTED_CAPABILITIES in filter-handler.js, or both? The three capability lists have drifted apart and it isn't obvious which is intended to be authoritative — happy to send a fix PR once you've indicated the preferred shape. One hypothesis, based on the FAQ entry above listing all five commands under a single mime row, and on 8665211's tests exercising foreverypart behind require "mime" alone: the implementation may have been designed around mime as an umbrella capability. Note that RFC 5703 defines these as distinct capability strings — foreverypart (§3), mime (§4), replace (§5), enclose (§6), extracttext (§7) — so a strictly compliant implementation should accept all five in require, and treating mime as an umbrella would be an extension beyond the spec rather than an implementation of it. Registering all five separately looks like the spec-correct fix; I'd rather confirm than assume, since it determines whether bug 2's execution-time guard should check the individual capability per command or a single umbrella.
  2. For bug 2, do you want execution-time capability guards added (strict RFC 5703 behavior), or is the permissiveness deliberate? Tightening it would be a breaking change for any script currently relying on the require-less form.
  3. For bug 3, would you accept a fold-tolerant boundary parse (or reusing mailsplit's already-parsed tree rather than re-regexing the raw string)? The latter seems more robust but is a bigger change.
  4. Is SieveIntegration.this.engine intended to be used at some point, or should it (and the unused validateScript/enabledExtensions plumbing) be removed?

Checklist

  • I have searched through GitHub issues for similar issues.
  • I have completely read through the README and documentation.
  • I have tested my code with the latest version of Node.js and this package and confirmed it is still not working.

* This issue and the resulting PR were researched and written in part by Claude, but I have verified the claims as much as I am able to (being new to this codebase).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions