fix: redact all query parameters in CRI error logs#12491
fix: redact all query parameters in CRI error logs#12491AkihiroSuda merged 1 commit intocontainerd:mainfrom
Conversation
0fa590e to
445b33e
Compare
445b33e to
4e17f79
Compare
| log.G(ctx).WithError(err).Errorf("PullImage %q failed", r.GetImage().GetImage()) | ||
| // Sanitize error to remove sensitive information | ||
| sanitizedErr := ctrdutil.SanitizeError(err) | ||
| log.G(ctx).WithError(sanitizedErr).Errorf("PullImage %q failed", r.GetImage().GetImage()) |
There was a problem hiding this comment.
It loos like this is fixing the logs but not the returned error.
Also needs to fix spans.
I think it should set err = ctrdutil.SanitizeError(err) instead of creating a new var.
internal/cri/util/sanitize.go
Outdated
|
|
||
| // sensitiveParams defines query parameters that should be redacted. | ||
| var sensitiveParams = map[string]bool{ | ||
| "sig": true, // Azure SAS signature |
There was a problem hiding this comment.
This will need more things for sure.
Maybe we can just redact all query params?
AkihiroSuda
left a comment
There was a problem hiding this comment.
The code looks good, but please squash the commits
Signed-off-by: Andrey Noskov <[email protected]>
4d4cb55 to
3e2cee2
Compare
|
/cherry-pick release/2.2 |
|
@AkihiroSuda: once the present PR merges, I will cherry-pick it on top of DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@AkihiroSuda: new pull request created: #12546 DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@AkihiroSuda: new pull request created: #12547 DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@AkihiroSuda: #12491 failed to apply on top of branch "release/1.7": DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
…pod events PR containerd#12491 fixed credential leaks in containerd logs but the gRPC error returned to kubelet still contained sensitive information. This was visible in Kubernetes pod events via Name: copy-helper Namespace: default Priority: 0 Service Account: default Node: aks-nodepool1-23688426-vmss000000/10.224.0.4 Start Time: Tue, 20 Jan 2026 20:00:18 +0000 Labels: run=copy-helper Annotations: <none> Status: Succeeded IP: 10.244.0.188 IPs: IP: 10.244.0.188 Containers: copy-helper: Container ID: containerd://4dfd76c7a08f2363bbfcc2a194cf96d9905d52d2a80741570b6705efb6eb8737 Image: mcr.microsoft.com/cbl-mariner/base/core:2.0 Image ID: mcr.microsoft.com/cbl-mariner/base/core@sha256:537f38378c574bfa8aee6c520c3317502d9a50f5bf8e7f2bf5c30774e2c25886 Port: <none> Host Port: <none> Command: sleep 3600 State: Terminated Reason: Completed Exit Code: 0 Started: Tue, 20 Jan 2026 20:00:20 +0000 Finished: Tue, 20 Jan 2026 21:00:20 +0000 Ready: False Restart Count: 0 Environment: <none> Mounts: /var/run/secrets/kubernetes.io/serviceaccount from kube-api-access-gf8kj (ro) Conditions: Type Status PodReadyToStartContainers False Initialized True Ready False ContainersReady False PodScheduled True Volumes: kube-api-access-gf8kj: Type: Projected (a volume that contains injected data from multiple sources) TokenExpirationSeconds: 3607 ConfigMapName: kube-root-ca.crt ConfigMapOptional: <nil> DownwardAPI: true QoS Class: BestEffort Node-Selectors: <none> Tolerations: node.kubernetes.io/not-ready:NoExecute op=Exists for 300s node.kubernetes.io/unreachable:NoExecute op=Exists for 300s Events: <none>. The issue was that SanitizeError was called inside the defer block, but errgrpc.ToGRPC(err) was evaluated before the defer ran, so the gRPC message contained the original unsanitized error. Move SanitizeError before the return statement so both the logged error and the gRPC error are sanitized. Ref: containerd#5453
…pod events PR containerd#12491 fixed credential leaks in containerd logs but the gRPC error returned to kubelet still contained sensitive information. This was visible in Kubernetes pod events via `kubectl describe pod`. The issue was that SanitizeError was called inside the defer block, but errgrpc.ToGRPC(err) was evaluated before the defer ran, so the gRPC message contained the original unsanitized error. Move SanitizeError before the return statement so both the logged error and the gRPC error are sanitized. Ref: containerd#5453 Signed-off-by: Aadhar Agarwal <[email protected]>
…pod events PR containerd#12491 fixed credential leaks in containerd logs but the gRPC error returned to kubelet still contained sensitive information. This was visible in Kubernetes pod events via `kubectl describe pod`. The issue was that SanitizeError was called inside the defer block, but errgrpc.ToGRPC(err) was evaluated before the defer ran, so the gRPC message contained the original unsanitized error. Move SanitizeError before the return statement so both the logged error and the gRPC error are sanitized. Ref: containerd#5453 Signed-off-by: Aadhar Agarwal <[email protected]>
…pod events PR containerd#12491 fixed credential leaks in containerd logs but the gRPC error returned to kubelet still contained sensitive information. This was visible in Kubernetes pod events via `kubectl describe pod`. The issue was that SanitizeError was called inside the defer block, but errgrpc.ToGRPC(err) was evaluated before the defer ran, so the gRPC message contained the original unsanitized error. Move SanitizeError before the return statement so both the logged error and the gRPC error are sanitized. Ref: containerd#5453 Signed-off-by: Aadhar Agarwal <[email protected]>
…pod events PR containerd#12491 fixed credential leaks in containerd logs but the gRPC error returned to kubelet still contained sensitive information. This was visible in Kubernetes pod events via `kubectl describe pod`. The issue was that SanitizeError was called inside the defer block, but errgrpc.ToGRPC(err) was evaluated before the defer ran, so the gRPC message contained the original unsanitized error. Move SanitizeError before the return statement so both the logged error and the gRPC error are sanitized. Ref: containerd#5453 Signed-off-by: Aadhar Agarwal <[email protected]> (cherry picked from commit 7b11d6c) Signed-off-by: Akihiro Suda <[email protected]>
Trying to fix #5453