misc: main sync

[prkhrkat]

Description

Fixes #

Checklist:

• The title of the PR states what changed and the related issues number (used for the release note).
• Does this PR requires documentation updates?
• I've updated documentation as required by this PR.
• I have performed a self-review of my own code.
• I have commented my code, particularly in hard-to-understand areas.
• I have tested it for all user roles.
• I have added all the required unit/api test cases.

Does this PR introduce a user-facing change?

In general, to fix uncontrolled data in a path expression you either (1) restrict the input so it cannot contain path separators or .. components when used as a directory or filename, or (2) normalize the constructed path and verify that it resides inside an expected base directory before performing filesystem operations.

Here the path of concern is workingDir := filepath.Join(clonedDir, gitOpsChartLocation), where gitOpsChartLocation is built from AppName and EnvName (user-controlled via the request DTO). The simplest, low-impact mitigation is:

1. Compute workingDir as today with filepath.Join.
2. Normalize it with filepath.Abs.
3. Normalize the base directory (clonedDir) with filepath.Abs.
4. Check that the resolved workingDir is still a child of the resolved clonedDir (e.g., with strings.HasPrefix on a path with a trailing separator).
5. If the check fails, log an error and return an error instead of proceeding to shouldDeleteProxyChartRequirementsYaml / shouldMigrateProxyChartDependencies.

This ensures that even if AppName or EnvName contain path traversal sequences, they cannot cause workingDir (and thus requirementsYamlPath and chartYamlPath) to escape the cloned repo directory. This does not change existing business logic beyond rejecting malicious/badly-formed names.

Concretely, in pkg/deployment/gitOps/git/GitOperationService.go inside MigrateProxyChartDependenciesIfRequired:

• After workingDir := filepath.Join(clonedDir, gitOpsChartLocation), add code to compute absClonedDir and absWorkingDir via filepath.Abs, ensure absWorkingDir starts with absClonedDir plus a path separator, and return an error if not.
• We can reuse existing imports (filepath, strings, fmt are already imported), so no new dependencies are needed.
• No other files need modification; the taint flowing into pushChartToGitRequest is now bounded by this validation.

---

General approach: ensure that any path derived from user-controllable values is validated or confined to a known safe directory before being used in file operations. For multi-component paths, the safest pattern is to resolve the path and verify that it is still within an expected root directory.

Best fix here: requirementsYamlPath is created by filepath.Join(clonedDir, chartRefBean.REQUIREMENTS_YAML_FILE), where clonedDir ultimately depends on user input via AppName/EnvName. We can make the deletion operation safe by:

1. Resolving the absolute path of requirementsYamlPath.
2. Resolving the absolute path of clonedDir (the intended safe root).
3. Ensuring the file to be deleted is within that root by checking HasPrefix on the cleaned absolute paths.
4. If the check fails, log and return an error instead of calling os.Remove.

Because the repository uses filepath already in this file, we can re-use it. To implement this without changing behavior for valid inputs, we only add a guard around os.Remove in deleteProxyChartRequirementsYaml, and we pass the necessary clonedDir down so the function can perform the containment check.

Concretely:

• Change the signature of deleteProxyChartRequirementsYaml in pkg/deployment/gitOps/git/GitOperationService.go to accept clonedDir string (the root of the cloned repo).
• Update its caller in MigrateProxyChartDependenciesIfRequired to pass clonedDir.
• Inside deleteProxyChartRequirementsYaml, compute absRoot := filepath.Clean(clonedDir) and absTarget := filepath.Clean(requirementsYamlPath) (optionally with filepath.Abs if needed) and ensure strings.HasPrefix(absTarget+string(os.PathSeparator), absRoot+string(os.PathSeparator)). If not, log an error and return.
• Then call os.Remove as before.

All required imports (filepath, strings, os) are already present in GitOperationService.go, so no new imports are needed.

Other files in the taint flow (AppStoreDeploymentRestHandler.go, AppStoreDeploymentService.go, InstalledAppGitOpsService.go, Adapter.go, CommonDeploymentRestHandler.go) do not need modifications for this specific path-usage issue; they simply propagate the tainted data.

---

In general, to fix uncontrolled data in path expressions you either (a) validate that user-controlled components contain only allowed characters and no path separators or .., or (b) normalize the resulting path and verify it remains within an expected base directory. Here, AppName and EnvName are being used as logical identifiers to construct a chart directory name under clonedDir; they should never contain path separators. The least-invasive and clearest fix is therefore to validate these two fields before using them to form gitOpsChartLocation and workingDir.

