Repository navigation
fix: review findings on #2631 (approval mirror forgery, trace id overwrite, export hardening) - #2635
Merged
Merged
Conversation
Any signed-in user can create an OpenRegister task with Integriq's appId and an approvalRequestId in its metadata, then complete it as skipped, failed or dead_letter to expire or dead-letter that approval. The listener now requires the task uuid to equal the record's taskUuid, and takes the timer branch only for an outcome the timer recorded (no completer, or a flow-timer: actor) after the record's expiresAt. Every outcome OpenRegister counts as a rejection (returned, declined, denied as well as rejected) now rejects the record, and so does a rejection OpenRegister rerouted to dead_letter for a mirror with onReject: dead_letter, behind the same two authorization layers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The local sweep skipped every mirrored record with a shared onTimeout, leaving its expiry to OpenRegister's timer. A mirror the requester or an admin cancelled, or one terminated with its run, never reaches that timer, so the record stayed pending and, once expired, could be neither decided nor swept. The sweep now leaves a record to the timer only while its mirror is still open; an ended or missing mirror is expired locally, and an unreadable one is left for the next sweep. The OpenRegister test stubs gain TaskService::get() and the Task states, copied from the real classes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… record's id On the public endpoint route the caller's W3C trace id became the execution_trace uuid, and persisting is an upsert by that uuid, so anyone who knew a trace id (every outbound partner receives one) could overwrite that execution's trace across tenants, and two executions under one upstream trace collapsed into one record. The record now always gets a fresh uuid. The caller's trace id is kept apart in a new otelTraceId field (execution_trace schema 1.2.0), which the OTLP export and outbound traceparent headers use, and which survives an approval suspension together with the parent span id. REQ-OTEL-004 is reworded to match, with a scenario for two requests under one traceparent. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… url.full
Only the query string and fragment were cut from url.full, so a BSN in a
path such as /ingeschrevenpersonen/999993653 and user:pass@ credentials
reached the external collector, against REQ-OTEL-003. url.full now keeps
scheme, host, port and path only, drops the userinfo, and replaces every
path segment holding four or more consecutive digits, or a long token
with a digit (a uuid, a hash), with {id}.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Each send could block cron for 10 s and a failure was re-queued at once, up to three times, and failed traces are always sampled, so a partner outage during a collector outage meant N x 40 s of serial cron work. - a circuit breaker: after five failed sends in a row, sends pause for five minutes and no new trace is queued meanwhile; - retries wait 5, 10 and 20 minutes instead of re-running at once; - the collector timeout drops from 10 s to 3 s. REQ-OTEL-002 now states the delay and the pause, with a scenario. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…hpmd limits The export circuit breaker moves out of OtelSettings into its own OtelExportBreaker, so OtelSettings stays under ten public methods, and OtelExportJob::run() is split into argument, send and retry helpers. The shared approval listener's mirror check, timer expiry and outcome mapping become helpers, and it reads the expiry with strtotime() instead of DateTime, which brings its complexity and coupling back under the thresholds. No behaviour change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
WilcoLouwerse
requested review from
Rem-Dam,
SudoThijn,
bbrands02,
remko48,
rjzondervan and
rubenvdlinde
as code owners
October 9, 2026 09:24
Contributor
Quality Report — ConductionNL/integriq @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| build | ✅ | ||||
| check-specs | ✅ | ||||
| test-l10n | ✅ | ||||
| format | ✅ | ||||
| check-schema-l10n | ✅ | ||||
| check-l10n-js | ✅ | ||||
| composer | ✅ | ✅ 143/143 | |||
| npm | ✅ | ✅ 647/647 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ❌ | ||||
| lockfile sync | ✅ | ||||
| PHPUnit | ✅ 5307/5307 · 1 skipped | ||||
| Newman | ✅ 130/130 | ||||
| Playwright | ⏭️ deferred: E2E runs locally and on the promotion path only. This pull request targets development, so the suite is asked once per promotion into beta and main rather than once per push per open pull request. Run it locally with npx playwright test, or from the Actions tab on a branch with no open pull request into development. |
||||
| Hydra gates | ✅ | ||||
| Hydra gates (axe) | ✅ |
Quality workflow — 2026-10-09 09:30 UTC
Download the full PDF report from the workflow artifacts.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the findings from @rjzondervan's review of the beta promotion #2154 (verdict), which covered #2631 (approvals in the shared task inbox, OpenTelemetry export, CTI call log). Merging this into
developmentupdates #2154, which followsdevelopment.task uuid === taskUuid). The timer branch requires a timer-recorded outcome (no completer or aflow-timer:actor) after the record'sexpiresAt.traceparentsets the trace record's uuidotelTraceIdfield (schema 1.2.0), used by the export and by outboundtraceparentheaders, and kept across an approval suspension.onReject: dead_letterand other rejecting outcomesrejected,returned,declined,denied). Adead_letterwith a real completer is a rerouted rejection and goes throughreject()with both authorization layers.url.fullurl.fullkeeps scheme, host, port and path, dropsuser:pass@, and masks identifier-like path segments as{id}.callIde140a3c is a behaviour-neutral refactor that keeps the new code within the phpmd thresholds (the breaker becomes its own
OtelExportBreakerclass).REQ-OTEL-002, -003 and -004 in
openspec/changes/observability-opentelemetry-exportare reworded to the new behaviour, each with a scenario.Test plan
traceparent;tests/Unit/AppInfoerrors, which show up identically on a cleandevelopmentcheckout in this environment.lib/file; schema-l10n ratchet 0 uncovered;check:l10n-jsup to date.developmentcheckout too (ConstantTypeResolver), so it is left to CI.🤖 Generated with Claude Code