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

change regex to filesystem policy matcher, first step of #871 #873

Merged
merged 4 commits into from
Sep 7, 2021

Conversation

Riolku
Copy link
Contributor

@Riolku Riolku commented Aug 30, 2021

This is the first step of migrating to Landlock support. A very simplistic implementation is given here.

@dmoj-build
Copy link
Collaborator

Can one of the admins verify this patch?

@Xyene
Copy link
Member

Xyene commented Aug 30, 2021

ok to test

dmoj/executors/mixins.py Outdated Show resolved Hide resolved
dmoj/executors/mixins.py Outdated Show resolved Hide resolved
dmoj/executors/mixins.py Outdated Show resolved Hide resolved
dmoj/executors/DART.py Outdated Show resolved Hide resolved
dmoj/executors/filesystem_rules.py Outdated Show resolved Hide resolved
dmoj/executors/mixins.py Outdated Show resolved Hide resolved
dmoj/executors/mixins.py Outdated Show resolved Hide resolved
dmoj/executors/mixins.py Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Sep 3, 2021

Codecov Report

Merging #873 (ce5cd07) into master (1538086) will increase coverage by 0.33%.
The diff coverage is 95.18%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #873      +/-   ##
==========================================
+ Coverage   85.51%   85.84%   +0.33%     
==========================================
  Files         135      137       +2     
  Lines        4466     4599     +133     
==========================================
+ Hits         3819     3948     +129     
- Misses        647      651       +4     
Impacted Files Coverage Δ
dmoj/executors/DART.py 100.00% <ø> (ø)
dmoj/executors/PHP.py 100.00% <ø> (ø)
dmoj/executors/SWIFT.py 100.00% <ø> (ø)
dmoj/judgeenv.py 55.91% <ø> (ø)
dmoj/executors/script_executor.py 81.39% <66.66%> (ø)
dmoj/executors/mixins.py 91.48% <68.42%> (-3.75%) ⬇️
dmoj/cptbox/filesystem_policies.py 98.73% <98.73%> (ø)
dmoj/cptbox/isolate.py 84.69% <100.00%> (+0.22%) ⬆️
dmoj/executors/COFFEE.py 88.46% <100.00%> (+0.46%) ⬆️
dmoj/executors/FORTH.py 100.00% <100.00%> (ø)
... and 10 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1538086...ce5cd07. Read the comment docs.

Copy link
Member

@kiritofeng kiritofeng left a comment

Choose a reason for hiding this comment

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

Will take another look tmr...

dmoj/cptbox/isolate.py Outdated Show resolved Hide resolved
dmoj/executors/SWIFT.py Outdated Show resolved Hide resolved
dmoj/executors/filesystem_policies.py Outdated Show resolved Hide resolved
dmoj/cptbox/isolate.py Outdated Show resolved Hide resolved
dmoj/executors/mono_executor.py Outdated Show resolved Hide resolved
@Riolku
Copy link
Contributor Author

Riolku commented Sep 3, 2021

Java doesn't self-test properly, complaining about a Protection Fault on sys_fchdir, which is surprising, to say the least. The other tests pass though. I'm still unsure what to do about is_file, but otherwise I cleaned up the pending items.

@Riolku
Copy link
Contributor Author

Riolku commented Sep 3, 2021

Also, the flake8 issue is not resolved. Not sure what to do there either.

@quantum5
Copy link
Member

quantum5 commented Sep 3, 2021

Temporary solution: ignore the local flake8 errors when you know they are wrong.

@Riolku
Copy link
Contributor Author

Riolku commented Sep 3, 2021

Temporary solution: ignore the local flake8 errors when you know they are wrong.

I get E302 locally, but I see no ignore rule in .flake8 for that...

@quantum5
Copy link
Member

quantum5 commented Sep 3, 2021

Temporary solution: ignore the local flake8 errors when you know they are wrong.

I get E302 locally, but I see no ignore rule in .flake8 for that...

E302 is for spacing between functions. Something must really be messed up. Do you have line-ending issues?

@Riolku
Copy link
Contributor Author

Riolku commented Sep 3, 2021

E302 is for spacing between functions. Something must really be messed up. Do you have line-ending issues?

riolku@midas:~/workspace/judge/judge$ xxd dmoj/executors/filesystem_policies.py 
00000000: 6672 6f6d 2074 7970 696e 6720 696d 706f  from typing impo
00000010: 7274 204c 6973 742c 2055 6e69 6f6e 0a0a  rt List, Union..
00000020: 0a63 6c61 7373 2041 4343 4553 535f 4d4f  .class ACCESS_MO
00000030: 4445 3a0a 2020 2020 4e4f 4e45 203d 2030  DE:.    NONE = 0
00000040: 0a20 2020 2045 5841 4354 203d 2031 0a20  .    EXACT = 1. 
00000050: 2020 2052 4543 5552 5349 5645 203d 2032     RECURSIVE = 2
00000060: 0a0a 0a63 6c61 7373 2044 6972 3a0a 2020  ...class Dir:.  
00000070: 2020 6465 6620 5f5f 696e 6974 5f5f 2873    def __init__(s
00000080: 656c 6629 3a0a 2020 2020 2020 2020 7365  elf):.        se
00000090: 6c66 2e61 6363 6573 735f 6d6f 6465 203d  lf.access_mode =

