fix(validation): rewrite P-016 as a per-field sinkCredential writeOnly check#383
Merged
hdamker merged 1 commit intoJul 21, 2026
Merged
Conversation
…y check r4.3 replaced the blanket response ban with a partial-disclosure model (Commonalities#633); detection moves from response-schema traversal to a schema-definition scan for SinkCredential subtypes, since secrets live on the discriminator subtypes, not the polymorphic base. Lifts the mute from tooling#297 with a graded warn/error ramp gated to commonalities_release >=r4.3.
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.
What type of PR is this?
correction
What this PR does / why we need it:
Commonalities r4.3 (camaraproject/Commonalities#633, guide-aligned by #656) replaced the blanket "sinkCredential MUST NOT appear in responses" ban with a partial-disclosure model:
sinkCredentialMAY appear in POST/GET responses, but its secret sub-fields —accessToken/accessTokenType(AccessTokenCredential),clientId/tokenUri(PrivateKeyJWTCredential) — MUST bewriteOnly: true. P-016 (check-sinkcredential-not-in-response) was muted pending this design decision.Rewrites the check as
check-sinkcredential-secrets-writeonly: a schema-definition scan forSinkCredentialdiscriminator subtypes (allOfreferencingSinkCredential) rather than a response-schema traversal —sinkCredentialis typed as the polymorphic base, so a response check would resolve to the base and never see the secrets, which live on the subtypes. This also covers implicit subscription uniformly (no/subscriptionspath) and passes trivially on specs that only reference the common schema (no local subtype to check).Lifts the mute with a graded severity ramp (
warnfor draft/alpha/rc,errorat public) gated tocommonalities_release: ">=r4.3", so pre-0.8.0 APIs (which predatewriteOnly) aren't checked at all.Which issue(s) this PR fixes:
Part of #297
Special notes for reviewers:
Replaced
TestCheckSinkCredentialNotInResponsewithTestCheckSinkCredentialSecretsWriteOnlyintest_python_checks_subscription.py, covering the clean-subtype case, missing/falsewriteOnlyon each secret field, response-visible fields being ignored, common-$refspecs (no local subtype), implicit-subscription coverage, and a false-positive guard for a generic non-subtypeclientIdfield.The
regression/r4.3-broken-spec-subscriptionsbranch oncamaraproject/ReleaseTesthas been reworked to match (its old sinkCredential-in-response defect no longer trips this rule) and pushed; final fixture recapture is pending this PR reachingvalidation-framework.Test evidence:
python3 -m pytest validation/tests -q— 1180 passed; 3 pre-existing, unrelatedtest_yaml_parser_conformance.pyfailures (reproduce identically on an unmodified checkout).Changelog input
Additional documentation
This section can be blank.