Dev to main v0.2

[r4ulcl]

No description provided.

To fix the problem, the password must not appear in logs. The safest general approach is to either (a) avoid logging the DSN entirely or (b) log a redacted version that excludes or masks the password. We can preserve existing debug functionality (seeing what host, port, database, user are being used) while ensuring the password is never logged.

The single best fix here is to change the debug logging to log a DSN-like string that omits or masks the password. For example, we can build a separate safeDSN string that replaces the password portion with a placeholder like "***" or excludes it entirely, and log that instead of dsn. This maintains functionality (developers can still see connection parameters when debugging) without leaking credentials.

Concretely, in manager/database/DB.go around line 76–87, we will:

• Keep the existing dsn construction unchanged (it’s needed for sql.Open).
• Introduce a new safeDSN variable for logging, constructed with the same fmt.Sprintf but replacing password with a constant such as "***".
• Change the log.Println call to log safeDSN instead of dsn.

No new imports or helper methods are needed; fmt is already imported and sufficient.

In general, the problem is fixed by replacing ssh.InsecureIgnoreHostKey() with a callback that validates the server’s host key against an allow‑list. The simplest and safest pattern in Go is to load one or more trusted public host keys from files (or another configuration source), parse them with ssh.ParseAuthorizedKey or ssh.ParsePublicKey, and then either (a) use ssh.FixedHostKey if there is exactly one expected key, or (b) implement a HostKeyCallback that compares the server’s presented key against the list of allowed keys and rejects unknown keys.

For this codebase, we should keep existing functionality (using the private key file and optional password) and only change how host keys are verified. Since we can’t assume additional fields on utils.ManagerSSHConfig, the least invasive change is to derive an expected host public key from a known file path. A common convention with OpenSSH is that for a private key path like /path/id_rsa, the corresponding public key is /path/id_rsa.pub. We can follow this convention locally within this file: construct the public‑key path by appending .pub to config.PrivateKeyPath, read and parse that as an authorized key, and use ssh.FixedHostKey to set HostKeyCallback. If reading/parsing the public key fails, we should log a fatal error, mirroring the existing fatal behavior on the private key.

Concretely, inside StartSSH in manager/sshTunnel/sshTunnel.go, we will:

• After successfully loading the private key (auth), derive the public key path (config.PrivateKeyPath + ".pub").
• Read the public key file with os.ReadFile.
• Parse it with ssh.ParseAuthorizedKey to get an ssh.PublicKey.
• Set HostKeyCallback: ssh.FixedHostKey(hostPublicKey) instead of ssh.InsecureIgnoreHostKey().
  No new imports are required, as os and ssh are already imported. This keeps authentication behavior the same while adding strong host key verification.

In general, the fix is to ensure that any path derived (even indirectly) from websocket data is validated or constrained before being passed to filesystem APIs such as os.MkdirAll, os.WriteFile, and os.Remove. The usual approaches are: (1) enforce that the path is a single filename (no separators / ..), or (2) resolve it against a configured safe root directory and verify that the resulting absolute path is still under that root.

Here, RemoteFilePath can contain directories (you are calling filepath.Dir on it), so we should use the “safe root directory” approach. A minimal, non‑breaking change is:

• Introduce a helper inside modules.go to map any provided RemoteFilePath into a safe directory. A simple choice (consistent with the existing code) is to root all paths under the process’s current working directory:
  • Compute safeBase, an absolute path (e.g. os.Getwd() once per call).
  • For each file.RemoteFilePath, join it with safeBase and normalize via filepath.Abs.
  • Verify that the resulting absolute path has safeBase as a prefix (taking care with separators).
  • If the check fails, return an error and skip processing/deletion for that file.
• Use the validated absolute path both for directory creation and writing the file in ProcessFiles, and for deletion in DeleteFiles. That way, even if an attacker passes "../../etc/passwd" or "/root/.ssh/authorized_keys", the normalized path will either be rejected or confined under the safe base.

Concretely, in worker/modules/modules.go:

• Add a small helper getSafeBaseDir() that returns os.Getwd() and caches/uses it per call to avoid recomputing it in every loop iteration.
• Add a helper sanitizePathUnsafe(baseDir, relativePath string) (string, error) which:
  • joined := filepath.Join(baseDir, relativePath)
  • abs, err := filepath.Abs(joined)
  • ensures abs starts with baseDir + string(os.PathSeparator) or equals baseDir
  • returns an error if not.
• In ProcessFiles, before using path := file.RemoteFilePath, compute safePath, err := sanitizePathUnsafe(baseDir, path) and use safePath for getDirectory, MkdirAll, WriteFile, and in logs.
• In DeleteFiles, do the same: compute safePath from file.RemoteFilePath, delete safePath, and log that path; if sanitization fails, return an error or at least skip that file.

No new imports are needed; all required functions (os.Getwd, filepath.Abs, filepath.Join) are already available via existing imports.