I don't think so?

@quantum5
Copy link
Member

quantum5 commented Sep 3, 2021

Then something is really messed up with your flake8 setup. What's the python version? Where are you running flake8?

The code as it currently is is not mergable. You have so many excessive new lines in your code that it hurts my eyes to read.

.freebsd.test.py Outdated Show resolved Hide resolved
dmoj/cptbox/isolate.py Outdated Show resolved Hide resolved
dmoj/cptbox/isolate.py Outdated Show resolved Hide resolved
dmoj/cptbox/isolate.py Outdated Show resolved Hide resolved
dmoj/executors/COFFEE.py Show resolved Hide resolved
dmoj/executors/filesystem_policies.py Outdated Show resolved Hide resolved
dmoj/executors/filesystem_policies.py Outdated Show resolved Hide resolved
dmoj/executors/mixins.py Outdated Show resolved Hide resolved
Copy link
Member

@quantum5 quantum5 left a comment

Choose a reason for hiding this comment

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

Looks pretty good!

dmoj/cptbox/isolate.py Outdated Show resolved Hide resolved
.freebsd.test.py Outdated Show resolved Hide resolved
.freebsd.test.py Outdated Show resolved Hide resolved
@Riolku Riolku force-pushed the landlock-fs branch 2 times, most recently from e7cf629 to e24b01d Compare September 5, 2021 04:42
Copy link
Member

@quantum5 quantum5 left a comment

Choose a reason for hiding this comment

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

Otherwise looks good.

dmoj/executors/mixins.py Outdated Show resolved Hide resolved
Copy link
Member

@quantum5 quantum5 left a comment

Choose a reason for hiding this comment

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

Seems legit. Good job!

dmoj/executors/filesystem_policies.py Outdated Show resolved Hide resolved
dmoj/executors/mixins.py Outdated Show resolved Hide resolved
.freebsd.test.py Outdated Show resolved Hide resolved
.freebsd.test.py Outdated Show resolved Hide resolved
dmoj/executors/filesystem_policies.py Outdated Show resolved Hide resolved
dmoj/executors/mixins.py Outdated Show resolved Hide resolved
dmoj/tests/test_filesystem_policy.py Show resolved Hide resolved
dmoj/executors/filesystem_policies.py Outdated Show resolved Hide resolved
dmoj/executors/filesystem_policies.py Outdated Show resolved Hide resolved
dmoj/executors/filesystem_policies.py Outdated Show resolved Hide resolved
@Riolku
Copy link
Contributor Author

Riolku commented Sep 6, 2021

Waiting on #872, and approval

@Xyene
Copy link
Member

Xyene commented Sep 6, 2021

Hmm, but #872 has already been merged...?

Copy link
Member

@Xyene Xyene left a comment

Choose a reason for hiding this comment

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

Otherwise LGTM, thanks for working on this.

@Riolku Riolku force-pushed the landlock-fs branch 3 times, most recently from 3337814 to 4606b58 Compare September 6, 2021 21:05
@Riolku
Copy link
Contributor Author

Riolku commented Sep 6, 2021

ran black and included changes you mentioned. I'll have to open a new PR with all the other formatting changes black wants to make.

@Riolku Riolku force-pushed the landlock-fs branch 3 times, most recently from c97e543 to 93ad9f5 Compare September 7, 2021 01:26
@Riolku
Copy link
Contributor Author

Riolku commented Sep 7, 2021

Just realized I have a ton of quote issues, thanks to flake8-quotes I can actually see them now...

@Riolku Riolku force-pushed the landlock-fs branch 2 times, most recently from f1934f8 to ce5cd07 Compare September 7, 2021 03:13
@Riolku
Copy link
Contributor Author

Riolku commented Sep 7, 2021

Simply rebasing after #887

@Riolku Riolku force-pushed the landlock-fs branch 2 times, most recently from c53f247 to 940fba3 Compare September 7, 2021 03:31
@Riolku
Copy link
Contributor Author

Riolku commented Sep 7, 2021

This is the first step of migrating to Landlock support. A very simplistic implementation is given here.

Notably, this is no longer at all true. A trie implementation was taken instead.

@quantum5 quantum5 merged commit e79b47b into DMOJ:master Sep 7, 2021
@Riolku Riolku deleted the landlock-fs branch September 7, 2021 04:36
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.

7 participants