fix: guard identifier lookups against object prototype collisions

.changeset/clean-cats-guard.md

@@ -0,0 +1,7 @@
+---
+'@openfn/runtime': patch
+'@openfn/engine-multi': patch
+'@openfn/ws-worker': patch
+---
+
+Handle step and adaptor identifiers that match built-in object properties.

packages/engine-multi/src/api/autoinstall.ts

@@ -14,6 +14,9 @@ import { AUTOINSTALL_COMPLETE, AUTOINSTALL_ERROR } from '../events';
 import { AutoinstallError } from '../errors';
 import ExecutionContext from '../classes/ExecutionContext';
 
+const hasOwn = (target: object, key: PropertyKey) =>
+  Object.prototype.hasOwnProperty.call(target, key);
+
 // none of these options should be on the plan actually
 export type AutoinstallOptions = {
   skipRepoValidation?: boolean;
@@ -122,7 +125,7 @@ const autoinstall = async (context: ExecutionContext): Promise<ModulePaths> => {
   }
 
   const adaptors = Array.from(identifyAdaptors(plan));
-  const paths: ModulePaths = {};
+  const paths = new Map<string, ModulePaths[string]>();
 
   const adaptorsToLoad = [];
   for (const a of adaptors) {
@@ -148,8 +151,13 @@ const autoinstall = async (context: ExecutionContext): Promise<ModulePaths> => {
     const alias = getAliasedName(resolvedAdaptorName);
 
     // Write the adaptor version to the context for reporting later
-    if (!context.versions[name]) {
-      context.versions[name] = [];
+    if (!hasOwn(context.versions, name)) {
+      Object.defineProperty(context.versions, name, {
+        value: [],
+        configurable: true,
+        enumerable: true,
+        writable: true,
+      });
     }
     if (!context.versions[name].includes(v)) {
       (context.versions[name] as string[]).push(v);
@@ -161,10 +169,10 @@ const autoinstall = async (context: ExecutionContext): Promise<ModulePaths> => {
     }
 
     // important: write back to paths with the RAW specifier
-    paths[a] = {
+    paths.set(a, {
       path: `${repoDir}/node_modules/${alias}`,
       version: v,
-    };
+    });
 
     if (!(await isInstalledFn(resolvedAdaptorName, repoDir, logger))) {
       adaptorsToLoad.push(resolvedAdaptorName);
@@ -175,11 +183,16 @@ const autoinstall = async (context: ExecutionContext): Promise<ModulePaths> => {
   for (const step of plan.workflow.steps) {
     const job = step as unknown as Job;
     for (const adaptor of job.adaptors ?? []) {
-      if (paths[adaptor!]) {
+      const modulePath = paths.get(adaptor!);
+      if (modulePath) {
         const { name } = getNameAndVersion(adaptor!);
         job.linker ??= {};
-        // @ts-ignore
-        job.linker[name] = paths[adaptor!];
+        Object.defineProperty(job.linker, name, {
+          value: modulePath,
+          configurable: true,
+          enumerable: true,
+          writable: true,
+        });
       }
     }
   }
@@ -196,11 +209,11 @@ const autoinstall = async (context: ExecutionContext): Promise<ModulePaths> => {
       if (err) {
         throw err;
       }
-      return paths;
+      return Object.fromEntries(paths);
     });
   }
 
-  return paths;
+  return Object.fromEntries(paths);
 };
 
 export default autoinstall;
@@ -232,7 +245,7 @@ const isInstalled = async (
   const pkg = await loadRepoPkg(repoDir);
   if (pkg) {
     const { dependencies } = pkg;
-    return dependencies.hasOwnProperty(alias);
+    return hasOwn(dependencies, alias);
   }
 };
 

packages/engine-multi/test/api/autoinstall.test.ts

@@ -92,6 +92,27 @@ test('Autoinstall basically works', async (t) => {
   });
 });
 
+test('autoinstall supports an adaptor named constructor', async (t) => {
+  const context = createContext(
+    {
+      handleInstall: mockHandleInstall,
+      handleIsInstalled: async () => false,
+    },
+    [{ adaptors: ['constructor@1.0.0'] }],
+    [/constructor/]
+  );
+
+  const paths = await autoinstall(context);
+
+  t.deepEqual(paths, {
+    'constructor@1.0.0': {
+      path: 'tmp/repo/node_modules/constructor_1.0.0',
+      version: '1.0.0',
+    },
+  });
+  t.deepEqual(context.versions.constructor, ['1.0.0']);
+});
+
 test('mock is installed: should be installed', async (t) => {
   const isInstalled = mockIsInstalled({
     name: 'repo',

packages/runtime/src/execute/compile-plan.ts

@@ -15,14 +15,29 @@ import { getNameAndVersion } from '../modules/repo';
 // map special condition strings to JS expressions
 // The special strings are generated by lightning, and are useful convenience for local dev
 export const conditions: Record<string, string> = {
-  on_job_success: 'Boolean(!state?.errors?.[upstreamStepId] ?? true)',
-  on_job_failure: 'Boolean(state?.errors && state.errors[upstreamStepId])',
+  on_job_success:
+    'Boolean(!state?.errors || !Object.prototype.hasOwnProperty.call(state.errors, upstreamStepId) || !state.errors[upstreamStepId])',
+  on_job_failure:
+    'Boolean(state?.errors && Object.prototype.hasOwnProperty.call(state.errors, upstreamStepId) && state.errors[upstreamStepId])',
   always: 'true',
 };
 // create a couple of aliases for future reference
 conditions.on_upstream_success = conditions.on_job_success;
 conditions.on_upstream_fail = conditions.on_job_failure;
 
+// Assignment to __proto__ invokes a setter on ordinary objects. Define lookup
+// entries as own data properties so external ids cannot collide with built-ins.
+const defineOwn = (target: object, key: string, value: unknown) =>
+  Object.defineProperty(target, key, {
+    value,
+    configurable: true,
+    enumerable: true,
+    writable: true,
+  });
+
+const hasOwn = (target: object, key: PropertyKey) =>
+  Object.prototype.hasOwnProperty.call(target, key);
+
 const compileEdges = (
   from: string,
   edges: string | Record<string, boolean | StepEdge>,
@@ -41,13 +56,13 @@ const compileEdges = (
     try {
       const edge = edges[edgeId];
       if (typeof edge === 'boolean') {
-        result[edgeId] = edge;
+        defineOwn(result, edgeId, edge);
       } else if (typeof edge === 'string') {
-        result[edgeId] = {
+        defineOwn(result, edgeId, {
           condition: compileFunction(mapCondition(edge), context, [
             'upstreamStepId',
           ]),
-        };
+        });
       } else {
         const newEdge = {
           ...edge,
@@ -59,7 +74,7 @@ const compileEdges = (
             ['upstreamStepId']
           );
         }
-        result[edgeId] = newEdge as CompiledEdge;
+        defineOwn(result, edgeId, newEdge as CompiledEdge);
       }
     } catch (e: any) {
       errs.push(
@@ -87,7 +102,7 @@ const findUpstream = (workflow: Workflow, id: string) => {
         if (job.next === id) {
           return job.id;
         }
-      } else if (job.next[id]) {
+      } else if (hasOwn(job.next, id) && job.next[id]) {
         return job.id;
       }
   }
@@ -141,7 +156,7 @@ export default (plan: ExecutionPlan) => {
 
   const maybeAssign = (a: any, b: any, keys: Array<keyof Job>) => {
     keys.forEach((key) => {
-      if (a.hasOwnProperty(key)) {
+      if (hasOwn(a, key)) {
         b[key] = a[key];
       }
     });
@@ -169,7 +184,7 @@ export default (plan: ExecutionPlan) => {
       newStep.linker ??= {};
       for (const adaptor of job.adaptors!) {
         const { name, version } = getNameAndVersion(adaptor);
-        newStep.linker[name] = { version: version! };
+        defineOwn(newStep.linker, name, { version: version! });
       }
     }
 
@@ -179,7 +194,7 @@ export default (plan: ExecutionPlan) => {
       });
     }
     newStep.previous = findUpstream(workflow, stepId);
-    newPlan.workflow.steps[stepId] = newStep;
+    defineOwn(newPlan.workflow.steps, stepId, newStep);
   }
 
   if (errs.length) {

packages/runtime/test/execute/compile-plan.test.ts

@@ -144,6 +144,18 @@ test('should convert steps to an object', (t) => {
   t.is(workflow.steps.b.expression, 'y');
 });
 
+test('should preserve a step whose id is __proto__', (t) => {
+  const { workflow } = compilePlan({
+    workflow: {
+      steps: [{ id: '__proto__', expression: 'x' }],
+    },
+  });
+
+  t.deepEqual(Object.keys(workflow.steps), ['__proto__']);
+  t.is(workflow.steps.__proto__.id, '__proto__');
+  t.is(workflow.steps.__proto__.expression, 'x');
+});
+
 test('should set previous job with 2 steps', (t) => {
   const plan: ExecutionPlan = {
     workflow: {

packages/ws-worker/src/api/reasons.ts

@@ -3,6 +3,9 @@ import { Step } from '@openfn/runtime';
 import { ExitReason, ExitReasonStrings } from '@openfn/lexicon/lightning';
 import type { RunState } from '../types';
 
+const hasOwn = (target: object, key: PropertyKey) =>
+  Object.prototype.hasOwnProperty.call(target, key);
+
 // This takes the result state and error from the job
 const calculateJobExitReason = (
   jobId: string,
@@ -18,7 +21,11 @@ const calculateJobExitReason = (
     reason = error.severity ?? 'crash';
     error_message = error.message;
     error_type = error.subtype || error.type || error.name;
-  } else if (state.errors?.[jobId]) {
+  } else if (
+    state.errors &&
+    hasOwn(state.errors, jobId) &&
+    state.errors[jobId]
+  ) {
     reason = 'fail';
     ({ message: error_message, name: error_type } = state.errors[jobId]);
   }

packages/ws-worker/test/api/reasons.test.ts

@@ -32,6 +32,14 @@ test('still success if a prior job has errors', (t) => {
   t.is(r.error_message, null);
 });
 
+test('built-in object properties are not treated as job errors', (t) => {
+  const r = calculateJobExitReason('constructor', { errors: {} } as any);
+
+  t.is(r.reason, 'success');
+  t.is(r.error_type, null);
+  t.is(r.error_message, null);
+});
+
 test('fail', (t) => {
   const jobId = 'a';
   const state: any = {