docs(security): point endpoint + credential docs at the real controls (#299, #251) - #333
Merged
Merged
Conversation
Two documentation-accuracy fixes surfaced while verifying the #299/#251 acceptance criteria; no behavior change. #299 (endpoint SSRF), acceptance #4 — the remoteControl.endpoint field godoc described only shape validation and never pointed at the control that actually bounds where the operator will dial. Add a note that confining the destination is an admin egress concern: the --remote-control-allowed-endpoint-hosts allow-list (empty = any, backward compatible) plus the egress NetworkPolicy. Flows into all four CRD copies via controller-gen. #251 (remoteControl.tls confused deputy) — the resolveRemoteControlTLS comment claimed to be "a partial mitigation, not an authorization boundary" and pointed only at a hypothetical future SubjectAccessReview. That is now stale and understates what shipped: the credentialRefPolicy ValidatingAdmissionPolicy (ADR-0009) IS the enforced boundary, requiring the CR's writer to hold `get` on the referenced Secret. Rewrite the comment to name it, while staying honest that the policy is opt-in and OFF by default, so the two in-resolver gates remain a partial mitigation out of the box. #251 stays open (automated restricted-user envtest and the default-posture decision are still outstanding).
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.
What
Two documentation-accuracy fixes surfaced while verifying the acceptance criteria of #299 and #251. No behavior change — comment + field-godoc only (plus the CRD regeneration the godoc triggers).
#299 (任意 endpoint SSRF) — acceptance #4
The
remoteControl.endpointfield godoc documented only the shape validation (host:port, DNS-1123, CEL) and never pointed at the control that actually bounds which host the operator will dial. Added a note that confining the destination is an admin egress concern, naming the two enforced controls:--remote-control-allowed-endpoint-hostsapp-layer allow-list (empty = any, backward compatible)NetworkPolicyatconfig/network-policy/allow-egress-traffic.yamlThe godoc flows into all four CRD copies via
controller-gen(config/crd/bases,bundle/manifests,dist/chart/templates/crd,nephio/packages).This closes the last open acceptance item for #299 (the allow-list, plaintext-relay gate, and egress policy already shipped in #300/#317/#324). #299 is closeable after this merges.
#251 (remoteControl.tls confused deputy)
The
resolveRemoteControlTLSSECURITY comment called itself "a partial mitigation, not an authorization boundary" and pointed only at a hypothetical futureSubjectAccessReview. That is now stale and understates what shipped: the opt-incredentialRefPolicyValidatingAdmissionPolicy (ADR-0009) IS the enforced boundary — it makes the apiserver require the CR's writer to holdgeton the referenced Secret (authorizer…namespace(request.namespace).name(…secretName).check('get')).Rewrote the comment to name that policy as the real boundary, while staying honest that it is opt-in and
enable: falseby default, so the two in-resolver gates (reject API-credential Secret types; require an owner opt-in label) remain a partial mitigation out of the box.#251 stays OPEN — remaining work: an automated envtest exercising a restricted (secrets-
get-less) user against the live policy, and the decision on whether to flip the default posture. This PR only corrects the misleading comment.Verification
go build ./...+go vet ./api/... ./internal/controller/...cleanmake manifests bundle nephio-sync— all four CRD copies regenerated consistently (+6 lines each)pkg/netutil/allowlist.go:50), NetworkPolicy path exists, and the VAP CEL scopesgetto the referenced Secret (dist/chart/templates/admission/credential-ref-policy.yaml:37-40)