chore(ers): failing tests for environment-first strategy ordering by pflynn-virtru · Pull Request #3980 · opentdf/platform

pflynn-virtru · 2026-09-04T15:36:35Z

These tests fail on purpose. Tests only, no behavior change — the point is to demonstrate the bug.

The failures

--- FAIL: TestResolveEntitiesFromTokenResolvesSubjectWhenEnvironmentStrategyIsFirst
    Error: Received unexpected error:
           no subject entities to resolve - all were environment entities and skipped
    Messages: a token matching a subject strategy must resolve regardless of strategy order

--- FAIL: TestShippedERSConfigResolvesASubjectForKeycloakToken
    Messages: winning strategy "client_environment_sql" resolves an environment entity, so a
              Keycloak token produces a chain with no subject; matched order was
              [client_environment_sql=environment user_subject_sql=subject
               client_environment_ldap=environment user_subject_ldap=subject
               jwt_claims_fallback=subject]

Why

First-match-wins chain building (multi-strategy/v2/registration.go:300) stops at the first matching strategy. When that strategy is entity_type: environment, the chain holds only an ENVIRONMENT entity.

GetDecision resolves tokens with skipEnvironmentEntities = true (just_in_time_pdp.go:160), so filterEntityChain empties the chain and the request fails. The error is not errResolvedTokenChainRequiresHydration, so the fallback at just_in_time_pdp.go:461 never fires — the decision request just errors.

This is reachable with a config already in-tree. opentdf-ers-test.yaml, which README.md:84 tells operators to start the platform with, runs failure_strategy: continue and lists client_environment_sql (entity_type: environment, condition azp exists) ahead of every subject strategy. Every Keycloak token carries azp.

Nothing guards the ordering: entity_type has no config validation, SelectStrategies preserves configuration order, and EntityChain in entity.proto has no category or cardinality constraints. No BDD feature uses entity_type: environment — every ERS feature is subject-only, which is why the suite is green on #3964.

Tests

service/internal/access/v2/just_in_time_pdp_environment_chain_test.go — runs the real ERS v2 handler through the PDP via a thin sdkconnect adapter.

Test	Status
`...ResolvesSubjectWhenEnvironmentStrategyIsFirst`	RED — the bug
`...ResolvesSubjectWhenSubjectStrategyIsFirst`	green — control, isolates ordering as the variable
`...EnvironmentFirstChainIsWellFormedButEmptyAfterFiltering`	green — pins the mechanism

service/entityresolution/multi-strategy/shipped_config_test.go — parses opentdf-ers-test.yaml and runs the real matcher.

Test	Status
`TestShippedERSConfigResolvesASubjectForKeycloakToken`	RED — the shipped config hits it
`TestShippedERSConfigOrderingIsUnvalidated`	green — nothing rejects or normalizes the order

Both RED tests assert the required outcome, not a mechanism, so any plausible fix satisfies them: reorder the YAML, skip environment-typed strategies when picking the chain winner, keep the environment entity but continue to the first subject match, or reject the ordering at config load.

Note on the ADR

The ADR does not cover this. Its first-match-wins language (lines 406, 425-427) is scoped to single-entity ResolveEntity. § Future Considerations → Entity Chains Handling (line 775) states chains are not yet handled and lists "how failure strategies apply across entity chains" as open. Deciding chain semantics for environment-typed strategies likely warrants an ADR amendment.

🤖 Generated with Claude Code

First-match-wins chain building stops at the first matching strategy. When that strategy is entity_type: environment, the chain holds only an ENVIRONMENT entity. GetDecision resolves tokens with skipEnvironmentEntities=true, so the chain filters to empty and the request fails with "no subject entities to resolve"; the error is not errResolvedTokenChainRequiresHydration, so the hydration fallback does not fire. Adds tests pinning that behavior end to end through the real ERS v2 handler, plus a control proving the same token resolves when a subject strategy is first. Also asserts that opentdf-ers-test.yaml -- the config the README tells operators to start the platform with -- selects an environment strategy first for a Keycloak-shaped token, since its client_environment_sql strategy is conditioned on "azp exists" and listed ahead of every subject strategy. Tests only; no behavior change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Paul Flynn <pflynn-virtru@users.noreply.github.com>

coderabbitai · 2026-09-04T15:36:46Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Team

Run ID: da59c943-0460-430b-ade3-af39572d07f3

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

github-actions · 2026-09-04T15:42:45Z

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric	Value
Approved Decision Requests	1000
Denied Decision Requests	0
Total Time	237.253968ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric	Value
Approved Decision Requests	1000
Denied Decision Requests	0
Total Time	129.166929ms

Benchmark Statistics

Name	№ Requests	Avg Duration	Min Duration	Max Duration

Bulk Benchmark Results

Metric	Value
Total Decrypts	100
Successful Decrypts	100
Failed Decrypts	0
Total Time	429.452313ms
Throughput	232.85 requests/second

TDF3 Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	5000
Failed Requests	0
Concurrent Requests	50
Total Time	45.99685105s
Average Latency	459.013237ms
Throughput	108.70 requests/second

github-actions · 2026-09-04T15:52:44Z

Flips the two characterization tests to assert the outcome that should hold, so they fail and demonstrate the bug rather than pinning it. RED: - resolveEntitiesFromToken must resolve alice's subject entity when an entity_type: environment strategy is configured ahead of the subject strategy. Fails with "no subject entities to resolve - all were environment entities and skipped". - opentdf-ers-test.yaml must select a subject-resolving strategy for a Keycloak token. Fails: client_environment_sql (environment, "azp exists") wins ahead of user_subject_sql. Both assert the required outcome rather than a mechanism, so reordering the YAML, skipping environment-typed strategies when picking the chain winner, or rejecting the ordering at config load all satisfy them. Controls stay green: the same strategies in subject-first order resolve, and the environment-first chain is well-formed until the decision flow filters it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Paul Flynn <pflynn-virtru@users.noreply.github.com>

