Allow passing arbitrary CLI options to the engine on a per block-level basis

[kainctl]

As per the discussion in #62

My wish is to allow passing arbitrary CLI options to the engine on a per block-level basis.

The biggest concern is the introduction of security issues and is the reason why this option should be opt-in.
But I personally would like to clarify what security issues are introduced.

From the previous disucssion:

Is shell injection possible?

From my investigation in #62 (comment)

From my short investigation:

https://github.com/jgm/pandoc/blob/d4d69fafb9bc8034cd39e99dbe48b2545eba2d3f/src/Text/Pandoc/Process.hs#L45-L51

shows that System.Process.proc is used to call the process and that function in turn uses a
RawCommand https://hackage-content.haskell.org/package/process-1.6.26.1/docs/src/System.Process.html#proc.

Shell injection attacks like args = {; rm -rf /} should not happen.
I manually tested it and it doesn't cause any issues.

I believe that there would be no shell injection vector.

File exfiltration

In response @tarleb clarified that: #62 (comment)

My main security concern here is the overwriting of files and info exfiltration, as allowing arbitrary arguments would also allow control over the input and output files.

Where I am now wondering again if this is actually an issue.
As long as we pass in the input and output file options last and separate out positional arguments via -- I don't think that files could be arbitrarily written or read (assuming the CLI parsing of the engine works as expected).

For example with d2 if we have following tree:

.
├── example.d2
└── pawned.txt

and ensure that we append to the end with {'--', infile, outfile} there is no way to access pawned.txt.

See:

d2 pawned.txt -- example.d2 out.svg
# we automatically insert `--` plus our file args.
# output: err: bad usage: too many arguments passed
d2 -- pawned.txt -- example.d2 out.svg
# even if the user tries to inject `--` before it breaks the execution
# err: bad usage: too many arguments passed

But to re-iterate: I still think that it is a good idea to disable this feature by default to be sure. I just want to provide some context and show that it isn't as bad as it may seem. If I would read the diagram.lua documentation and saw that this option enables a potential exploit I would probably be "scared" to use it. IMHO it would be nice to show how or when such as exploits could happen. But maybe this is just my view. And I could understand if we should default to "scary warnings" and maybe just document it in the code for the interested users. 🤷

Implementation

Before either @cparadis6191 or I implement the feature, I would like to hear where you (@tarleb) would like to have the additional options. Do you have any additional experience maybe with pandoc quirks? Or would you first like to see an option

[tarleb]

Converters like mermaid and plantuml allow to set --input and/or --output. These tools do no specify what happens if the option is given twice, and it's a bad idea™ to rely on undocumented behavior for security features. Plus, I'd always rather err on the side of caution than expose users to security problems out of convenience.

I think something like this would be nice:

---
diagram:
  allow-arbitrary-converter-options: true
---

[kainctl]

it's a bad idea™ to rely on undocumented behavior for security features.

Fair enough. I guess it is better to be safe than sorry. 👍

---
diagram:
  allow-arbitrary-converter-options: true
---

sounds good. Do you have any opinions/recommendations where the options should be parsed from?
Would it make sense to use the quarto comment functionality?

[tarleb]

No need for parsing, this is handled by some shared code. It should be enough to adjust the compile functions for all formats.

Maybe we can add a new allow_all_user_options property to all formats. This option can then be checked inside the compile function via self.allow_all_user_options, and the global option could set it for all formats. That gives fine-grained control without much of an impact on convenience.

Feel free to use your own names for those properties and options.

[tarleb]

This was mentioned in the PR, just to state it again: pandoc's Markdown doesn't allow element attributes that start with -, so those would need a prefix that would then be stripped in the code. E.g.

``` {opt--my-cli-option=foo}
...
```