test(concurrent-keys): integration coverage for view-scoped key rejection - #796
Merged
aaron-zeisler merged 5 commits intoAug 12, 2026
Conversation
aaron-zeisler
force-pushed
the
aaronz/SDK-2860/reject-view-scoped-keys
branch
from
August 5, 2026 20:40
c6bcde9 to
677f834
Compare
aaron-zeisler
force-pushed
the
aaronz/SDK-2860/view-scoped-keys-integration-tests
branch
5 times, most recently
from
August 6, 2026 17:55
9479d41 to
bd308e1
Compare
…ial wire format Adds hasViews to each sdkKeys[]/mobileKeys[] entry and carries it onto the accepted-key params. Nothing reads it yet — the filter that consumes it follows in the next commit. A plain bool rather than a pointer: an absent field means "not view-scoped" and there is no third state, unlike expiry where nil genuinely means permanent. Old-format payloads synthesize their accepted keys from the singular sdkKey/mobKey and so never set it, which is correct — the backend forbids views on a default key.
The sdkKeys[]/mobileKeys[] arrays hand relay every non-default credential for an environment, including keys scoped to a view. A view-scoped key is only entitled to a subset of the environment's flags, but relay serves the whole environment payload and has no view support — so accepting one would silently over-deliver every flag to an SDK that must see only a subset. Filter them in BuildAcceptedSet, the single funnel for both the auto-config stream and the offline archive. They never enter the accepted set, so an SDK presenting one is rejected because the credential is absent from the lookup map, and no state for it reaches /status, event forwarding, or the expiry ticker. BuildAcceptedSet returns the identifiers it dropped so the handlers that apply a set can WARN; it takes no logger because the stream parse boundary validates every payload and would double-log each one. Rejection is keyed by credential value rather than evaluated per entry. The accepted-set builder is first-wins, so if two entries carry the same value and only one is marked, per-entry filtering would admit the credential via the unmarked entry while still reporting it rejected. Tainting the value makes one marked entry sufficient whichever position it holds. Excluding the designated key from the taint set is also what keeps the anchor guard structural: the marker is disregarded on the anchor and the primary mobile key, because dropping a designated key would take the whole environment down and the backend forbids views on a default key in the first place. Acquiring a view mid-session needed no new code: reconcileAcceptedKeys already revokes any key absent from the desired set immediately rather than on an expiry timestamp, and RemoveConnectionMapping unmaps before the streams are torn down so a reconnect is rejected.
aaron-zeisler
force-pushed
the
aaronz/SDK-2860/reject-view-scoped-keys
branch
from
August 6, 2026 20:12
706ca7a to
ffc86d5
Compare
aaron-zeisler
force-pushed
the
aaronz/SDK-2860/view-scoped-keys-integration-tests
branch
from
August 6, 2026 20:15
bd308e1 to
279df24
Compare
aaron-zeisler
marked this pull request as ready for review
August 10, 2026 18:49
aaron-zeisler
force-pushed
the
aaronz/SDK-2860/reject-view-scoped-keys
branch
from
August 11, 2026 18:32
ffc86d5 to
69bd4de
Compare
aaron-zeisler
force-pushed
the
aaronz/SDK-2860/view-scoped-keys-integration-tests
branch
from
August 11, 2026 18:34
279df24 to
76b7dad
Compare
aaron-zeisler
force-pushed
the
aaronz/SDK-2860/reject-view-scoped-keys
branch
from
August 11, 2026 18:36
69bd4de to
f4d56e7
Compare
aaron-zeisler
force-pushed
the
aaronz/SDK-2860/view-scoped-keys-integration-tests
branch
from
August 11, 2026 18:47
76b7dad to
f268417
Compare
keelerm84
approved these changes
Aug 12, 2026
Judge each entry on its own marker instead of collecting the set of view-scoped credential values in a preliminary pass. This drops a helper, a map, and an iteration over both key arrays. The pre-pass existed so that one marked entry would reject a credential value even if another entry carried the same value unmarked — the accepted-set builder is first-wins, so per-entry filtering would otherwise admit such a value through the unmarked entry while still reporting it as dropped. That shape is not reachable: the backend enforces one row per credential value with a unique index, and the offline archive is generated from the same data. Also drops the placeholder for an empty key identifier. Only entries from the sdkKeys[]/mobileKeys[] arrays can be marked view-scoped, and the backend requires a non-empty identifier on each of those, so there was nothing for it to substitute for.
…ormed-payload error AcceptedSetBuilder.Build returned a bare sentinel when no SDK key survived, while every other rejection was a *MalformedCredentialSetError. That framed an empty set as a caller mistake, which it no longer is: the payload reaches it by combining an undefined anchor with an sdkKeys[] array that is either empty or made up entirely of keys relay excludes — the view-scoped case being new. Return the same type, with a message describing the payload rather than the builder, since that is what an operator reading the log can act on. With the type now uniform, the offline handlers no longer branch on it. Both branches already preserved previous credentials and differed only in message text, and the fallback text was the wrong one: it claimed the environment was auto-configured, and in UpdateEnvironment that the environment failed to initialize when it had already been running. Log the accurate message unconditionally instead. Stop documenting the error type as a contract. Nothing in production inspects it now, and BuildAcceptedSet cannot honor such a promise on its own — the guarantee would depend on a function in another package whose author has no reason to know about it. The taxonomy stays pinned by tests, which is where a claim like this can actually be enforced; TestBuildAcceptedSet_NoSDKKeys now covers both shapes that reach the empty-set path, as it is the only test that would catch this classification regressing.
aaron-zeisler
force-pushed
the
aaronz/SDK-2860/reject-view-scoped-keys
branch
from
August 12, 2026 16:46
f4d56e7 to
cef8393
Compare
…tion Exercises the ingestion filter through both real harnesses — the auto-config stream and the offline archive — rather than at the BuildAcceptedSet unit boundary, so the assertions run against actual downstream authentication and live SDK connections. Covers the cases the unit tests cannot reach: that a rejected key really does fail auth on every SDK endpoint while its siblings keep working, that it is absent from the /status arrays, that a key which gains a view mid-session has its live stream torn down and cannot reconnect while other keys' connections stay up, that losing the view again re-admits it, and that a malformed payload carrying view-scoped keys logs the malformed error without claiming to have rejected anything. Also pins that the ingestion WARN fires exactly once per payload, which is what makes the decision to keep the stream parse boundary silent regress loudly rather than silently doubling every message.
aaron-zeisler
force-pushed
the
aaronz/SDK-2860/view-scoped-keys-integration-tests
branch
from
August 12, 2026 16:47
f268417 to
9f89000
Compare
Base automatically changed from
aaronz/SDK-2860/reject-view-scoped-keys
to
feat/concurrent-keys
August 12, 2026 17:04
aaron-zeisler
deleted the
aaronz/SDK-2860/view-scoped-keys-integration-tests
branch
August 12, 2026 17:05
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.
Summary
Integration coverage for the view-scoped key rejection added in #795. Stacked on that branch — review #795 first, and this will retarget to
feat/concurrent-keysautomatically once it merges.Jira: SDK-2860
Background
#795 proves the filter at the
BuildAcceptedSetunit boundary: given params, the right keys land in the accepted set and the right identifiers are reported. That leaves the questions a unit test structurally cannot answer — whether a rejected credential actually fails downstream authentication, whether it really carries no state anywhere in the environment, and whether a live SDK connection on a key that gains a view is genuinely torn down.These run through the two real harnesses in the existing
concurrent_keys_*_test.gofamily (autoConfTestfor the auto-config stream,offlineModeTestfor the archive), so the assertions hit actual HTTP endpoints, a real/statusresponse, and live event streams.Changes
Seven tests, mirroring the four cases named in the ticket:
/statussdkKeys[]/mobileKeys[]arrays (asserted by exact count, so a key surfacing under an unexpected obscured value is still caught).The ingestion WARN is also asserted to fire exactly once per payload. That is what makes the decision to keep
StreamManager.validateCredentialPayloadsilent regress loudly — it runs on every environment of every payload, so logging from there would double every message.Every test was mutation-checked: disabling the non-anchor filter fails four of them, disabling the designated-key branch fails the other two, and the malformed-payload case fails only when both of its independent defenses are broken.
Note
Overview
Adds
relay/concurrent_keys_views_test.go— integration tests for ingestion-time rejection of view-scoped credentials (behavior from #795), using the existingautoConfTestandofflineModeTestharnesses.Rejection on ingest (RAC and offline): Non-anchor SDK/mobile keys marked with views fail SDK endpoints, stay out of the accepted set and
/statuskey arrays (with exact counts), while anchor and other siblings keep working. Asserts the ingestion WARN names wire identifiers and fires once per payload.Malformed payloads: When the archive is structurally invalid, logs the malformed error and does not emit the view-scoped rejection WARN.
Mid-session view acquisition: Offline tests close live streams when a connected non-anchor key gains a view, revoke auth, then re-admit the key when the view is removed (SDK
/alland mobile/mevalsubtests). RAC test patches keys to view-scoped while an anchor stream is open, expects routing revocation and reconnect rejection without disturbing the anchor connection.Reviewed by Cursor Bugbot for commit 9f89000. Bugbot is set up for automated code reviews on this repo. Configure here.