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

Overly restrictive tar-file requirements (leading ./ needed) #203

Open
tgolsson opened this issue Jul 26, 2023 · 5 comments
Open

Overly restrictive tar-file requirements (leading ./ needed) #203

tgolsson opened this issue Jul 26, 2023 · 5 comments

Comments

@tgolsson
Copy link

Hey,

I'm investigating a very specific issue using deploy-pages, and I think I've narrowed it down to a 100% repro. I'm using a custom build setup with a static page generator. Due to some build process changes I ended up moving from packaging loose files to directly building the tar archive (i.e., replacing upload-pages-artifact). That change broke this action. After a lot of testing; I've found that the two commands below end up bisecting the behavior. In practice, the latter archive is generated by a build tool without using the tar command.

This works:

tar cvf "artifact.tar" ./*

While this fails:

tar cvf "artifact.tar" *

In a hypothetical situation where we're only tarring an index.html the first command creates the tar file [./index.html], whereas the latter is simply [index.html]. Not handling this seems like a bug, but at a minimum it should probably be documented on this action if it's intentionally required. As far as I can tell, on my specific version of tar, these two archives extract identically regardless of cwd, -C, etc.

This is the error message when using the latter format:

Artifact could not be deployed. Please ensure the content does not contain any hard links, symlinks and total size is less than 10GB.

The same behavior can also be observed by adding --xform s:'./':: to the working command, which indicates that this is indeed a result of the directory structure inside the tar.

The tar used for testing:

tar (GNU tar) 1.34

Happy to supply any more info as necessary.

@yoannchaudet
Copy link
Collaborator

Hello,

This is an intentional check. We validate tar files before deployments and won't allow anything that does not start with the ./ prefix (or anything that contains .. for that matter).

We do that for security purposes mainly and have no intentions of lowering this requirement.

You are free to package your artifact the way you like and not use actions/upload-pages-artifact. You will just need to format the archive in the format we expect.

@tgolsson
Copy link
Author

Thanks for the quick response @yoannchaudet!

This is an intentional check. We validate tar files before deployments and won't allow anything that does not start with the ./ prefix (or anything that contains .. for that matter).

We do that for security purposes mainly and have no intentions of lowering this requirement.

I understand the concern with absolute paths and .., and am definitely not arguing for "anything goes" here. However; I'm missing how ./index.html is fine and index.html is dangerous. I've not found anything in the tar documentation for how these would behave differently, and cannot show such a difference myself. In the interest of learning, can you expand a bit on this or share any references where I can read more about why this matters?

You are free to package your artifact the way you like and not use actions/upload-pages-artifact. You will just need to format the archive in the format we expect.

Is the full format you expect documented somewhere? This repository says the following:

This action expects an artifact named github-pages to have been created prior to execution

and the upload-pages-artifact repository says the following:

A Pages artifact must:

The tar file must:

  • be under 10GB in size
  • not contain any symbolic or hard links
  • contain only files and directories that all meet the expected minimum file permissions

And neither mentions this ./ prefix requirement. I cannot find any information about this in the GitHub pages documentation either.

@yoannchaudet
Copy link
Collaborator

Thank you for pointing our documentation gaps. Will make some amendments. Consider actions/upload-pages-artifact the implementation reference today. Documentation can go stale, code will not.

@tgolsson
Copy link
Author

Thank you. Maybe a separate error message would be great too, as the current one does not mention this possibility at all. :-)

@1e1001
Copy link

1e1001 commented Apr 16, 2024

Also it seems archive is actually not gzip'd? if i upload a .tar.gz i get the invalid archive error

(although i guess with just a tar i get an error anyways, but it's just a generic "Deployment failed")

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

3 participants