denox
denox copied to clipboard
Document workspace file resolution logic
Issue Type
- Other (Documentation)
I see there is resolution logic for this, but it is not yet documented.
Ref. source: https://github.com/BentoumiTech/denox/blob/7774ec1709bdfc5b9dc756a6b4642ac849274455/src/parser/deno_workspace.ts#L12-L27
https://github.com/BentoumiTech/denox/blob/7774ec1709bdfc5b9dc756a6b4642ac849274455/src/const.ts#L3-L14
I think it would be good to reconsider the resolution order based on some rules. This is how ESLint does it. Here is a suggested idea with included reasoning:
(Don't support extensionless deno-workspace
because of ambiguity — but if you feel strongly about this, explain clearly which format(s) are valid without an extension, e.g. yaml
/json
)
-
First, files that begin with
deno-workspace
(not dotfiles because in the event of a conflict, the dotfile is potentially hidden and could be a surprise). -
Next, in the following extension order, preferring more human-readable formats and static files, which offer better security at the cost of flexibility (more on this below):
-
.yaml
-
.yml
-
.json
- (no extension would go here if it is not a script)
-
.ts
-
.js
-
-
Files that begin with
.deno-workspace
following the same extension order as above
So, the suggested order would be:
-
deno-workspace.yaml
-
deno-workspace.yml
-
deno-workspace.json
-
deno-workspace
(if it is not a script) -
deno-workspace.ts
-
deno-workspace.js
-
.deno-workspace.yaml
-
.deno-workspace.yml
-
.deno-workspace.json
-
.deno-workspace
(if it is not a script) -
.deno-workspace.ts
-
.deno-workspace.js
Security
It appears that when evaluating workspace script files, they are evaluated with the permissions inherited from denox
, which is suggested to be --allow-all
at installation. Is that correct? If so, that could be very dangerous.
Static configuration files are easier to reason about than dynamic scripts, so they are easier to secure.
@jsejcksn I like your suggested order.
I'll go further by actually removing the hidden file support as it's better if the workspace file is explicitly visible.
What solution do you think is possible to avoid possible issues when evaluating workspace script files?
What solution do you think is possible to avoid possible issues when evaluating workspace script files?
I think it's not simple, and that all solutions involve trade-offs.
At the most security-oriented end of the spectrum: don't support script config files. At the opposite end of the spectrum: make no changes.
All of the in-between relies on taking a position about what a config script should and shouldn't be allowed to do.
You could evaluate and serialize the export from the config in a sub-process (Deno.run
), providing only the appropriate permissions. Determining what those permissions should be is the real question.
For example: providing --allow-run
is the same as providing all permissions, so I think that should not be allowed.
Should a config script be able to write to the filesystem? I don't think config files should create filesystem side-effects, so --allow-write
is also not appropriate, IMO.
Is reading files ok (--allow-read
)? Probably. Should it be limited to the current directory and its children? Perhaps—I don't know.
Is accessing the network ok? Providing this permission would allow a malicious config to exfiltrate data. I can imagine how there might be a legitimate use case for this permission, but I have never seen an example of one.
What do you think? Take a look at the other permissions, too.
I actually like your idea of evaluating and exporting in a sub-process.
Regarding the permissions, I think allow-env
and allow-read
are a good starting point that would cover 90% of the use cases. I would rather add more permissions in the future if it makes sense than remove some.