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

File-edited check doesn't check permissions on subsequent invocations #1595

Open
ag-eitilt opened this issue Jul 10, 2024 · 0 comments
Open

Comments

@ag-eitilt
Copy link
Collaborator

Split from #1589 but not covered in the meeting: If the user changes the permissions of a file previously output by Wake but doesn't edit its contents, Wake doesn't detect that difference and assumes that the cached Job may still be used as-is:

$ wake -x 'makePlan "test" Nil "echo -n test > test.txt" | runJob'
Job 617
$ ls -l test.txt
-rw-r--r-- 1 sammay sammay 4 Jul 10 14:56 test.txt
$ chmod -w test.txt
$ wake -x 'makePlan "test" Nil "echo -n test > test.txt" | runJob'
Job 617
$ ls -l test.txt
-r--r--r-- 1 sammay sammay 4 Jul 10 14:56 test.txt

(Note the same Job ID but different permissions.)

This might not actually be much of a problem in practice, as if a later Job tries to write to that same file Wake itself is supposed to complain, permissions or not (however, note that for some reason in my first invocation below it's the Job which is failing rather than a File output by multiple jobs panic being thrown), but it is easily possible to create a Job which fails if it doesn't have write permissions in such a way it wouldn't produce any output file, and therefore wouldn't trigger the panic. Theoretically, this is no different from what's possible with the stat prim/function, but that at least is known to be if not unsafe, at least advanced-audience.

$ wake -x 'makePlan "test" Nil "echo -n test > test.txt" | runJob | getJobOutputs |> (makePlan "edit" _ "echo edit >> test.txt" | runJob | getJobOutputs)'
Fail (Error "Non-zero exit status (Exited 1) for job 748: 'edit'" Nil)
$ chmod -w test.txt 
$ wake -x 'makePlan "test" Nil "echo -n test > test.txt" | runJob | getJobOutputs |> (makePlan "edit" _ "echo edit >> test.txt" | runJob | getJobOutputs)'
/usr/bin/dash: 1: cannot create test.txt: Permission denied
Fail (Error "Non-zero exit status (Exited 2) for job 755: 'edit'" Nil)

(I'm not actually sure what's going on there; given the current limitations of the checker, I would have expected the first edit to work within the sandbox but to panic immediately in the process of exiting that sandbox, and only the second to have one of the "successfully completed with a failure" cases we laugh about.)

Note that this isn't a problem with chmod -r as (presumably) the lack of a readable hash triggers the Job to be rerun, and similar to #1594, the file to be recreated; it might be a problem if the executable bit is the one being toggled.

$ wake -x 'makePlan "test" Nil "echo -n test > test.txt" | runJob'
Job 767
$ chmod -r test.txt 
$ ls -l test.txt 
--w------- 1 sammay sammay 4 Jul 10 15:18 test.txt
$ wake -x 'makePlan "test" Nil "echo -n test > test.txt" | runJob'
Job 780
$ ls -l test.txt 
-rw-r--r-- 1 sammay sammay 4 Jul 10 15:19 test.txt

All confusion aside, the third example is what I'd actually expect: the file after the second Job is exactly as it was (modtime aside) immediately after the first Job. The first example is illustrating a bug where Wake's reproducibility checks aren't actually tight enough to ensure that assumption, and the second was meant to illustrate a potential panic/ignorable-Fail break in reproducibility leading from that but is actually accidentally illustrating some bug I'm not even sure how to categorize.

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

No branches or pull requests

1 participant