feat(CHAIN-3050): add ci

[jackchuma]

No description provided.

[linear]

CHAIN-3050 Add CI checks to signer tool repo

In general, user-controlled input used to build filesystem paths must be normalized (e.g., via path.resolve/fs.realpath) and then checked to ensure the resulting path stays within a trusted root directory before being passed to filesystem APIs. When a function is exported and might be reused from elsewhere, you should not rely solely on upstream callers to perform these checks; instead, validate inside the function as well.

For this codebase, the best fix is to make createDeterministicTarball self‑contained with respect to path safety:

• Require that allowedDir be provided; otherwise throw an error instead of silently proceeding with potentially unbounded paths.
• Normalize allowedDir itself (realpath) so comparisons are reliable, and then normalize taskFolderPath relative to that root if not already done.
• Ensure all derived paths (resolvedTaskFolderPath, libPath, and the generated tarball path) are validated with assertWithinDir before any fs operations.
• Avoid writing the tarball into an uncontrolled location; instead, place it within the validated root (e.g., under resolvedTaskFolderPath) so that even if process.cwd() is something unexpected, output remains confined.

Concretely, in src/lib/task-origin-validate.ts:

• Strengthen createDeterministicTarball so it:
  • Throws if allowedDir is missing.
  • Normalizes allowedDir via fs.realpath.
  • Normalizes taskFolderPath using fs.realpath only after constructing it under allowedDir and checking with assertWithinDir.
  • Derives libPath and a new tarballPath under resolvedTaskFolderPath and validates both with assertWithinDir before using them.
• Adjust tarball output location to be inside resolvedTaskFolderPath (or another validated directory under allowedDir) instead of using process.cwd() without checks.

These changes keep existing functional behavior (creating a deterministic tarball of the task folder and checking lib/ for reproducibility) while enforcing that all filesystem activity remains within the configured allowedDir.

General approach: ensure any path that can be influenced by user input is normalized and verified to lie within an allowed root directory before being passed to file-system APIs. Even if upstream code intends to always pass safe values, placing the check at the boundary (just before fs.readFile) avoids future misuse and satisfies the static analyzer.

Best concrete fix here:

• buildAndValidateSignature receives TaskOriginVerifyOptions with an optional allowedDir. Today it only enforces assertWithinDir if allowedDir is truthy and trusts the caller to supply it.
• We can make buildAndValidateSignature robust by:
  1. Treating allowedDir as required for safe use (at least for signatureFile), and
  2. Resolving and validating the real path of signatureFile against allowedDir before reading it.
• This mirrors the recommended pattern: normalize (via path.resolve / fs.realpath) then check containment (assertWithinDir), and only then call fs.readFile.

Concretely, in src/lib/task-origin-validate.ts:

1. In buildAndValidateSignature, before fs.readFile(signatureFile, 'utf8'):
   • If allowedDir is not provided, throw an error (instead of silently skipping validation) to prevent unsafe use of this function.
   • Use const resolvedSignatureFile = await fs.realpath(signatureFile); to resolve symlinks.
   • Call assertWithinDir(resolvedSignatureFile, allowedDir);.
   • Use resolvedSignatureFile for fs.readFile and the subsequent fs.stat in verifyTaskOrigin if you want to be fully consistent, but to minimize behavior changes we’ll at least ensure the read in buildAndValidateSignature uses the validated path.
2. Keep the existing assertWithinDir in verifyTaskOrigin and validateSigner as defense-in-depth; we’re just adding an extra guarantee at the actual sink.

No new imports are required: fs (promises) and path are already imported, and assertWithinDir already exists. All changes occur inside the provided snippet in src/lib/task-origin-validate.ts.

---

General approach: Ensure that any filesystem operations on paths influenced by user input are performed only after validating those paths against a trusted root or sanitizing them. Here, the best fit is “containment within a safe root” using assertWithinDir.

Concrete fix for this codebase:

