-
Notifications
You must be signed in to change notification settings - Fork 3
feat: automatically configure k8s authentication #28
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Greptile OverviewGreptile SummaryThis PR adds opt-in Kubernetes authentication setup via the Major Issues Found:
Recommendations:
Confidence Score: 1/5
Important Files ChangedFile Analysis
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
7 files reviewed, 5 comments
| echo "$TOKEN" | ||
| echo "" | ||
| echo "" | ||
| if [ -n "$CA_CERT" ]; then | ||
| echo "CA certificate:" | ||
| echo "" | ||
| echo "$CA_CERT" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: CRITICAL SECURITY ISSUE: Printing sensitive service account tokens directly to stdout exposes them in Kubernetes logs, which persist indefinitely and can be accessed by anyone with log viewing permissions.
Attack vector: A malicious actor with kubectl logs access can extract this token and use it to authenticate as the infisical-token-reviewer service account with system:auth-delegator cluster-level permissions. This token could be used to validate other service account tokens across the entire cluster.
Risks:
- Tokens remain in logs even after rotation
- Logs may be forwarded to external logging systems (Splunk, ELK, etc.)
- Audit logs expose credentials
- No expiration control once logged
Safer alternatives:
- Store token in a Kubernetes Secret and instruct users to retrieve it via
kubectl get secret - Use a one-time retrieval mechanism
- Don't echo the token at all - provide instructions for users to extract it themselves when needed
- Use short-lived tokens with automatic rotation
| restartPolicy: Never | ||
| containers: | ||
| - name: token-setup | ||
| image: bitnami/kubectl:latest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: Using latest tag without digest pinning creates supply chain security risks. A compromised or malicious image pushed to this tag could execute arbitrary code during Helm installation.
Attack vector: If the bitnami/kubectl repository is compromised, attackers could push malicious code that executes with the permissions of the service account during post-install hooks.
Consider pinning to a specific version and digest:
| image: bitnami/kubectl:latest | |
| image: bitnami/kubectl:1.31.3@sha256:<digest-here> |
| {{- include "secrets-operator.labels" . | nindent 4 }} | ||
| app.kubernetes.io/component: token-setup | ||
| spec: | ||
| activeDeadlineSeconds: 30 # Timeout after 30 seconds to prevent Helm from hanging |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: 30 seconds may be too short for slower clusters or during high load. If the token secret takes longer to provision, the job will fail and Helm may hang or report failure even though the resources were created successfully.
Consider increasing to 60-120 seconds to account for cluster variability.
| annotations: | ||
| "helm.sh/hook": post-install,post-upgrade | ||
| "helm.sh/hook-weight": "1" | ||
| "helm.sh/hook-delete-policy": before-hook-creation # we keep the job after successful creation so we can view logs afterwards |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: The comment states "we keep the job after successful creation so we can view logs afterwards" but this perpetuates the security issue of having sensitive tokens in logs.
Consider using hook-delete-policy: hook-succeeded to clean up the job and its logs after successful completion, reducing the window of token exposure.
| scopedNamespace: "" | ||
| scopedRBAC: false | ||
| installCRDs: true | ||
| configureBasicKubernetesAuth: false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: The new configureBasicKubernetesAuth parameter lacks documentation in the README. Users won't discover this feature without examining source code. Add documentation explaining what it does, when to enable it, security implications, and how to use the generated token.
Context Used: Context from dashboard - When naming sections in documentation, use clear and descriptive titles that reflect the functionali... (source)
This PR adds support for automatically configuring kubernetes authentication directly through Helm.