In general, to fix uncontrolled path usage you must ensure that any path derived from untrusted data is either (a) validated as a single safe filename (no separators, no ..), or (b) resolved relative to a trusted base directory and checked to remain within that directory after normalization. For this worker, it is more flexible and backward-compatible to keep supporting subdirectories but force all paths to live under a known safe root (e.g., a worker’s base working directory from config).

The best fix here is to change ProcessFiles so that it no longer uses file.RemoteFilePath directly. Instead:

1. Decide a safe base directory (we must rely only on code we see; utils.WorkerConfig is available but we don’t see its fields, so we’ll conservatively use the current working directory as a base and enforce that all paths stay within it).
2. For each file.RemoteFilePath:
   • Reject empty paths.
   • Clean it with filepath.Clean.
   • Reject any absolute path (filepath.IsAbs(cleaned)).
   • Build an absolute path under the safe base: absPath := filepath.Join(baseDir, cleaned) and then absPath, err = filepath.Abs(absPath).
   • Ensure absPath is still within baseDir by prefix check (after obtaining baseDirAbs and appending a trailing separator).
3. Use absPath for directory creation and writing, instead of the raw path.
4. Keep logging using the original path or the resolved absPath as appropriate.

We will implement this entirely inside ProcessFiles in worker/modules/modules.go, adding local variables for baseDirAbs and absPath, and update lines 204–207 and 211–212 to use absPath instead of the unvalidated path. No new imports are required because filepath and strings are already imported.

In general, to fix uncontrolled-path issues, derive a normalized absolute path from a trusted base directory plus the user-supplied path, then verify that the resulting path is still within that base directory. Reject or skip the operation if the check fails. Apply this consistently for all file operations (create, write, delete) that use user-controlled path data.

For this codebase, the minimal, non-breaking fix is:

1. Define a helper function in worker/modules/modules.go that:

   • Takes a *utils.WorkerConfig (to get a base directory; if there is a suitable field in the real code, use it; since we cannot see it, we’ll assume a field like WorkDir exists and fall back safely if empty).
   • Takes the untrusted relative path (file.RemoteFilePath).
   • Joins it with the base directory, obtains an absolute path using filepath.Abs, and checks that:
     • It’s under the base directory (by checking HasPrefix against a cleaned absolute base, taking care to include a path separator).
   • Returns the safe absolute path or an error.

2. In ProcessFiles:

   • Replace direct use of file.RemoteFilePath with the validated safe path from the helper.
   • Use that safe path to compute the directory (getDirectory) and to call os.MkdirAll and os.WriteFile.
   • Return a clear error if validation fails.

3. In DeleteFiles:

   • Change the signature to accept config *utils.WorkerConfig (so we can reuse the same validation).
   • Replace direct use of file.RemoteFilePath in os.Remove with the validated safe path from the helper.
   • Log or return an error when validation fails or deletion fails.

4. In worker/process/processTask.go:

   • Update the call to modules.DeleteFiles to pass config.

Concretely:

• Add a new helper in modules.go, e.g.:

  func getSafePath(config *utils.WorkerConfig, relPath string) (string, error) {
      baseDir := config.WorkDir // or another existing field; fallback to current dir if empty
      if baseDir == "" {
          baseDir = "."
      }
      baseAbs, err := filepath.Abs(baseDir)
      if err != nil {
          return "", fmt.Errorf("failed to resolve base directory: %w", err)
      }
      candidate := filepath.Join(baseAbs, relPath)
      candidateAbs, err := filepath.Abs(candidate)
      if err != nil {
          return "", fmt.Errorf("failed to resolve path %q: %w", relPath, err)
      }
  
      // Ensure candidateAbs is within baseAbs
      baseWithSep := baseAbs
      if !strings.HasSuffix(baseWithSep, string(os.PathSeparator)) {
          baseWithSep += string(os.PathSeparator)
      }
      if candidateAbs != baseAbs && !strings.HasPrefix(candidateAbs, baseWithSep) {
          return "", fmt.Errorf("path %q escapes base directory", relPath)
      }
      return candidateAbs, nil
  }

• Modify ProcessFiles:

  • After path := file.RemoteFilePath, call safePath, err := getSafePath(config, path) and use safePath instead of path in getDirectory, MkdirAll, WriteFile, and logging.

• Modify DeleteFiles:

  • Change signature to func DeleteFiles(task *globalstructs.Task, config *utils.WorkerConfig, verbose, debug bool) error.
  • For each file, call safePath, err := getSafePath(config, path) and use safePath in os.Remove and logging.

• Modify Task in processTask.go:

  • Change modules.DeleteFiles(task, verbose, debug) to modules.DeleteFiles(task, config, verbose, debug).

This keeps existing behavior for valid in-sandbox paths, while preventing arbitrary filesystem access and addressing the CodeQL alert.

---