request-validation: 29 constraint/param-constraint negative tests false-positive (server returns 200, not 400) — not spec drift

[esraagamal6]

Summary

In the camunda-oca secured request-validation suite, 29 negative tests assert 400 but the server returns 200 — the malformed value rides on an otherwise-valid request, reaches the backend, and is accepted rather than rejected. These are false-positive failures: the server does not enforce the constraint the spec declares (or enforces it differently), so the generated 400 assertion can never pass against a real backend.

This is distinct from the path-id precondition class (#352) and the string path-param 400/404 class (#363), and from the /v2/setup/user bootstrap 403s (#121) — those are already tracked. This got 200 class has no owning issue.

Proof it is NOT spec-vs-server drift

The suite is generated from a pinned 8.9-era spec; the broker is 8.10.0-SNAPSHOT, so "version drift" was the obvious suspect. Ruled out:

• Re-fetched the spec at camunda/camunda@main (8.10) and regenerated, then ran the secured suite against the matching 8.10.0-SNAPSHOT broker.
• The got 200 count was essentially unchanged: 30 → 29 (only one missing-required case dropped). The 16 constraint-violation + 13 param-constraint-violation 200s persisted identically.

So matching the spec to the server does not resolve them — the server simply doesn't enforce these constraints.

The 29 failures — three patterns

1. tenantId constraint-violation (8) — server accepts a tenantId that violates the declared pattern/length:

• publishMessage (tenantId)
• broadcastSignal (tenantId)
• getProcessDefinitionInstanceVersionStatistics (filter.tenantId, 4 variants)

2. operationReference out-of-range constraint-violation (8) — server does not range-check operationReference on batch operations (variants #2/#3, e.g. 0 / -99):

• cancelProcessInstancesBatchOperation, deleteProcessInstancesBatchOperation, modifyProcessInstancesBatchOperation, resolveIncidentsBatchOperation

Note vs #352: #352/#124 list operationReference variants too, but for operations with a synthetic path id (e.g. resolveIncident, deleteProcessInstance) those surface as 404 (path-id masks body validation) and are covered there. The batch operations above have no path id, reach validation, and return 200 — a different root cause that belongs here.

3. Path-param length-max violation (13) — an over-long path key is accepted and the search just returns empty instead of 400:

• search{Clients,Users,Roles,MappingRules,GroupIds,Groups}For{Group,Role,Tenant} (groupId / roleId / tenantId length-max)

Why this matters

These 29 erode trust in the secured suite: they fail on every run against a real backend for reasons unrelated to the code under test. Combined with #352 (404 class) and #121 (403 class), they account for the bulk of the standing secured-suite red.

Proposed direction

Per-pattern triage — each case is either (a) a generated test the suite should stop emitting / not assert 400 for (the server legitimately doesn't constrain that field at that endpoint), or (b) a genuine upstream server gap to file against camunda/camunda. Suggested:

• Add a precondition/affordance check so constraint-violation / param-constraint-violation scenarios are only emitted for fields the backend actually enforces (mirrors the "don't emit a test the contract doesn't require" theme in negative-validation generator: precondition gap causes 32 false-positive failures against real backends (and 1 spurious additional-prop case) #352's Class 3).
• For fields that should be enforced (e.g. tenantId format, operationReference range), file the corresponding server-side bugs and keep the tests as known-failing references until fixed.

Repro

# 8.10 spec + 8.10 server (secured: UNPROTECTEDAPI=false, basic auth demo/demo)
SPEC_REF=main CONFIG=camunda-oca npm run fetch-spec
CONFIG=camunda-oca npm run generate:request-validation
RV_PROFILE=secured CORE_APPLICATION_URL=http://localhost:8080 \
  CAMUNDA_BASIC_AUTH_USER=demo CAMUNDA_BASIC_AUTH_PASSWORD=demo \
  npm run test:pw:request-validation
# -> 29 tests assert 400, receive 200 (see categories above)

Found while verifying #362 / PR #370 against 8.10.0-SNAPSHOT (full secured-suite triage).

[jwulf]

Recommendation: config-driven unenforcedConstraints suppression (no-op default)

I traced all three patterns to two generators that emit expectedStatus: 400 purely from spec-declared constraints, with no check that the backend enforces them:

• request-validation/src/analysis/constraintViolations.ts (body) → pattern 1 (tenantId format/length) and pattern 2 (operationReference range on batch ops).
• request-validation/src/analysis/paramConstraintViolations.ts (path/query) → pattern 3 (length-max on identifier path params).

There is already a clean, accepted precedent for exactly this class of false-positive: enumCaseInsensitive (#129) and the URL-collapsing path-segment elision (#147). Both are config-driven suppressions of a known "spec says 400, server says otherwise" class, and both default to a no-op so non-camunda-oca configs are unaffected. I'd mirror that design rather than invent a new mechanism.

Proposed shape

A new optional block in configs/<config>/request-validation.json:

{
  "unenforcedConstraints": {
    "body": [
      {
        "field": "tenantId",            // matched on the target's leaf segment
        "kinds": ["patternMismatch", "aboveMaxLength", "wayAboveMaxLength",
                  "belowMinLength", "emptyString"],
        "$reason": "OCA backend does not enforce tenantId format/length",
        "$upstream": "camunda/camunda#<NNN>"
      },
      {
        "field": "operationReference",
        "kinds": ["belowMinimum", "wayBelowMinimum", "belowExclusiveMinimum",
                  "atMinimumMinusEpsilon"],
        "$reason": "batch ops (no path id) do not range-check operationReference",
        "$upstream": "camunda/camunda#<NNN>"
      }
    ],
    "param": [
      {
        "name": "groupId", "in": "path", "kinds": ["length-max"],
        "$reason": "over-long lookup key returns empty result set, not 400"
      }
      // roleId, tenantId path params likewise
    ]
  }
}

• Loaded in request-validation/src/config.ts alongside enumCaseInsensitive, defaulting to empty ({ body: [], param: [] }) → observably a no-op for every other config.
• constraintViolations.ts and paramConstraintViolations.ts consult it and skip matching (field|param, kind) mutations.
• Matching the body field on the leaf segment of target makes it class-scoped (per AGENTS.md): it suppresses tenantId / filter.tenantId across all operations, not just the listed instances, because the server behaviour is field-intrinsic — so a sibling endpoint with the same unenforced field can't silently reintroduce the false-positive.

One required prerequisite

Body constraint-violation scenarios currently carry the mutation kind only in their id/description, not in a structured field (constraintKind is set on param scenarios only — see model/types.ts:116). I'd populate constraintKind on body scenarios too, so suppression matches precisely rather than by string-sniffing the id.

On the "genuine server gap" cases

Patterns 1 and 2 (tenantId format, operationReference range) are arguably constraints the server should enforce. My recommendation is still to stop emitting them rather than keep them as known-failing: a perpetually-red test against a real backend carries no signal, whereas the config $reason + a filed camunda/camunda issue ($upstream) preserves the record of the gap. If/when upstream fixes it, deleting the suppression entry re-arms the test. (Pattern 3 is a legitimate non-constraint — an over-long lookup key is a cache miss, not a malformed request — so it's a permanent suppression.)

Tests (red/green/class-scoped)

• Unit on both generators: a config entry removes exactly the targeted kinds and retains the rest.
• L3 invariant over generated output: none of the 29 listed (operationId, target, kind) triples emit a 400 scenario, and the suppression block is honest (every field/name it names still exists in the spec, so a renamed field surfaces as a stale-config failure rather than silently doing nothing).

Happy to implement this once the direction is confirmed and the two upstream camunda/camunda issue numbers exist to cite in the config.