You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "villebro (via GitHub)" <gi...@apache.org> on 2026/09/02 14:31:46 UTC
[PR] fix(controller): singleton hardening [superset-kubernetes-operator]
villebro opened a new pull request, #331:
URL: https://github.com/apache/superset-kubernetes-operator/pull/331
## Summary
Four independent low-severity findings from a Claude security scan, each a small blast-radius / consistency hardening with no shared root cause.
## Details
- **Leader-election Role granted unused ConfigMap CRUD.** controller-runtime defaults to the Leases lock and `cmd/main.go` never selects a ConfigMap lock, so the grant is dead. Removed from the Kustomize Role and the Helm chart twin (snapshots regenerated); the manager ClusterRole's legitimate ConfigMap access is unchanged.
- **Reserved `superset.apache.org/` labels were forgeable on Deployment/Service metadata.** Only pod templates were stripped, so a CR author could stamp another instance's parent label onto their Deployment/Service object metadata. `MergeDeploymentTemplate` now strips reserved labels (covering Deployment metadata via the resolver), and the two Service metadata merges strip them via the newly exported `resolution.StripReservedLabels`. *(Residual: Ingress/HTTPRoute user-label merges still pass non-parent reserved keys — candidate follow-up.)*
- **Credential redaction missed quoted-key formats.** `credentialAssignmentRe` couldn't match JSON / Python-dict-repr forms (`"password": "x"`, `'db_password': 'x'`) — the closing quote sits between the keyword and the separator — a whole serialization class. Added `quotedKeyCredentialRe` (also covering `authorization`) that masks the quoted value in full with a bare placeholder, keeping redaction idempotent even on adversarial unbalanced-quote input; added quoted-key cases to the unit and fuzz corpora.
- **User pod annotations could override the operator config-checksum annotation.** `buildDeploymentSpec` merged user pod-template annotations last, so a CR author could pin `superset.apache.org/config-checksum` and freeze rollouts (config and SECRET_KEY rotations stop rolling pods while status misreports as reconciled). The merge order now applies operator annotations last (matching the label rule and the lifecycle task-Job path). Also covers the maintenance-page Deployment (same builder).
## Release notes
- Instances that had pinned the `superset.apache.org/config-checksum` pod annotation roll once on upgrade as the operator's computed checksum reasserts itself.
Found via a Claude security scan.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(controller): harden labels, checksum, and redaction [superset-kubernetes-operator]
Posted by "villebro (via GitHub)" <gi...@apache.org>.
villebro merged PR #331:
URL: https://github.com/apache/superset-kubernetes-operator/pull/331
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(controller): singleton hardening [superset-kubernetes-operator]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #331:
URL: https://github.com/apache/superset-kubernetes-operator/pull/331#issuecomment-5511594557
## [Codecov](https://app.codecov.io/gh/apache/superset-kubernetes-operator/pull/331?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `83.33333%` with `1 line` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 93.06%. Comparing base ([`01f4b2f`](https://app.codecov.io/gh/apache/superset-kubernetes-operator/commit/01f4b2f9177ee3d88551cd423f6d55d12648f5bb?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`cf1edce`](https://app.codecov.io/gh/apache/superset-kubernetes-operator/commit/cf1edce45edd214096c04ede5eda7d1e9ae1830f?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 3 commits behind head on main.
| [Files with missing lines](https://app.codecov.io/gh/apache/superset-kubernetes-operator/pull/331?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [internal/resolution/merge.go](https://app.codecov.io/gh/apache/superset-kubernetes-operator/pull/331?src=pr&el=tree&filepath=internal%2Fresolution%2Fmerge.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aW50ZXJuYWwvcmVzb2x1dGlvbi9tZXJnZS5nbw==) | 50.00% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/superset-kubernetes-operator/pull/331?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset-kubernetes-operator/pull/331?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(controller): singleton hardening [superset-kubernetes-operator]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #331:
URL: https://github.com/apache/superset-kubernetes-operator/pull/331#discussion_r3936488625
##########
internal/controller/redact.go:
##########
@@ -50,6 +50,19 @@ var (
// "password=hunter2", "PGPASSWORD: hunter2", or "secret_key = hunter2".
// The key may carry prefixes/suffixes (DB_PASSWORD, api-key-prod).
credentialAssignmentRe = regexp.MustCompile(`(?i)([A-Za-z0-9_-]*(?:password|passwd|pwd|secret|token|api[_-]?key|credential|passphrase)[A-Za-z0-9_-]*\s*[=:]\s*)(\S+)`)
+
+ // quotedKeyCredentialRe matches quoted-key credential assignments as emitted
+ // by JSON payloads and Python dict/traceback reprs, e.g.
+ // {"password": "hunter2"} or {'db_password': 'hunter2'}. credentialAssignmentRe
+ // cannot match these — the closing quote sits between the keyword and the
+ // separator, which its key class does not allow — so quoted-key forms are a
+ // whole serialization class it systematically misses. The value is matched as
+ // a quoted string of either style (RE2 has no backreferences, so both quote
+ // styles are spelled out) and masked in full, including embedded whitespace.
+ // "authorization" is included because authorizationHeaderRe is equally
+ // quote-blind. The value is replaced with a bare placeholder so a re-run
+ // finds no quoted value to match (idempotent).
+ quotedKeyCredentialRe = regexp.MustCompile(`(?i)(["'][A-Za-z0-9_-]*(?:password|passwd|pwd|secret|token|api[_-]?key|credential|passphrase|authorization)[A-Za-z0-9_-]*["']\s*[=:]\s*)("[^"]*"|'[^']*')`)
Review Comment:
Not blocking, but I poked at this and a quoted key with a bare value still slips through, e.g. `"password": hunter2` or `'password': 12345`. Adding a `|\S+` alternative to the value group seems to close it and stays idempotent since `***` re-matches itself. Happy to leave it for a follow-up if you'd rather keep this one tight.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org