v4 preview

[boy-hack]

No description provided.

In general, to fix uncontrolled path usage, you must ensure that any path built from user-controlled data is (1) syntactically safe (no traversal/meta characters) and (2) resolved to an absolute path that is verified to remain under a known safe root directory. This should be done as close as possible to where the path is used with filesystem APIs (os.MkdirAll, os.Open, etc.), or by centralizing path construction in helper functions that enforce these rules.

For this handler, the best fix with minimal functional change is:

1. Harden getAgentUserDir so that it returns an absolute, normalized path under a fixed safe directory derived from AgentConfigRoot, and refuses (by returning a safe error value) if resolution fails or escapes that directory.
2. Update HandleSaveAgentConfig to use the (now safer) getAgentUserDir and handle possible errors before calling os.MkdirAll.
3. Keep the existing validateUsername logic but do not rely on it as the only defense. Instead of silently substituting PublicUser when validation fails, explicitly reject the request, which both eliminates surprising behavior and ensures username is always of a safe form.

Concretely, within common/websocket/knowledge2_api.go:

• Change getAgentUserDir to return (string, error) and internally:
  • Resolve AgentConfigRoot to an absolute path.
  • Join it with username.
  • Call filepath.Clean and filepath.Abs on the result.
  • Verify the final absPath has the safe-root prefix (using strings.HasPrefix on a normalized root with trailing separator).
  • On any failure, return an error.
• Update HandleSaveAgentConfig so that:
  • It rejects invalid usernames (using validateUsername) with a 400 instead of forcing PublicUser.
  • It calls the new getAgentUserDir and, if it returns an error, responds with a 400/500 and does not call os.MkdirAll.
  • Use the returned safe directory path for os.MkdirAll.

These changes only touch the shown code and reuse existing imports (filepath, strings, fmt, errors), so no new imports are necessary.

In general, the fix is to ensure that any user-controlled component used to build a filesystem path is validated or constrained so it cannot escape an intended root directory. That means: (1) validating that username is safe before using it to construct userDir, and/or (2) resolving the final candidate path(s) to absolute paths and checking that they are contained within the intended base directory (derived from AgentConfigRoot).

The most targeted and robust change, without altering higher-level behavior, is to harden resolveAgentConfigPathForWrite. This function is where a user-influenced username is converted into a directory and where the final path is computed before being returned to the caller and passed into os.WriteFile. We can:

1. Ensure username is valid by reusing the existing validateUsername check and falling back to PublicUser if it fails (mirroring what HandleSaveAgentConfig already does).
2. Normalize and constrain the generated paths so they always reside under the intended base directory. To do this, we compute a base directory as baseDir := filepath.Join(AgentConfigRoot) (optionally Abs if desired), then for each candidate file (userDir/name.yaml and .yml) we compute the absolute path and verify that it is still inside baseDir using strings.HasPrefix(absPath, baseDir+string(os.PathSeparator)) or an equivalent safe-prefix check. If the check fails, return an error instead of allowing writing outside the base directory.

Concretely, in common/websocket/knowledge2_api.go, you only need to modify the body of resolveAgentConfigPathForWrite. The function will now (a) sanitize username (fallback to PublicUser), (b) compute baseDir and ensure it exists (or at least use it in checks), (c) for each candidate file, Abs it and ensure it is a descendant of baseDir before using it. No new imports are required because os, filepath, strings, and errors are already imported. All callers can stay unchanged, preserving existing functionality except that malicious or malformed usernames/paths will now be rejected safely.

In general, to fix uncontrolled path usage, you should normalize and validate any user-derived path components before creating filesystem paths, and enforce that the final absolute path stays within a known, safe base directory. Even if individual call sites validate input, putting the core safety checks close to the actual path construction makes the code resilient to future misuse.

For this specific code, the safest non-breaking fix is:

• Make AgentConfigRoot an absolute, cleaned path at startup using filepath.Abs and filepath.Clean. This ensures consistent behavior for subsequent safety checks.
• Change getAgentUserDir so it:
  • Cleans the username component.
  • Rejects any username that would introduce path traversal (e.g., .., path separators), independent of validateUsername.
  • Builds the path with filepath.Join.
  • Resolves it to an absolute path and verifies that the result still lies under the cleaned AgentConfigRoot using strings.HasPrefix.
  • If validation fails for any reason, falls back to the directory for PublicUser, which is safe.
• This way, any future use of getAgentUserDir automatically benefits from these checks, and the dir argument passed to listAgentConfigNamesFromDir is guaranteed to be within AgentConfigRoot, directly addressing the CodeQL alert without changing the external behavior of the handlers.

