codeant-ai-for-open-source[bot] commented on PR #33924: URL: https://github.com/apache/superset/pull/33924#issuecomment-3606802382
## 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/33924/files#diff-0e048f8139d3946f36c34f04df82508f36c1e8fb99e3e909b6ab324672fa2743R25-R78'><strong>Missing propagation</strong></a><br>The newly added `attributes` field in `GuestTokenUser` is not assigned to the instantiated `GuestUser` object in `__init__`. As a result, guest users created from tokens will not surface attributes to downstream code (templates, cache key generation, access checks), which appears to contradict the PR intent.<br> - [ ] <a href='https://github.com/apache/superset/pull/33924/files#diff-ddc89915e8af096a98449594cef72e9d33b26bfd37de6b549fd677000a24ee9bR60-R60'><strong>Data validation</strong></a><br>`attributes` accepts arbitrary values via `fields.Raw()`. This can introduce non-serializable or unexpected types into guest tokens (and downstream code that assumes JSON-serializable primitives). Validate that keys are strings and values are restricted to safe, JSON-serializable primitives and enforce size limits.<br> - [ ] <a href='https://github.com/apache/superset/pull/33924/files#diff-2803d15408a51d2724a474c4ce31763b9e11c92f9eb41777b5ba79378c4f1239R352-R359'><strong>JSON serialization failure risk</strong></a><br>The code uses `json.dumps(result, sort_keys=True)` before appending to the cache key. If `result` contains non-serializable objects (or very large values), `json.dumps` may raise a TypeError or produce very large cache keys. Consider robustly handling serialization errors and bounding key size.<br> - [ ] <a href='https://github.com/apache/superset/pull/33924/files#diff-2803d15408a51d2724a474c4ce31763b9e11c92f9eb41777b5ba79378c4f1239R302-R364'><strong>Sensitive attribute exposure & caching</strong></a><br>`get_guest_user_attribute` exposes guest token attributes to templates and, by default, includes them in cache key generation. Confirm that attributes passed in guest tokens cannot contain secrets/PII, or add a whitelist/validation or hashing/truncation to avoid leaking or persisting sensitive values.<br> - [ ] <a href='https://github.com/apache/superset/pull/33924/files#diff-0e048f8139d3946f36c34f04df82508f36c1e8fb99e3e909b6ab324672fa2743R25-R37'><strong>Sensitive information exposure</strong></a><br>The `attributes` bag may unintentionally contain sensitive data (tokens, PII, internal identifiers). Since guest tokens are exported and consumed externally, attributes included in tokens could leak sensitive information if not restricted. Ensure attribute keys/values are vetted and documented.<br> - [ ] <a href='https://github.com/apache/superset/pull/33924/files#diff-ddc89915e8af096a98449594cef72e9d33b26bfd37de6b549fd677000a24ee9bR60-R60'><strong>Sensitive data exposure</strong></a><br>Allowing arbitrary attributes may let callers include PII or secrets in guest tokens. Confirm policy on what can be passed, and ensure tokens or logs do not inadvertently expose sensitive values.<br> - [ ] <a href='https://github.com/apache/superset/pull/33924/files#diff-b58b27c8628fa734d26ecadeb23f8c78701dd56b66fc445a4a1c1b5e82ffe805R304-R407'><strong>SQL injection risk</strong></a><br>Examples interpolate guest attribute values directly into SQL templates (strings, booleans, arrays). If attribute values come from untrusted sources and are not escaped/serialized, they can enable SQL injection or produce invalid SQL. Verify the macro implementation always serializes/escapes values safely (e.g., using JSON serialization or proper quoting) and update docs/examples to show safe usage patterns.<br> - [ ] <a href='https://github.com/apache/superset/pull/33924/files#diff-b58b27c8628fa734d26ecadeb23f8c78701dd56b66fc445a4a1c1b5e82ffe805R304-R407'><strong>Sensitive data exposure</strong></a><br>Guest tokens (and their `attributes`) may be stored, logged, or used in cache keys. Storing secrets, PII, or credentials in attributes can lead to accidental exposure through logs, cache keys, or third-party integrations. Confirm server-side validation and clearly document that sensitive values must not be placed in guest attributes.<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]
