Y2Z / monolith

⬛️ CLI tool for saving complete web pages as a single HTML file
https://crates.io/crates/monolith
Creative Commons Zero v1.0 Universal
10.91k stars 314 forks source link

Parameter --output doesn't write a file if using docker container #361

Open thomasmerz opened 5 months ago

thomasmerz commented 5 months ago

I'm using monolith in docker container and followed your installation instructions. But when I run this:

docker run --rm y2z/monolith https://web.de -o webde.html

I'm getting no file written at all on my local disk because the docker container has no volume ($PWD) mounted 😞

This can be fixed by this:

docker run --rm -v "$PWD:/mnt" y2z/monolith https://web.de -o /mnt/webde.html
snshn commented 5 months ago

Oh, wow, that's a really neat idea! The only downside I can think of, is that'd be to a detriment to the security aspect that Docker provides... e.g. if the $PWD happens to be ~, it'll just expose everything the user owns to whatever's in the container. Right now it has no ability to "infect" or "hijack" anything, but the -v thing would eliminate that sense of security.

snshn commented 5 months ago

I could make it create a temporary directory, e.g. .docker-monolith-temp-39t84tXb, put the -o file there, move it out of there to one level above, and then clean up by removing .docker-monolith-temp-39t84tXb after. That'd preserve the security, yet allow to use the -o option.

thomasmerz commented 5 months ago

I'm doing this (-v "$PWD:/mnt") already for running shellcheck on my Macbook in a docker container - so it should be safe enough when someone normally wants the output to be written to a file relative to the current working directory 😄

docker run --rm -v "$PWD:/mnt" koalaman/shellcheck:latest -Calways --exclude=SC1090,SC1091 -S warning $*
snshn commented 5 months ago

Wow, that's some 1337 CLI stuff you have going in there. I'll try to do that thing with the temporary directory, will show what I have in a PR.

thomasmerz commented 5 months ago

create a temporary directory

From a user's perspective: do whatever is neccessary to keep it as simple as possible when using it. So you would need to fix this line: $DOCKER run --rm y2z/$PROG_NAME "$@" to something like that? $DOCKER run --rm -v "$PWD:/mnt" y2z/$PROG_NAME $url -o /mnt/$outputfile