OpenFn / OpenFn/kit

Linker: more controls and better errors

Open
#364 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

security
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

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.