Linker: more controls and better errors
Nobody has claimed this yet.
- Dominant language
- TypeScript
- Stars
- 21
- Forks
- 23
- Avg merge
- 1d 22h
- Merged PRs (30d)
- 17
Description
The runtime currently supports import statements. It has to - that's how it works!
All imports are handled by our linker function, which will attempt to load the module.
The linker will run through a few steps:
- It will check to see if there's a whitelist set and, if so, ensure that the module is on the whitelist of allowed modules
- If all is OK, work out the path to the desired module:
-
- If the specifier is am absolute path to a js file, just use that path
-
- If the linker has an explicit mapping for that path (which can be set on linker options), use that path
-
- See if the module exists in the repo, and return that path
- If no path was mapped, throw an error
- Finally, try to
import()the resolved path and inject the module interface into the job sandbox.
Note that nested imports will just work - it's on the imports in job code that are directly process
Also note that the runtime or linker will make no attempt to install modules. We're at the mercy of node's standard module lookup algorithm when we call import().
This is fairly secure but kind of by accident. If I try to import fs from 'node:fs', it'll fail because node:fs` happens to not be in the repo. When really we should be throwing a message like "cannot import native node modules" or "node:fs forbidden".
Also the devx of error messages could be a lot better. Here's the error for node:fs:
file:///home/jclark/.asdf/installs/nodejs/18.12.1/lib/node_modules/@openfn/cli/node_modules/@openfn/runtime/dist/index.js:265
throw new Error(`Failed to load module "${specifier}"`);
^
Error: Failed to load module "node:util"
at loadActualModule (file:///home/jclark/.asdf/installs/nodejs/18.12.1/lib/node_modules/@openfn/cli/node_modules/@openfn/runtime/dist/index.js:265:9)
at async linker (file:///home/jclark/.asdf/installs/nodejs/18.12.1/lib/node_modules/@openfn/cli/node_modules/@openfn/runtime/dist/index.js:202:19)
at async file:///home/jclark/.asdf/installs/nodejs/18.12.1/lib/node_modules/@openfn/cli/node_modules/@openfn/runtime/dist/index.js:279:22
at async ModuleWrap.<anonymous> (node:internal/vm/module:315:24)
at async Promise.all (index 1)
at async SourceTextModule.<computed> (node:internal/vm/module:333:11)
at async SourceTextModule.link (node:internal/vm/module:199:5)
at async module_loader_default (file:///home/jclark/.asdf/installs/nodejs/18.12.1/lib/node_modules/@openfn/cli/node_modules/@openfn/runtime/dist/index.js:277:3)
at async prepareJob (file:///home/jclark/.asdf/installs/nodejs/18.12.1/lib/node_modules/@openfn/cli/node_modules/@openfn/runtime/dist/index.js:396:21)
at async file:///home/jclark/.asdf/installs/nodejs/18.12.1/lib/node_modules/@openfn/cli/node_modules/@openfn/runtime/dist/index.js:361:35
Some practical steps we should take here:
- Support a blacklist. Maybe I want to enable all modules to be importrted apart from node:fs.
- Also a black/whitelist should be an arrays of regex and strings.
- Ensure the CLI can set a whitelist through arguments - I don' think it can right now
- Ensure that Lightning sets a whitelist
- If a module failed to load, report a better error
-
- That means treat it like any job error (ie, write the error to state and carry on the workflow), and don't throw an ugly stacktrace
- The CLI and runtime manager should be able to set an option to disable path loading - this seems like it could be exploited
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start at the runtime linker function and trace how whitelist, module-path, and import failures move through the runtime manager and CLI option handling. Review the Lightning integration mentioned in the issue as well. Done means blacklist and regex/string controls, CLI and Lightning whitelist support, optional path loading, and clearer job-level errors are all covered.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- node.js, typescript
- Domain
- backend, cli, security
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 25/100