[OPIK-7352] [BE] reject non-UUIDv7 referenced ids on ingest#7553
Merged
Conversation
Retention sweeps spans/traces via an id-range predicate that assumes the referenced id is a time-ordered UUIDv7 (SpanDAO orders by the trace_id range; TraceDAO by the id range). Ingest validated an entity's own id but not the ids it references, so a client could store a span with a non-UUIDv7 trace_id and have those rows ordered lexicographically instead of chronologically — deleted or retained against the wrong window. Validate referenced ids on ingest with a not-in-future policy (version 7 required, future-dated rejected, past allowed — referencing older entities such as late spans on old traces is legitimate). Renames validateIdForUpdate -> validateIdNotInFuture and reuses it for both the update paths and referenced ids; adds a public sync overload. Covered paths: span traceId/parentSpanId (create, batch, update, batch-update, and the experiment-items bulk path via batch create); single and batch feedback-score entity ids; trace/span/thread comment entity ids; attachment entity ids; dataset-item trace/span ids (batch, patch, from-traces, from-spans); annotation-queue item ids. Version enforcement is always on (mirrors own-id validation); the not-in-future check is gated by the existing uuidValidation kill-switch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Extends referenced-id validation to config-entity references (verify-then- enforce: confirmed zero non-UUIDv7 ids across prod projects, datasets, dataset_versions, prompts, prompt_versions, alerts, dashboards, webhooks, automation_rules, and guardrails, so enforcement rejects no existing data). Adds null-safe validateIdNotInFutureIfPresent(Async) helpers and validates: projectId across span/trace update + batch-update, feedback scores (single + batch), guardrails, assertion results, annotation queues, experiments, optimizations, alerts, automation-rule evaluators, prompts + prompt versions, dashboards, and thread open/close; feedback/guardrail sourceQueueId; guardrail secondaryId; experiment optimizationId + datasetVersionId; dataset-item datasetId + copy-from ids. Upgrades the guardrail/assertion referenced entityId from version-only to the shared not-in-future policy. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
java
Contributor
⏱️ pre-commit per-hook timing
⏭️ 40 skipped (no matching files changed)
|
Contributor
Backend Tests - Integration Group 6184 tests +9 181 ✅ +9 3m 22s ⏱️ -6s Results for commit 7df5573. ± Comparison against base commit 4dc6b0c. This pull request removes 10 and adds 19 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Contributor
Backend Tests - Integration Group 12297 tests - 4 297 ✅ - 4 6m 2s ⏱️ -1s Results for commit f1e48d3. ± Comparison against base commit 4dc6b0c. This pull request removes 51 and adds 47 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Contributor
Backend Tests - Integration Group 13325 tests - 17 323 ✅ - 17 4m 37s ⏱️ - 1m 1s Results for commit f1e48d3. ± Comparison against base commit 4dc6b0c. This pull request removes 39 and adds 22 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Contributor
Backend Tests - Integration Group 8370 tests +1 368 ✅ - 1 5m 16s ⏱️ - 2m 33s Results for commit f1e48d3. ± Comparison against base commit 4dc6b0c. This pull request removes 70 and adds 71 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Contributor
Backend Tests - Integration Group 16212 tests +13 212 ✅ +13 5m 34s ⏱️ - 1m 51s Results for commit f1e48d3. ± Comparison against base commit 4dc6b0c. This pull request removes 17 and adds 30 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Contributor
Backend Tests - Integration Group 71 584 tests - 7 1 584 ✅ - 7 6m 57s ⏱️ -20s Results for commit f1e48d3. ± Comparison against base commit 4dc6b0c. This pull request removes 19 and adds 12 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Contributor
Contributor
Backend Tests - Integration Group 14650 tests ±0 650 ✅ ±0 7m 5s ⏱️ -10s Results for commit f1e48d3. ± Comparison against base commit 4dc6b0c. This pull request removes 41 and adds 41 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Contributor
Contributor
Backend Tests - Integration Group 11 39 files + 1 39 suites +1 7m 56s ⏱️ -33s Results for commit 7df5573. ± Comparison against base commit 4dc6b0c. This pull request removes 15 and adds 5 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Contributor
Backend Tests - Integration Group 9520 tests +16 519 ✅ +16 10m 25s ⏱️ -29s Results for commit 7df5573. ± Comparison against base commit 4dc6b0c. This pull request removes 17 and adds 33 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
![baz-reviewer[bot]](https://avatars.githubusercontent.com/in/933528?s=60&v=4){kind=small}
Contributor
Backend Tests - Integration Group 41 823 tests +21 1 823 ✅ +21 9m 18s ⏱️ +2s Results for commit f1e48d3. ± Comparison against base commit 4dc6b0c. This pull request removes 49 and adds 70 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Refine scope to the rule: validate a referenced id iff the operation persists it to the entity's table. Drop validation where the id is not written by the operation: - Thread open/close: projectId only resolves/locates an already-existing thread (its project_id was written at trace ingestion), so nothing is persisted here. - Dataset item batch copy-from ids: only a read source for carry-forward rows, never stored as version lineage. All create/update paths that write the referenced id to a column keep their validation (spans trace_id/parent_span_id, feedback/comment/attachment/ guardrail/assertion entity ids, dataset-item trace/span + dataset ids, annotation-queue items, and projectId/optimizationId/datasetVersionId/ sourceQueueId on the rows that store them). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Existing tests assigned random v4 UUIDs to referenced ids that are now validated as UUIDv7 (guardrail secondaryId, experiment optimizationId / datasetVersionId). Switch that test data to generator.generate() so the tests exercise the intended paths (the invalid-version conflict test now uses a v7-but-nonexistent id, preserving its 409 expectation). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…d; fix test data Address reviewer + CI findings: - Remove projectId validation in GuardrailsService and AssertionResultService: both resolve the project by name and overwrite projectId with the resolved project.id() before persistence, so the client-supplied projectId is never stored — validating it only breaks clients that send arbitrary project ids. - Fix remaining test data to use current UUIDv7 (never v4 or future-dated) for validated referenced ids: span traceId, dataset-item trace/span ids, annotation-queue item ids, feedback sourceQueueId, alert projectId, automation-rule projectIds, experiment optimizationId. 404/409 negative tests use a v7-but-nonexistent id to preserve their intent. - Strengthen createWithOldTraceIdSucceeds to round-trip and assert the old traceId is persisted verbatim; add a batch non-v7 parentSpanId rejection test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…checks Addresses reviewer findings: - create(SpanBatch) validated traceId/parentSpanId inside bindSpanToProjectAndId, which runs AFTER deleteAutoStrippedAttachments + project getOrCreate — so a bad batch could delete attachments / create projects before failing 400. Move the id checks up front (before any side effect) so a rejected batch never mutates state. - Extract a shared validateSpanReferences(traceId, parentSpanId) helper used by every span write path (single/batch create, single/batch update) so the reference-id rules can't drift between them. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
andrescrz
previously approved these changes
Jul 22, 2026
andrescrz
left a comment
andrescrz
left a comment
Member
There was a problem hiding this comment.
Some comments, but no blockers.
…er fixes Address reviewer feedback (andrescrz): - Drop UUIDv7 validation on referenced config ids that are already existence-checked (so a bad id fails with the proper 404/409, not 400): projectId on span/trace update, prompt, experiment, optimization, dashboard (all go through validateProjectIdExists / resolveProjectIdOrCreate / get); feedback-batch projectId (overwritten by the name-resolved project); datasetVersionId (FK-checked) and datasetId (findById). - Keep it where the ref is persisted WITHOUT an existence check, so a UUIDv4 orphan can't be ingested: alert projectId, annotation-queue projectId, automation-rule projectIds, experiment optimizationId, feedback sourceQueueId. - Move validateIdNotInFutureIfPresent(Async) bodies out of the IdGenerator interface into IdGeneratorImpl. - Tests: route added generators through TestIdGeneratorFactory (static final) instead of local TimeBasedEpochGenerator fields; parameterize the span invalid-reference tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
andrescrz
approved these changes
Jul 22, 2026
thiagohora
added a commit
that referenced
this pull request
Jul 22, 2026
…d validation Resolve the semantic overlap after merging main (#7553): unify IdGenerator on the merged API — implement validateIdNotInFuture / validateIdNotInFutureAsync (and the IfPresent variants) and drop the superseded validateIdForUpdate*. Own-id validateId stays workspace-tagged for the audit metric; referenced-id validateIdNotInFuture falls back to UNKNOWN on the sync path and resolves the workspace from the reactive context on the async path. Keep #7553's fail-fast (validate before side effects) for span batches, now run inside deferContextual so the batch's own ids attribute to the request workspace; bindSpanToProjectAndId no longer re-validates. Trace batches keep validating in bindTraceToProjectAndId with the workspace threaded through. Update the async test to the renamed methods. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
thiagohora
added a commit
that referenced
this pull request
Jul 22, 2026
bindTraceToProjectAndId validated own ids only after deleteAutoStrippedAttachments and project getOrCreate, so a bad trace id in a batch still mutated state before failing (unlike spans, which #7553 validates fail-fast). Hoist trace own-id validation into a leading deferContextual that runs before any side effect and attributes the audit metric to the request workspace, and stop re-validating in bindTraceToProjectAndId — mirroring create(SpanBatch). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
2 participants
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.
Details
Retention sweeps spans/traces via an id-range predicate that assumes the referenced id is a time-ordered UUIDv7 (
SpanDAOorders spans by thetrace_idrange;TraceDAOby theidrange). Ingest validated an entity's own id but not the ids it references, so a client could store e.g. a span with a non-UUIDv7trace_idand have those rows ordered lexicographically instead of chronologically — deleted or retained against the wrong window. This validates referenced ids on ingest.projectIdon thread open/close, whoseproject_idwas written at trace ingestion) or as a read source (dataset copy-from ids) are not validated.validateIdForUpdate→validateIdNotInFutureand reuses it for both the update paths and referenced ids; adds null-safevalidateIdNotInFutureIfPresent(Async)helpers.traceId/parentSpanId(create, batch, update, batch-update, experiment-items bulk); feedback-score entity ids (single + batch) +sourceQueueId; trace/span/thread comment entity ids; attachment entity ids; guardrailentityId+secondaryId; assertionentityId; dataset-itemtraceId/spanId+ batchdatasetId; annotation-queue item ids; experimentprojectId/optimizationId/datasetVersionId; andprojectIdon span/trace update + batch-update, optimization, alert, dashboard, prompt (+ version), annotation-queue, guardrail, assertion, feedback batch, automation-rule evaluators.uuidValidationkill-switch (off in prod), so this ships dormant beyond the version check.Change checklist
Issues
AI-WATERMARK
AI-WATERMARK: yes
Testing
mvn -o compile/test-compile/spotless:applyclean.SpansResourceTestcases (single and batch create): reject non-v7traceId(400), reject future-datedtraceId(400), reject non-v7parentSpanId(400), and accept an old (past) v7traceId(spans on old traces are valid). These exercise the sharedvalidateIdNotInFuturemechanism used by every path here.Documentation
No user-facing docs. The referenced-id validation policy (version 7 + not-in-future, past allowed) is documented in the
IdGeneratorjavadoc.