• In verifyTaskOrigin (src/lib/task-origin-validate.ts), normalize and validate taskFolderPath and signatureFile before using them in fs.stat, even when allowedDir is not provided or caller forgot to validate.
• We already have assertWithinDir imported; we should make use of it unconditionally. Since verifyTaskOrigin is always called from validateSigner with allowedDir: CONTRACT_DEPLOYMENTS_ROOT today, strengthening the checks inside verifyTaskOrigin will not change existing behavior for current callers but will harden the function against any future misuse.
• The simplest, non‑breaking change is:
  • If allowedDir is provided, resolve both paths and validate them with assertWithinDir before any filesystem calls (this is already done, but we should base all subsequent operations on the validated values).
  • If allowedDir is not provided, still normalize the paths via path.resolve before passing them to fs.stat. This mitigates some issues and avoids odd relative paths, although without an allowed root there’s no strong containment. However, the main security invariant for this app is that callers should pass allowedDir; we’ll enforce that at compile-time by leaving the type as-is but making the runtime behavior safer.
• To keep behavior identical while satisfying CodeQL, we can take a stronger step: once allowedDir is provided and the check passes, overwrite taskFolderPath and signatureFile with their resolved versions and only use those from then on. This ensures we’re operating on normalized, validated paths and makes the data flow clearer to the analyzer.

Specific edits (all in src/lib/task-origin-validate.ts):

• Introduce new local variables validatedTaskFolderPath and validatedSignatureFile that, when allowedDir is set, are the outputs of assertWithinDir (assuming it returns the normalized path; if it currently returns void, we can instead call path.resolve ourselves before and after). Because we cannot change assertWithinDir here, we’ll implement normalization directly but still call assertWithinDir for the security check.
• Use these validated variables in both fs.stat calls instead of the raw taskFolderPath / signatureFile so the sink sees only paths that have passed validation.

No new imports or external dependencies are required; we already import path and assertWithinDir.

General approach: Before using any path derived from user-controlled data in filesystem operations, normalize the path and enforce that it resides within an allowed root directory. In this codebase, that is exactly what assertWithinDir is meant to do. We should call it in verifyTaskOrigin with the allowedDir for all file system uses of taskFolderPath and signatureFile, regardless of whether upstream callers already validated them.

Best concrete fix here: strengthen verifyTaskOrigin so that it always validates taskFolderPath and signatureFile against allowedDir at the point of use, right before the fs.stat calls. This makes verifyTaskOrigin self-contained in terms of security: any caller that passes allowedDir gets guaranteed validation, and the static analysis tool will see the validation happening in the same function as the sink.

Specific changes in src/lib/task-origin-validate.ts:

• In verifyTaskOrigin, keep the existing early validation block:

  // Validate paths are within the allowed directory if specified
  if (allowedDir) {
    assertWithinDir(taskFolderPath, allowedDir);
    assertWithinDir(signatureFile, allowedDir);
  }

• Additionally, before calling fs.stat(taskFolderPath) and fs.stat(signatureFile), re-assert the directory constraint if allowedDir is provided. This is a no-op in terms of functionality (since the earlier checks should already pass) but clarifies to both humans and tools that, just before the filesystem operation, the path is known to be safe.

That means:

• Just before const taskStats = await fs.stat(taskFolderPath);, insert:

    if (allowedDir) {
      assertWithinDir(taskFolderPath, allowedDir);
    }

• Just before const sigStats = await fs.stat(signatureFile);, insert:

    if (allowedDir) {
      assertWithinDir(signatureFile, allowedDir);
    }

No new imports or types are required; assertWithinDir is already imported at the top of task-origin-validate.ts, and we are not changing function signatures or external behavior.

General approach: ensure any filesystem path derived (even indirectly) from user-controlled data is (a) resolved/normalized and (b) required to lie within a designated safe root before performing operations like realpath, readdir, stat, or tar creation. Avoid optional security parameters where possible: make allowedDir mandatory for functions that walk directories or create tarballs, and validate inputs as early as possible.

Best concrete fix here:

