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

Preventing imeta mod bypass #69

Merged
merged 1 commit into from
Feb 20, 2024
Merged

Conversation

FifthPotato
Copy link
Contributor

@FifthPotato FifthPotato commented Feb 15, 2024

Seems to work so far. Need to do a bit more research to figure out if there are any other cases I missed.

Need to write tests, too.

@trel
Copy link
Member

trel commented Feb 15, 2024

this is to address #58?

@trel
Copy link
Member

trel commented Feb 16, 2024

good so far - let's get a couple tests in today.

@korydraughn
Copy link
Collaborator

All we need now are the tests and we're good to go.

@FifthPotato FifthPotato marked this pull request as ready for review February 19, 2024 11:03
@FifthPotato
Copy link
Contributor Author

Added test, and force-pushed because I forgot to update main on my repo after my last PR.

Copy link
Collaborator

@korydraughn korydraughn left a comment

Choose a reason for hiding this comment

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

Once the final review comments are resolved. Please run the tests and report whether they succeeded/failed.

@korydraughn
Copy link
Collaborator

Don't forget to mark comments resolved as you complete them.

Copy link
Collaborator

@korydraughn korydraughn left a comment

Choose a reason for hiding this comment

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

This may be the one.

Please run the tests and let us know if they pass or fail.

@FifthPotato
Copy link
Contributor Author

Tests pass.

@korydraughn
Copy link
Collaborator

Good. Please squash to taste, no #'s

@FifthPotato
Copy link
Contributor Author

Squashed

@korydraughn
Copy link
Collaborator

korydraughn commented Feb 19, 2024

Let's adjust the commit message so that it isn't specific to imeta. How about ...

[58] Do not allow renaming non-protected AVU to existing protected AVU.

Looking at the test again, it appears we're missing tests which prove imeta mod <protected_avu> <protected_avu> is covered.

I'll write up an issue for that. We can add tests in a later PR.

@FifthPotato
Copy link
Contributor Author

Amended!

Copy link
Collaborator

@korydraughn korydraughn left a comment

Choose a reason for hiding this comment

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

Please add the pound.

@FifthPotato
Copy link
Contributor Author

Octothorpe'd

@alanking alanking merged commit de5d5ac into irods:main Feb 20, 2024
2 checks passed
@korydraughn
Copy link
Collaborator

Please cherry-pick to the 4-3-stable branch, no #'s.

@korydraughn
Copy link
Collaborator

@FifthPotato ping!

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

Successfully merging this pull request may close these issues.

4 participants