Concretely, all changes will be in common/websocket/knowledge2_api.go:

1. Add a new package-level variable agentConfigRootAbs and an init function that computes its absolute, cleaned value from AgentConfigRoot. If that computation fails (unlikely), we conservatively leave it as the original AgentConfigRoot.
2. Replace the implementation of getAgentUserDir so it:
   • Uses filepath.Clean on the username.
   • Rejects unsafe usernames: empty, containing path separators (/ or os.PathSeparator), or "..".
   • Joins the cleaned username to agentConfigRootAbs, obtains the absolute path, and checks it starts with agentConfigRootAbs + string(os.PathSeparator) (and also allows exact equality).
   • On any failure, falls back to returning the directory for PublicUser.

No new external packages are required: we reuse os, filepath, and strings which are already imported.

In general, to fix uncontrolled path usage, you must ensure that any path derived from user input is properly constrained. This is typically done by (1) validating the input (disallowing separators, .., etc.) when it is meant to be a single component, and/or (2) resolving the final path against a known safe root and verifying that the resulting absolute path still lies under that root before any filesystem operations (read, write, stat, mkdir, etc.) occur.

For this code, the best fix with minimal functional change is:

1. Keep validateUsername as-is, but add a helper that safely resolves a user’s agent config directory as an absolute path under a fixed safe root.
2. Use this helper in place of getAgentUserDir(username) wherever we prepare to touch the filesystem, so that:
   • We always compute absUserDir := filepath.Abs(filepath.Join(AgentConfigRoot, username)).
   • We reject any path that does not remain under AgentConfigRoot by checking with strings.HasPrefix(absUserDir, absRoot+string(os.PathSeparator)) (or an equivalent robust check).
3. Use this safe directory both when creating directories and when enumerating candidate config file paths.

Within common/websocket/knowledge2_api.go, we will:

• Introduce a new function, e.g. getSafeAgentUserDir(username string) (string, error), placed near getAgentUserDir / validateUsername, that:
  • Resolves AgentConfigRoot to an absolute path once per call.
  • Builds userDir := filepath.Join(AgentConfigRoot, username), gets absUserDir := filepath.Abs(userDir), and verifies absUserDir is still inside the resolved AgentConfigRoot (by prefix check).
  • Returns an error if the check fails.
• Update the creation of the user directory in HandleSaveAgentConfig to use getSafeAgentUserDir rather than the raw getAgentUserDir, and use the resulting safe directory path for os.MkdirAll.
• Update the code around line 669–674 (where candidates and path are built and os.Stat(path) is called) to:
  • Use getSafeAgentUserDir(username) to compute userDir.
  • Ensure that candidate paths (filepath.Join(userDir, name+".yaml" / ".yml")) are under the safe root (this will hold automatically if userDir is safe).

These changes stay within the shown file and reuse existing imports (filepath, strings, os, errors), so no new dependencies are required.

In general, to fix uncontrolled path usage, you should ensure that any path built from user input is constrained to a specific base directory and does not escape it. This is typically done by combining a trusted base directory with the (sanitized) user input, resolving the result to an absolute path, and then verifying that this absolute path still lies within the intended base directory. Only then should you perform file operations like os.Remove.

For this specific code, we already sanitize username with validateUsername and fall back to PublicUser when invalid. However, CodeQL still sees tainted data reaching os.Remove at line 690 through userDir := getAgentUserDir(username) and path := filepath.Join(userDir, name+ext). The minimal, functionality‑preserving hardening is to add a safety check after building userDir and before deleting files: (1) compute the absolute path of userDir, (2) compute the absolute path of each candidate file to delete, and (3) verify that each absolute candidate path has the absolute user directory as its prefix. If any resolution fails or the prefix check fails, we abort deletion and return an error. This prevents any path traversal abuse even if username validation fails elsewhere or changes in the future, and it also addresses the static analysis concern by demonstrating that the sink is reached only with a path within the allowed tree.

Concretely, in deleteAgentConfig (lines 686–699 in common/websocket/knowledge2_api.go), we will:

• Call filepath.Abs(userDir) to get absUserDir.
• For each ext, build candidate := filepath.Join(userDir, name+ext), then compute absCandidate := filepath.Abs(candidate).
• Check that absCandidate starts with absUserDir + string(os.PathSeparator) or equals absUserDir; if not, return an error like errors.New("invalid path") without attempting deletion.
• Only call os.Remove(absCandidate) after this check.

We can use existing imports (os, filepath, errors, strings are already imported), so no new imports are needed. All changes are confined to the deleteAgentConfig function in this file.