1. Make allowedDir mandatory for getAllFilesRecursively and createDeterministicTarball. That way, these functions can always enforce a root directory and cannot be accidentally called in an unsafe way.
2. In getAllFilesRecursively, call assertWithinDir(dir, allowedDir) before fs.realpath(dir). This ensures that even the initial path string (before symlink resolution) is anchored under the allowed root, avoiding using completely arbitrary dir with realpath. Then call fs.realpath and validate the resolved path again if desired (we’re already validating currentDir after realpath today).
3. In createDeterministicTarball, make allowedDir required and move assertWithinDir(taskFolderPath, allowedDir) before the fs.realpath call, mirroring the same pattern: validate the user-influenced path string, then normalize it, then validate subsequent derived paths (libPath etc.) against the same root.
4. Update the call site in buildAndValidateSignature to match the new createDeterministicTarball signature (no optional allowedDir), and keep the pre-existing assertWithinDir checks intact.

This preserves existing behavior for the current caller (which already always passes allowedDir), but makes the safety invariant local to the helpers and eliminates the CodeQL warning by ensuring untrusted data is never used with fs.realpath before being range-checked.

---

In general, to fix uncontrolled path usage when walking directories recursively, you should (1) normalize each directory path (e.g., using fs.realpath or path.resolve) and (2) ensure it is still within an allowed root directory before performing filesystem operations like readdir or recursing further. This prevents an attacker from leveraging symlinks or crafted directory names under a trusted root to escape to arbitrary locations.

The best minimally invasive fix here is to ensure that getAllFilesRecursively enforces the allowedDir constraint on every directory it recurses into, not just the initial dir. We can do this by: (a) always normalizing dir at the beginning, (b) calling assertWithinDir on that normalized path whenever allowedDir is supplied, and (c) when recursing into fullPath, passing fullPath as the dir argument, letting the next call repeat normalization and validation before calling fs.readdir. Concretely, in src/lib/task-origin-validate.ts within getAllFilesRecursively, we should change the logic around currentDir so that fs.realpath is always used to normalize dir, and the assertWithinDir check applies to that normalized path, then fs.readdir uses the normalized currentDir. The recursion already calls getAllFilesRecursively(fullPath, baseDir, allowedDir), so once we normalize and validate dir consistently at the top of the function, all recursive steps will be covered. No changes are necessary in validation-service.ts or the API handler for this specific issue, and no new external dependencies are required.

In general, the problem is that createDeterministicTarball trusts its taskFolderPath argument and only conditionally normalizes and validates it based on an optional allowedDir. Static analysis correctly shows that this argument can originate from user input. The safest fix is to ensure that createDeterministicTarball always performs normalization and directory containment validation, or at least refuses to operate if no allowedDir is provided, instead of silently proceeding with a raw, potentially attacker-controlled path.

Best concrete fix without changing external behavior for existing callers:

• Require that allowedDir be provided when creating a tarball and enforce it inside createDeterministicTarball.
• Normalize taskFolderPath with fs.realpath and check it (and any derived important subpaths such as libPath) using assertWithinDir, even when the caller has already performed checks. This is cheap and removes any reliance on caller discipline.
• If allowedDir is not provided, throw an error rather than proceeding with an unconstrained filesystem operation.

This only requires changes within src/lib/task-origin-validate.ts:

1. In createDeterministicTarball:

   • At the top of the function, check if allowedDir is provided; if not, throw a descriptive error.
   • Remove the conditional “if (allowedDir)” branch and instead always:
     • Resolve taskFolderPath with fs.realpath.
     • Call assertWithinDir on the resolved path and allowedDir.
   • For the libPath, we already call assertWithinDir when allowedDir is set; after the change, that guard will always run because allowedDir is required.

2. In buildAndValidateSignature:

   • Before calling createDeterministicTarball, ensure allowedDir is present; if not, throw an error. This is defensive and makes the requirement explicit at the only call site we see.

No new helpers or imports are needed; we reuse fs.realpath and assertWithinDir which are already imported in this file.