rusackas commented on PR #39645: URL: https://github.com/apache/superset/pull/39645#issuecomment-4787577648
@orbisai0security @aminghadersohi want to make sure I'm reading this right before we do anything with it. The diff against current master is now basically just the debug-log cleanup (dropping the exception detail and the missing scopes from the logger.debug lines). The actual hardening this PR set out to add (the alg=none rejection via _FORBIDDEN_ALGORITHMS, the mandatory exp check, the int(exp) handling) all already seems to be on master, so I think it landed separately somewhere along the way. If that's right, there's not much security substance left here... and the bot note about OverflowError on a non-finite exp is really about that pre-existing code on master, not anything this PR introduces. Am I missing anything? If the security bits are genuinely already covered, I'm inclined to close this as superseded and file the OverflowError edge case as a small follow-up against master instead. What do you think we should do here? -- 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]