The single best fix with minimal behavior change is to add a small validation helper in GitOperationServiceImpl that checks pushChartToGitRequest.AppName and .EnvName for path separators (/, \) and parent directory segments (..). If any such patterns are found, we log an error and return a validation error instead of proceeding. We then call this helper at the start of MigrateProxyChartDependenciesIfRequired before calling CloneChartForHelmApp and before building gitOpsChartLocation. This keeps all path manipulation under clonedDir safe from traversal via these user-provided components, and it covers both CodeQL alert variants because both flows end up in this same method.

Concretely:

• In pkg/deployment/gitOps/git/GitOperationService.go, define a new method validatePushChartToGitRequestForPathSafety (or similar) on GitOperationServiceImpl that:
  • Takes pushChartToGitRequest *bean.PushChartToGitRequestDTO.
  • Checks AppName and EnvName strings for "..", "/", and "\\" (you can use strings.Contains).
  • If invalid, logs an error/warn with app/env names and returns a fmt.Errorf explaining invalid application or environment name.
• At the beginning of MigrateProxyChartDependenciesIfRequired, before CloneChartForHelmApp, call this helper and, on error, log and return it.
• This change is entirely contained within the shown GitOperationService.go snippet, requires no new external dependencies, and does not alter normal behavior for valid app/environment names; it only rejects clearly unsafe ones.

---

General approach: when constructing filesystem paths from data that ultimately comes from the user, we should ensure that the final resolved path remains under a known “safe” base directory (here, the clonedDir returned by CloneChartForHelmApp). The concrete file name (chartRefBean.CHART_YAML_FILE) is already safe; the risk is from the intermediate directory (gitOpsChartLocation / workingDir). We can mitigate this by normalizing and checking the paths with filepath.Abs and strings.HasPrefix against the base directory.

Best concrete fix here:

1. After cloning (clonedDir), compute the intended chart directory (gitOpsChartLocation := fmt.Sprintf("%s-%s", pushChartToGitRequest.AppName, pushChartToGitRequest.EnvName) as before).
2. Build workingDir := filepath.Join(clonedDir, gitOpsChartLocation), then:
   • Resolve both clonedDir and workingDir to absolute paths using filepath.Abs.
   • Verify that workingDirAbs starts with baseAbs + string(os.PathSeparator) (or equals baseAbs) using strings.HasPrefix. If the check fails, log and return an error instead of proceeding.
3. In shouldMigrateProxyChartDependencies, we construct chartYamlPath := filepath.Join(clonedDir, chartRefBean.CHART_YAML_FILE). To satisfy CodeQL and enforce safety consistently, we similarly:
   • Resolve clonedDir and chartYamlPath to absolute paths.
   • Validate that chartYamlPathAbs is under baseAbs. If not, log and return an error.

These changes do not alter existing external behavior for valid inputs, but they prevent directory traversal via malicious AppName/EnvName (or other components that influence clonedDir) by rejecting any request that would cause access outside the cloned repo directory. All changes are local to pkg/deployment/gitOps/git/GitOperationService.go, using only standard-library functions already imported (filepath, os, strings, fmt).

---

[bito-code-review]

Bito Automatic Review Skipped - Branch Excluded

Bito didn't auto-review because the source or target branch is excluded from automatic reviews. To trigger review, type /review in the comment and save. You can change the settings here, or contact the agent instance creator at shivam@devtron.ai.

[gitguardian]

⚠️ GitGuardian has uncovered 3 secrets following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

🔎 Detected hardcoded secrets in your pull request

GitGuardian id	GitGuardian status	Secret	Commit	Filename
9257608	Triggered	Base64 AWS keys	f733a1d	scripts/devtron-reference-helm-charts/deployment-chart_4-22-0/test-values.json	View secret
2763127	Triggered	Generic High Entropy Secret	4b8eb26	scripts/devtron-reference-helm-charts/reference-chart_5-2-0/secrets-test-values.yaml	View secret
141558	Triggered	Generic High Entropy Secret	f733a1d	scripts/devtron-reference-helm-charts/statefulset-chart_5-2-0/env-values.yaml	View secret

🛠 Guidelines to remediate hardcoded secrets

1. Understand the implications of revoking this secret by investigating where it is used in your code.
2. Replace and store your secrets safely. Learn here the best practices.
3. Revoke and rotate these secrets.
4. If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider

• following these best practices for managing and storing secrets including API keys and other credentials
• install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.

---

^{🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.}

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
1.4% Duplication on New Code

See analysis details on SonarQube Cloud