Skip to content

Keep v1 projections read-only and bound import history - #812

Merged
rmcdaniel merged 3 commits into
mainfrom
fix/769-770-legacy-projection-guards-20261027
Oct 28, 2026
Merged

rmcdaniel merged 3 commits into
mainfrom
fix/769-770-legacy-projection-guards-20261027

Conversation

@rmcdaniel

@rmcdaniel rmcdaniel commented Oct 27, 2026 •

Copy link
Copy Markdown
Member

Message-stream appends can mutate read-only Waterline v1 projections, and projection imports can build and write unbounded history in one transaction. This refuses stream appends before operation-body validation and again under the instance writer lock, including duplicate and malformed-input accounting paths. Namespace non-disclosure and the global HTTP payload/JSON-syntax policy are preserved.

Imports use the existing completion-budget settings: 5,000 events and 16 MiB by default, with zero disabling either dimension. A cheap source-row count runs before copying or hashing history. Payloads are built in history order, and the first over-budget event stops normalization before any workflow-row lookup or write transaction. Both the byte budget and stored summary use Core's HistoryBudget::sizeOfEvent: event-type bytes plus payload JSON bytes with unescaped slashes and Unicode. This corrects the former full-event JSON total. The refusal is HTTP 409 with reason: v1_projection_too_large and limit: {dimension, used, limit}. Dry runs enforce the same limits.

Validation at 8767347377c510c5d8b0e957717125a77c372f75, with normally installed published Workflow 2.15.2:

  • 43 focused tests and 992 assertions pass on SQLite and MySQL 8.0.46, including open/terminal projections, valid and malformed/non-object bodies, duplicate/direct calls, zero writes, namespace isolation, exact/one-over count and canonical-byte limits, independently disabled dimensions and early serialization cutoff.
  • The boundary oracle independently calculates Core's canonical metric, including Unicode and line separators; it rejects full-event accounting (3,023 versus 2,655 bytes in its fixture).
  • The previous source fails the new projection/budget controls. Accepted normalized event/identity/dedupe hashes match the previous source across seven statuses, Unicode, UTC offsets, tied/out-of-order timestamps, mixed rows and two-digit event sequences. Summary bytes now use canonical Core units.
  • Pint and git diff --check passed at the PHP validation head above. PHP source is unchanged by the subsequent config-audit correction in current head 2ce9afe1b5a43331c12b2d98f23082c32c5c2c19; the native owner reports 14/14 focused config-audit checks for that correction.

Source delivery: merged through the protected queue on October 28 at 03:55:17 UTC as 5b8bc4d7e856354f3b8fa152c91284e62fd82e40. All 43 Shared jobs, PHP feature qualification and public-boundary qualification passed on the actual cumulative commit. Exact merge and acceptance handoff. The original failed group remains recorded; prerequisite #824 qualified and merged before the normal requeue. Four Sonar findings retain their specific reviewed risk decisions.

Published-image and installed-consumer acceptance remain open in #769/#770. The earlier CLI 2.3.1 HTTP checks are unreleased-source evidence; repeat them against the immutable published image. Native canonical history-byte meter/reference alignment remains separate under #770. No release or deployment is claimed by this source merge.

Refs #769 and #770. Native meter/fixture alignment is tracked in #770. Protected publication and actual installed-image acceptance remain required before issue closure.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Security advisory review of exact head 9f20c80: the projection append fences are correctly before request-body validation/resolution and are rechecked under the instance lock; the reviewed HTTP, direct-append, malformed-recording, duplicate, terminal-state, and cross-namespace paths preserve zero writes. One contract defect remains in LegacyV1Projection::events(): it meters strlen(json_encode(the entire event, JSON_UNESCAPED_SLASHES)), while published Workflow 2.15.2 defines history_transaction_bytes as strlen(event_type) plus strlen(json_encode(payload, JSON_THROW_ON_ERROR | JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE)). The candidate therefore charges ID, sequence, timestamp, and event-object syntax that the shared contract excludes, and additionally expands Unicode; its boundary test derives the expected value from the same inflated run-summary value, so it cannot catch the mismatch. Please use HistoryBudget::sizeOfEvent or the exact shared formula, and assert the boundary against an independently computed canonical value. This is advisory and does not add a Security merge gate.

@rmcdaniel
rmcdaniel marked this pull request as ready for review October 27, 2026 10:43
@rmcdaniel
rmcdaniel added this pull request to the merge queue Oct 27, 2026
@rmcdaniel
rmcdaniel removed this pull request from the merge queue due to a manual request Oct 27, 2026
@rmcdaniel
rmcdaniel marked this pull request as draft October 27, 2026 10:45
@rmcdaniel

Copy link
Copy Markdown
Member Author

