Skip to content
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

Fix(Security): Re-define AuthorizationExecuteWithPrivileges #712

Merged
merged 1 commit into from
Feb 13, 2025

Conversation

PaulDance
Copy link
Contributor

@PaulDance PaulDance commented Feb 11, 2025

Fixes #711 by manually re-defining the problematic function. Another possibility could have been to add a patch system that runs on the SDK before generating the modules since the bug comes from there, but I don't think it exists yet and one could consider it overkill for this small need. In the meantime, this works.

@PaulDance PaulDance marked this pull request as ready for review February 11, 2025 12:11
@PaulDance
Copy link
Contributor Author

@madsmtm would it be possible to review this, please? Doing so somewhat soon would help avoid having to rebase this regularly to fix conflicts since it touches the generated modules.

Copy link
Owner

@madsmtm madsmtm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@madsmtm would it be possible to review this, please? Doing so somewhat soon would help avoid having to rebase this regularly to fix conflicts since it touches the generated modules.

Yeah sorry, I forgot about this PR when I made 349dc40 🙄

@PaulDance
Copy link
Contributor Author

PaulDance commented Feb 13, 2025

Wow, that was fast 😄

Yeah sorry, I forgot about this PR when I made 349dc40 🙄

That's alright 👌

@madsmtm
Copy link
Owner

madsmtm commented Feb 13, 2025

I typed that before you wrote your comment, got a notification ping because you rebased ;)

@madsmtm madsmtm merged commit a06bdec into madsmtm:master Feb 13, 2025
20 checks passed
@PaulDance PaulDance deleted the security-fix branch February 13, 2025 18:31
@PaulDance
Copy link
Contributor Author

Thanks!

Do you know if it would be possible to publish a 0.3.1 patch release for objc2-security containing this or do you for now prefer to keep all framework crates synchronized in versioning?

@madsmtm
Copy link
Owner

madsmtm commented Feb 13, 2025

I'm still unsure, but I think I'd prefer to have framework crates synchronized, yeah.

@madsmtm
Copy link
Owner

madsmtm commented Feb 13, 2025

Have created a milestone with a reasonable deadline (which, as you know, I'm terrible at, but hopefully this one won't be as stressful).

@PaulDance
Copy link
Contributor Author

I'm still unsure, but I think I'd prefer to have framework crates synchronized, yeah.

Given the quantity, that's definitely understandable 👌

Have created a milestone with a reasonable deadline

Seems fair, thanks!

(which, as you know, I'm terrible at, but hopefully this one won't be as stressful)

😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

objc2_security::AuthorizationExecuteWithPrivileges has a wrong type for its arguments parameter
2 participants