github-actions · 2026-09-04T16:04:57Z

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric	Value
Approved Decision Requests	1000
Denied Decision Requests	0
Total Time	127.845158ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric	Value
Approved Decision Requests	1000
Denied Decision Requests	0
Total Time	72.41373ms

Benchmark Statistics

Name	№ Requests	Avg Duration	Min Duration	Max Duration

Bulk Benchmark Results

Metric	Value
Total Decrypts	100
Successful Decrypts	100
Failed Decrypts	0
Total Time	270.312908ms
Throughput	369.94 requests/second

TDF3 Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	5000
Failed Requests	0
Concurrent Requests	50
Total Time	29.3752967s
Average Latency	293.213782ms
Throughput	170.21 requests/second

Replaces the hand-written realERSV2Client adapter, which was "real" only in that it delegated to the ERS implementation; the client half was still test scaffolding standing in for sdkconnect. The harness now mounts the generated entityresolutionv2connect.NewEntityResolutionServiceHandler over the real ERSV2 on an httptest server, and the PDP reaches it through sdkconnect.NewEntityResolutionServiceClientV2ConnectWrapper -- the same client sdk.New builds. Proto marshalling, the Connect codec, and the production client wrapper are all exercised; only the strategy configuration is test-supplied. Call-count assertions move to a connect.Interceptor keyed on the generated procedure constants, so they observe real RPCs instead of adapter bookkeeping. Same RED, same failure reason. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Paul Flynn <pflynn-virtru@users.noreply.github.com>

github-actions · 2026-09-04T16:13:36Z

github-actions · 2026-09-04T16:17:29Z

⚠️ Govulncheck found vulnerabilities ⚠️

The following modules have known vulnerabilities:

examples
otdfctl
sdk
service
lib/ocrypto
lib/fixtures
tests-bdd

See the workflow run for details.

github-actions · 2026-09-04T16:17:34Z

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric	Value
Approved Decision Requests	1000
Denied Decision Requests	0
Total Time	210.883449ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric	Value
Approved Decision Requests	1000
Denied Decision Requests	0
Total Time	116.852562ms

Benchmark Statistics

Name	№ Requests	Avg Duration	Min Duration	Max Duration

Bulk Benchmark Results

Metric	Value
Total Decrypts	100
Successful Decrypts	100
Failed Decrypts	0
Total Time	436.95593ms
Throughput	228.86 requests/second

TDF3 Benchmark Results:

Metric	Value
Total Requests	5000
Successful Requests	5000
Failed Requests	0
Concurrent Requests	50
Total Time	44.095769723s
Average Latency	440.063912ms
Throughput	113.39 requests/second

github-actions · 2026-09-04T16:27:40Z

pflynn-virtru requested a review from a team as a code owner September 4, 2026 15:36

github-actions Bot added the size/xs label Sep 4, 2026

pflynn-virtru changed the title ~~test(ers): cover environment-first strategy ordering in token chains~~ chore(ers): cover environment-first strategy ordering in token chains Sep 4, 2026

pflynn-virtru changed the title ~~chore(ers): cover environment-first strategy ordering in token chains~~ test(ers): failing tests for environment-first strategy ordering Sep 4, 2026

pflynn-virtru changed the title ~~test(ers): failing tests for environment-first strategy ordering~~ chore(ers): failing tests for environment-first strategy ordering Sep 4, 2026

pflynn-virtru mentioned this pull request Sep 4, 2026

fix(ers): continue failure_strategy bug - first-match-wins per ADR #3964

Open

3 tasks

dmihalcik-virtru mentioned this pull request Sep 4, 2026

fix(sdk): DSPX-4590 default per-segment sizes when a writer omits them #3979

Open

Uh oh!

Conversation

pflynn-virtru commented Sep 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

The failures

Why

Tests

Note on the ADR

Uh oh!

coderabbitai Bot commented Sep 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

github-actions Bot commented Sep 4, 2026

Benchmark authorization.GetDecisions Results:

Benchmark authorization.v2.GetMultiResourceDecision Results:

Benchmark Statistics

Bulk Benchmark Results

TDF3 Benchmark Results:

Uh oh!

github-actions Bot commented Sep 4, 2026

X-Test Results

Uh oh!

github-actions Bot commented Sep 4, 2026

Benchmark authorization.GetDecisions Results:

Benchmark authorization.v2.GetMultiResourceDecision Results:

Benchmark Statistics

Bulk Benchmark Results

TDF3 Benchmark Results:

Uh oh!

github-actions Bot commented Sep 4, 2026

X-Test Failure Report

Uh oh!

github-actions Bot commented Sep 4, 2026

⚠️ Govulncheck found vulnerabilities ⚠️

Uh oh!

github-actions Bot commented Sep 4, 2026

Benchmark authorization.GetDecisions Results:

Benchmark authorization.v2.GetMultiResourceDecision Results:

Benchmark Statistics

Bulk Benchmark Results

TDF3 Benchmark Results:

Uh oh!

github-actions Bot commented Sep 4, 2026

X-Test Results

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

pflynn-virtru commented Sep 4, 2026 •

edited

Loading

coderabbitai Bot commented Sep 4, 2026 •

edited

Loading