tcli deletes executable bit when building

[art0007i]

we have a mod loader which we upload to thunderstore that includes a shell script in it's build. currently when running tcli build the shell script is marked as non-executable, which is a problem because then users can't execute it after they download the file

[MythicManiac]

Thanks for the report, I don't think we're explicitly doing this so I'd assume whatever logic we use to write the files into the zip simply doesn't set the flag and it's not included by default.

I suspect this would be a pretty simple fix and easy to contribute (hacktoberfest anyone?)

The ZIP format itself doesn't contain explicit support for the flag, but it seems like the External file attributes property of each file entry is used for this, so this would be a matter of writing the correct content to it. See https://superuser.com/a/213782 for a bit more context.

In the meanwhile you can probably work around this somewhat awkwardly by either modifying the zip directly or unzipping, setting the flag on the files, and re-zipping with a tool that preserves the flag. I was about to suggest writing a few lines of python for postprocessing the zip, but I suspect submitting a patch to the CLI here would be roughly the same amount of effort

[art0007i]

so I looked into it a little, the code responsible for creating the zip file is here:

thunderstore-cli/ThunderstoreCLI/Commands/BuildCommand.cs

Lines 182 to 200 in 1116dc7

	using (var archive = new ZipArchive(outputFile, ZipArchiveMode.Create))
	{
	var isWindows = OperatingSystem.IsWindows();
	foreach (var entry in plan)
	{
	Write.Light($"Writing /{entry.Key}");
	var archiveEntry = archive.CreateEntry(entry.Key, CompressionLevel.Optimal);
	if (!isWindows)
	{
	// https://github.com/dotnet/runtime/issues/17912#issuecomment-641594638
	// modifed solution to use a constant instead of a string conversion
	archiveEntry.ExternalAttributes |= 0b110110100 << 16; // rw-rw-r-- permissions
	}
	using (var writer = new BinaryWriter(archiveEntry.Open()))
	{
	writer.Write(entry.Value());
	}
	}
	}

what happens is the build command creates a dictionary of string -> byte[] and then there's a forloop which adds a zip entry for each one of those strings with the given bytes and sets the permission to rw-rw-r--

to fix it a refactor would have to be made, to keep the file metadata or something similar, since at the time of packing, information about the original file is gone, because it was turned into a byte[]

we've already added a workaround on our end by unzipping the archive, adding executable perm, and packing it again like you suggested.

[MythicManiac]

Seems like we need a refactor to pass through more info as you suggested if we want it to reproduce the flags from disk, although that makes me wonder if we should support setting the flags even if the build machine itself is running Windows? This is more about the consumer of the zip than the build source after all.

And if we want this to be buildable on Windows, given that Windows doesn't have the flags at all, we'd need some other way to define which files in the zip should be marked executable. The options I can think of right now is to add config options for marking paths within the zip as executable, or to automate it based on some filename (or content) pattern matching. The former seems a bit cumbersome (and surprising to new users, which they'd probably get wrong at least once before knowing how to configure) but would work, whereas the latter seems like it could be a minor security concern. Granted, given that most consumers of packages are Windows machines and Windows doesn't support the flag in the first place, we'd not be trading down on ecosystem-wide security assumptions here even if we automated it.

Any thoughts?

[art0007i]

I mean any solution will work for me, from the ones you've listed I think putting something in the toml to mark a specific path executable makes sense. I don't think this is too cumbersome at all, especially because most people aren't shipping shell scripts in their ts packages. as for not knowing how to configure it, it's possible to verify if everything worked with tools like 7zip which shows those permission bits even on windows

autodetection could work but there could be a case which isnt covered by the automatic system at which could be annoying or confusing