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

Typst command implementation #1626

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

RundownRhino
Copy link

Relevant Issues

Closes #1623.

Description

  • The overall structure is based on the Latex cog, with about the same features (caching, support for code blocks, etc)
  • Refactored some utility functions (such as the codeblock regex) out of the Latex cog to reuse them here.
  • Initially I was going to use the typst PyPI package, but while it works, it had a weird issue where (only in the bot's container and not on my computer) it needed ~1GB RAM to render the simplest things. I failed to figure out what caused this, and as a result I switched to using the CLI, which is less hungry (~100MB for sane inputs) and supports some nice features the python bindings don't (such as taking the input from stdin with no files involved).
  • The typst executable is not included, but instead downloaded (with hash verification) at cog load time, from a link that can be altered with envvars, and then stored in the cache.
  • If the global typst packages directory is missing, a default set of packages is installed and the directory then chmoded to not allow writes.
  • To render, the typst executable gets called in a subprocess, with a time limit (and gets killed if the limit is exceeded) and a memory usage rlimit.
  • If the rendered image is too big, this is considered an error (to hopefully make it a bit harder to maliciously post big outputs).
  • The rendered image also gets manually cropped to only the content and not the white background (typst's own formatting doesn't seem to have an option for it, and this is the more resilient choice anyway).

Security-wise, the typst invocation sets --root to an empty temporary directory, which in theory should prevent access to any files outside of it. As for the packages, even though arbitrary typst packages are supposed to be safe, I still chose to prevent installation of them at runtime by locking the packages directory from writes. Nevertheless, I am pretty worried about the security of this and want someone to take a second look at whether it's exploitable. And of course, if there's ever a new vulnerability in typst itself, that'd potentially make Lancebot's environment vulnerable.

Did you:

log.info("Storing the typst executable on disk")
exe_path = Path(Config.typst_path)
exe_path.parent.mkdir(exist_ok=True)
exe_path.write_bytes(typst_executable)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't going to work in our infrastructure. We use read-only file systems.

I am also very uneasy with the fact we're downloading and executing a binary from the internet like this into the bot's pod.

Is there a typst API similar to what we use for latex? Either that, or we can look into having a typst deployment in our infra, where it won't have access to the bot's secrets.

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

Successfully merging this pull request may close these issues.

Add Typst support
2 participants