Security/version compatibility filter is not applied uniformly across discovery paths — Closes #293#304
Draft
conradbzura wants to merge 4 commits into
Conversation
The secure-match and same-major version floor were enforced only on the pool-URI and static-workers construction paths; user-supplied discovery subscribers and every WorkerPool subscription admitted workers unfiltered, deferring failures to connect/handshake with transient-error and eviction noise. Move enforcement into the worker sentinel as a single admission rule: worker-added and worker-updated events must satisfy the combined security/version predicate regardless of construction path, checked before the lease so a rejected worker never consumes capacity; worker-dropped events stay ungated. Construction-path subscription filters retain only their tag semantics, and the static-workers arm keeps its fail-fast quorum-versus-compatible-count validation. The predicate is deliberately not serialized: a restored proxy rebuilds its gate from its own credential context and protocol version. This is a behavior change for callers relying on unfiltered custom discovery: incompatible workers are no longer admitted, and quorum waits no longer count them.
Adapt existing pins to the gate: mock worker factories and discovery fixtures advertise the ambient protocol version, sentinel-feeding tests pin the local version explicitly, and the pool-URI subscription filter test now asserts tag-only semantics with compatibility deferred to admission. Add gate coverage on the user-supplied discovery path (version, security, unparseable-version, and lease interplay; ignored incompatible updates; ungated drops), construction-path boundaries for the static-workers quorum count, quorum-timeout and retry behavior on all-incompatible streams, credential resolution captured at construction, the fail-closed unparseable local version, restored proxies re-gating carried events under the restoring context, and Hypothesis properties for the version predicates plus a fold-model oracle over arbitrary event streams. Remove test_property_workers_list_accurate: it asserted nothing and spent ten seconds of wall clock; the fold-model property supersedes it.
Cover the durable arm with a real proxy: an all-incompatible custom discovery source times out the eager quorum wait, and a mixed stream admits only the compatible worker without the rejected one consuming the single lease slot. Pin the tag-only subscription predicate on the spawn arms, including the match-all no-tags branch, and the credentials forwarding that aims the security half of the gate.
Standalone scenarios rather than pairwise dimensions: a mismatched security posture would break the covering array's dispatch-success oracle, and the admitted-set oracle needs the active proxy, which the scenario builder does not surface. Publish fabricated incompatible metadata through the real local discovery pipeline alongside real workers and assert the admitted set is exactly the compatible workers before and after dispatching, across the durable, hybrid, and static-workers paths and both security-posture crosses.
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
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
Every discovery source now passes one admission rule: the worker sentinel applies the combined security/version predicate to every
worker-addedandworker-updatedevent, regardless of how the proxy was constructed. Previously the predicate lived in the construction paths — the pool-URI arm baked it into its subscription filter and the static-workers arm filtered at construction — so user-supplied discovery subscribers and everyWorkerPoolsubscription admitted workers unfiltered, deferring the failure to connect/handshake where it surfaced as transient-error and eviction noise.This is a behavior change for callers relying on unfiltered custom discovery: an incompatible worker is never admitted, and quorum waits no longer count one. A proxy restored from a pickle re-derives the predicate from the restoring process's credential context and protocol version — the gate is deliberately not serialized, so admission always reflects the environment doing the admitting.
Closes #293
Proposed changes
Enforce the admission gate in the worker sentinel
Gate
worker-addedandworker-updatedevents on the combined security/version predicate before the lease check, so a rejected worker never consumes lease capacity and never wakes the quorum wait. Leaveworker-droppedungated — a drop is not an admission, and removing an unknown worker is a no-op. Reduce the pool-URI subscription filter to tag semantics only; the static-workers arm keeps its construction-time filter for the fail-fast quorum-versus-compatible-count validation, and the sentinel re-checks those events idempotently on restore. Document the behavior change on thediscoveryparameters ofWorkerProxyandWorkerPool.Adapt the worker test suite to the gate
Advertise the ambient protocol version from the mock worker factories and discovery fixtures so test doubles behave like real in-process workers, pin the local version explicitly in tests that feed the sentinel, and rewrite the pool-URI subscription-filter test to assert tag-only semantics. Remove
test_property_workers_list_accurate: it asserted nothing and spent ten seconds of wall clock, and the fold-model property below supersedes it.Pin the gate at every scope
Cover the gate at proxy scope (per-rule admission tests, quorum and lease interplay, serialization re-gating, Hypothesis properties over the version predicates and arbitrary event streams), at pool scope (durable-arm composition with a real proxy, tag-only subscription predicates, credentials forwarding), and end to end (real workers plus fabricated incompatible metadata published through the real local-discovery pipeline, and both security-posture crosses). The integration tests are standalone scenarios rather than pairwise dimensions: a mismatched security posture breaks the covering array's dispatch-success oracle, and the admitted-set oracle needs the active proxy, which the scenario builder does not surface.
Test cases
TestWorkerProxyTestWorkerProxyTestWorkerProxyTestWorkerProxyTestWorkerProxylease=1and an incompatible worker preceding a compatible oneTestWorkerProxyTestWorkerProxyTestWorkerProxyTestWorkerProxyTestWorkerProxyquorum=1, a short timeout, and an all-incompatible streamasyncio.TimeoutErrorpropagates and nothing is admittedTestWorkerProxyTestWorkerProxyTestWorkerProxyTestWorkerProxyquorum=2and a stepped add/drop/add/add streamTestWorkerProxyTestWorkerProxyquorum=2TestWorkerProxyquorum=2TestWorkerProxyquorum=NoneTestWorkerProxyTestWorkerProxyTestWorkerProxyTestWorkerProxyTestWorkerProxyparse_versionis calledTestWorkerProxyTestWorkerProxyTestWorkerPoolasyncio.TimeoutErrorraises at entryTestWorkerPoollease=1over an incompatible-then-compatible streamTestWorkerPoolTestWorkerPoolTestWorkerPoolTestDiscoveryAdmissionGateTestSecurityPostureAdmissionTestSecurityPostureAdmissionTestHybridAdmissionGateTestStaticWorkersAdmissionhttps://claude.ai/code/session_01Da7KQMtQgbgwqvPeL4Wfu3