github-actions-demo
github-actions-demo copied to clipboard
slsa-framework
Improve error message for broken symlinks on artifact_path.
Fixes #22
Well, it doesn't really fix #22 but it's the best we can do to highlight the issue.
Actually hold on, I'm not sure I know what I'm talking about.
~~My understanding is that #22 fails on a valid symlink. The file exists, we just can't get to it with filepath.Walk? Would something like https://pkg.go.dev/github.com/facebookgo/symwalk be the way to solve this?~~
Nah the issue is that we'd be walking from inside the container so the symlink will be broken.
I thought that mark's example actually had a valid file in a valid location because https://github.com/MarkLodato/example-build/blob/main/.github/workflows/build.yaml#L17 still uploaded something?
The symlink is valid. It's just not visible to the binary running in the container.
I don't think this PR is sufficient to close #22. To improve the error message, we should (1) put a "known issue" on the main page and (2) if we detect that it's a broken symlink, mention that this is a known issue. Just saying "broken symlink" is still going to be confusing since it's not actually broken.
The symlink is valid. It's just not visible to the binary running in the container.
I don't think this distinction is valuable. We're running this action in a container so we have no other view of the system. To us, the symlink is broken.
We could provide more context around why the symlink may be broken but that's pretty much it.
I don't think this PR is sufficient to close #22. To improve the error message, we should (1) put a "known issue" on the main page and (2) if we detect that it's a broken symlink, mention that this is a known issue. Just saying "broken symlink" is still going to be confusing since it's not actually broken.
I think it's a good idea to add it as a known issue.
The symlink breakage is the result of the execution environment but it's broken to the code being run. I'll add more context to the error message but a broken symlink could be either broken by the fs mapping done by docker or broken on the system and we have no way of reliably differentiating between the two.
Friendly reminder that this PR is still open :-). Probably good to submit even an imperfect solution now while we wait for a better one.
I guess the alternative is: we could rewrite everything in typescript or something that wouldn't have the container sandbox.