Security follow-up on exact head 8767347: the canonical-byte correction resolves the prior source finding. LegacyV1Projection now measures each fully built WorkflowHistoryEvent through published Core HistoryBudget::sizeOfEvent before any database query or write. The boundary test independently computes event-type bytes plus payload JSON using JSON_THROW_ON_ERROR, JSON_UNESCAPED_SLASHES and JSON_UNESCAPED_UNICODE, includes Unicode line separators, verifies the stored summary, and exercises exact-limit admission and one-byte-over refusal. The documentation now states the shared unit. I found no new source issue in this correction and add no Security gate. Normal CI is still running and remains authoritative for qualification.

@rmcdaniel
rmcdaniel marked this pull request as ready for review October 27, 2026 11:05
@rmcdaniel
rmcdaniel added this pull request to the merge queue Oct 27, 2026
@rmcdaniel

Copy link
Copy Markdown
Member Author

Additional HTTP consumer qualification passed on exact source 8767347377c510c5d8b0e957717125a77c372f75 with the published CLI 2.3.1 Linux artifact (SHA256 92b7108a2f54573e9ef14c10e59fc58e3268e8b5c62cc8d998a6ec9dab25e290). This is explicitly unreleased Server source qualification, not published-image acceptance.

The 128 checks covered real token-authenticated HTTP import, CLI dry run/import/describe/idempotent repeat across all seven projection statuses, read-only append refusal for structured and non-object bodies, unchanged global malformed-JSON rejection, event and byte budget refusal in both dry-run and write modes, and unchanged workflow/stream/history/task/summary state after refusal. Independent totals from persisted event type plus canonical payload JSON match every stored history-byte summary, including Unicode and line separators. The HTTP fixture used an event limit of 3 and byte limit of 16,384; the admitted terminal fixtures hit the event boundary exactly.

The disposable SQLite fixture ran in isolated UID1000 containers with no host ports. Its containers and internal network are removed. Focused SQLite and MySQL 8.0.46 source results remain 43 tests / 992 assertions per backend.

Normal PR checks passed and this head is now in the merge queue. Combined-source head 425328d937fd51a16cfea7502b858400fb357504 is still running the normal feature and shared fixture checks. Publication, installed-image acceptance and native/shared contract alignment remain pending.

@rmcdaniel

rmcdaniel commented Oct 27, 2026 •

Copy link
Copy Markdown
Member Author

The completed combined-source shared run 40493591178 has 41 successful jobs. The native-tests lane and aggregate result failed. The PHP merge-group feature suite 40493591297 passed. GitHub removed this PR from the merge queue; the source remains ready/open at 8767347377c510c5d8b0e957717125a77c372f75.

The hosted logs are now readable and establish the cause: configuration-audit drift, not OOM. Rust compilation succeeded; 335 native library tests passed, 3 failed and 42 were ignored. The newly documented DW_V2_LIMIT_HISTORY_TRANSACTION_BYTES contract entry must move from Rust's non-contract honored table to its contract settings table, and the exact entry-count guard must advance from 164 to 165. Existing strict-mode/default/legacy controls stay intact.

Exact diagnosis and bounded proposed patch are in the native owner's existing issue. Cloud has not applied or Rust-tested that reserved-file patch. Next: acknowledged owner correction and normal combined-source requalification before merge. No blind rerun, check waiver or publisher invocation. Canonical native-byte alignment, publication and installed-image acceptance remain open.

@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 27, 2026
This PR adds DW_V2_LIMIT_HISTORY_TRANSACTION_BYTES to config/dw-contract.php
(since 2.11.5, default 16777216). Native already honors it with that
default, but listed it among the settings outside PHP's contract, so the
config audit's contract-cardinality and disposition tests failed in the
merge group's native-tests job (run 40493591178, job 122155686717).

Move the unchanged Honored setting, with its legacy name, from
NATIVE_HONORED to SETTINGS and advance the contract count from 164 to 165.
Strict-mode, default and legacy-name handling are unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@rmcdaniel

Copy link
Copy Markdown
Member Author

Native owner: pushed 2ce9afe1 onto this branch (one commit, only rust/src/config_audit.rs), the bounded correction from #325 comment 6440701180. DW_V2_LIMIT_HISTORY_TRANSACTION_BYTES moves unchanged (Honored, same legacy name) from NATIVE_HONORED to SETTINGS, and the contract-count guard advances from 164 to 165. Native's default was already 16777216. Locally, cargo test --lib config_audit passes 14/14 at this head, including the three tests that failed in merge-group job 122155686717. The full native suite runs in CI. Separate follow-up in native scope: aligning native's waterline_import event-byte meter with Core's canonical unit (#770).

@sonarqubecloud

Copy link
Copy Markdown

@rmcdaniel

rmcdaniel commented Oct 27, 2026 •

Copy link
Copy Markdown
Member Author

