codeant-ai-for-open-source[bot] commented on PR #37070: URL: https://github.com/apache/superset/pull/37070#issuecomment-3742002097
## Nitpicks 🔍 <table> <tr><td>🔒 <strong>No security issues identified</strong></td></tr> <tr><td>⚡ <strong>Recommended areas for review</strong><br><br> - [ ] <a href='https://github.com/apache/superset/pull/37070/files#diff-75da08c58e675e1ffd330e584d9127ff5d88cd299b439da5ddf145b08feeecedR11-R11'><strong>Sensitive secrets storage</strong></a><br>The tenant model lists `client_secret` and `db_sqlalchemy_uri` as columns. Storing secrets and full DB URIs in plaintext introduces risk of accidental exposure (logs, backups, admin UIs). Confirm how secrets are stored, who can access them, rotation strategy, and whether secrets are only referenced (IDs/ARns) rather than stored directly.<br> - [ ] <a href='https://github.com/apache/superset/pull/37070/files#diff-75da08c58e675e1ffd330e584d9127ff5d88cd299b439da5ddf145b08feeecedR15-R17'><strong>Dynamic OAuth provider selection risks</strong></a><br>Dynamically selecting OAuth provider from tenant records increases attack surface: a malicious tenant record could point to a rogue IdP or alter redirect URIs. Ensure tenant records are validated at creation, redirect URIs are pinned/whitelisted, and flows cannot be persuaded to send tokens to untrusted endpoints.<br> - [ ] <a href='https://github.com/apache/superset/pull/37070/files#diff-75da08c58e675e1ffd330e584d9127ff5d88cd299b439da5ddf145b08feeecedR9-R12'><strong>Tenant discovery / routing safety</strong></a><br>The discovery pattern (subdomain per tenant) and request flow rely on parsing hostnames into tenant slugs. Validate inputs and canonicalize host headers to prevent host header attacks, subdomain takeover, or spoofed tenant resolution. Define reject/deny behavior for ambiguous or missing tenants.<br> - [ ] <a href='https://github.com/apache/superset/pull/37070/files#diff-75da08c58e675e1ffd330e584d9127ff5d88cd299b439da5ddf145b08feeecedR28-R31'><strong>RBAC / role source trust</strong></a><br>Role mapping from IdP claims or FAB API must define a clear trust boundary. Prevent privilege escalation from crafted claims, and specify how role changes are audited and reconciled across tenants.<br> - [ ] <a href='https://github.com/apache/superset/pull/37070/files#diff-75da08c58e675e1ffd330e584d9127ff5d88cd299b439da5ddf145b08feeecedR21-R26'><strong>Provisioning & idempotency guarantees</strong></a><br>Provisioning APIs (create DB, import assets) must be authenticated, authorize the caller for tenant operations, and guarantee idempotent behavior and safe rollback on partial failures. Consider transactional semantics and cleanup for multi-step imports.<br> </td></tr> </table> -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