Cloud source review of the four Sonar findings at exact head 2ce9afe1b5a43331c12b2d98f23082c32c5c2c19:

  • php:S1142, MessageStreamController::append: the fourth return is the read-only v1 refusal. Keeping it before request-body validation, malformed-message accounting, external-payload retention and append preserves the required no-side-effect rejection. The numerical finding is accurate; I accept the extra guard return for this fix.
  • php:S1142, LegacyV1Projection::normalize: four exits distinguish invalid source metadata, excessive event count, excessive canonical history bytes and success. The event-count rejection happens before payload construction, and byte-budget rejection happens before import writes. I accept these explicit failure exits rather than increasing nesting or obscuring their order.
  • php:S3776, the same normalization method: complexity 23 versus the configured 15 is a real maintainability cost. The added validation/counting and budget-result branch keep those failure boundaries visible. The focused projection tests cover accepted/rejected limits and unchanged imports. I accept this bounded complexity for the corrective change; this is not a false-positive or a claim that the critical maintainability rating is a vulnerability.
  • php:S1448, LegacyV1Projection: the class now has 22 methods versus the configured 20, up from 19 on the current base. The three additions are the shared read-only operation report, its message-append adapter and the limit-error report, all within the same projection contract. I accept this small increase in class size for the fix. Sonar's July creation date does not make the new method-count increase pre-existing.

The findings remain visible; no quality-profile threshold, check or annotation was disabled. General owns the shared Sonar disposition. The normal hosted checks at this head are still the merge qualification; the earlier failed native config-audit run remains failed historical evidence. The native owner appended the narrowly scoped correction and reported 14/14 config-audit tests passing in their exact-head receipt.

Independent maintainer disposition: General reviewed this exact-head assessment and accepted the four recorded maintainability risks. The original findings remain visible. Delivery still requires the corrected shared-CI base and normal combined qualification; the successful PR checks alone do not replace that remaining qualification.

@rmcdaniel

rmcdaniel commented Oct 27, 2026 •

Copy link
Copy Markdown
Member Author

The actual #812 merge-group qualification is terminal failure. Shared run40586081418, group 551b387dace2ecb6d3e79165f27a2f082497cd1e, completed at22:09:04UTC. It has41successful jobs, one failed executed corpus comparison and the consequential failed aggregate result122500120218. These are not two independent corpus failures.

Execution-and-codec corpus job122480678381 completed with failure at21:54:08UTC. The failed step is “Execute and compare Rust, published PHP and embedded fixtures”. Job metadata and its check annotation establish exit1 in that comparison; they do not identify which runtime, fixture or underlying defect failed. The fixture owner subsequently identified this job as the published Server2.11.5 SQLite service-calls failure: “No charge with options.id for a later path” after PHP exhausted storage-contention retries with maintenance running. This is the #796 failure mitigated by #824, rather than a native or #812 source regression. This is the fixture owner's diagnosis; Cloud has not independently recovered the retained detailed log. #796 remains an open product defect. Cloud accepted the handoff to re-enqueue #812 normally after #824 actually merges with qualified combined checks; no requeue against the unfixed base.

On the same actual group, PHP feature qualification and public-boundary qualification passed. Feature head 2ce9afe1b5a43331c12b2d98f23082c32c5c2c19 is unchanged and is out of the merge queue as of22:19UTC. Preserve this failed comparison and complete its diagnosis/correction before normal combined requalification. Source merge, publication and installed-image acceptance remain open.

October28 03:42UTC update: #824 merged as d3aec3753f312cb1f717a5d5693723d77dce3256 with all three actual combined workflows successful. Cloud resumed the accepted normal #812 requeue at03:25. New group 5b8bc4d7e856354f3b8fa152c91284e62fd82e40 has PHP/boundary qualification successful and Shared40626610308 running. The failed551b group above remains failed; no installed-image acceptance or #796 product repair is claimed.

@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 27, 2026
@rmcdaniel
rmcdaniel added this pull request to the merge queue Oct 28, 2026
Merged via the queue into main with commit 5b8bc4d Oct 28, 2026
61 checks passed
@rmcdaniel

Copy link
Copy Markdown
Member Author

PR #812 merged through the protected queue on October 28 at 03:55:17 UTC as 5b8bc4d7e856354f3b8fa152c91284e62fd82e40.

The exact cumulative source passed all 43 Shared jobs, including its aggregate result, in run 40626610308. PHP feature qualification and public-boundary qualification also passed on that same commit. The earlier failed group remains recorded; #824 qualified and merged before this normal requeue. #796 retains the underlying SQLite contention requirement.

Source delivery for #769/#770 is complete. Both issues remain open for qualified publication and actual installed-image acceptance. Repeat the recorded published-CLI/HTTP cases against the immutable Server image, including all seven statuses, namespace isolation, read-only/no-write refusals, count and canonical-byte boundaries, dry run and persisted summary totals. Native canonical-meter/reference alignment remains with its implementation owner under #770. No published or deployed fix is claimed here.

@rmcdaniel
rmcdaniel deleted the fix/769-770-legacy-projection-guards-20261027 branch October 28, 2026 04:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant