Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: orchestrion pin can track enabled integrations #376

Merged
merged 17 commits into from
Nov 8, 2024

Conversation

RomainMuller
Copy link
Contributor

Initial changes to how orchestrion pin works to support incremental edits to the file. This is in preparation to moving the aspects/integration definitions from orchestrion itself to the tracer library.

@RomainMuller RomainMuller requested a review from a team as a code owner November 4, 2024 08:57
}

func runGoMod(command string, modfile string, stdout io.Writer, args ...string) error {
cmd := exec.Command("go", "mod", command, "-modfile", modfile)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Code Vulnerability

Check command call and ensure there is no unsanitized data used. The variable `command` may need to be validated (...read more)

In Go, the exec.Command function is used to run external commands. Using this function carelessly can lead to command injection vulnerabilities.

Command injection occurs when untrusted input is passed directly to a system shell, allowing an attacker to execute arbitrary commands. This can result in unauthorized access to the system, data leaks, or other security breaches.

To prevent command injection vulnerabilities when using exec.Command in Go, follow these coding best practices:

  1. Sanitize User Input: Always validate and sanitize user inputs before passing them to exec.Command. Avoid executing commands constructed using user-provided data.
  2. Avoid using Shell Expansion: If possible, pass the command and arguments as separate strings to exec.Command. This prevents the shell from interpreting special characters in a potentially malicious way.
  3. Use Absolute Paths: When specifying the command to be executed, use absolute paths for executables whenever possible. This reduces the risk of inadvertently running a similarly named malicious command from the system's PATH.
  4. Avoid String Concatenation: Refrain from dynamically constructing commands by concatenating strings. Instead, use the arg ...string parameter of exec.Command to pass arguments safely.
  5. Limit Privileges: Run commands with the least privilege required to carry out the task. Avoid running commands with elevated privileges unnecessarily.

By following these practices, you can reduce the risk of command injection vulnerabilities when using exec.Command in Go and enhance the security of your application.

View in Datadog  Leave us feedback  Documentation

internal/pin/auto.go Outdated Show resolved Hide resolved
…EC-55160/enhance-pin

# Conflicts:
#	internal/pin/pin.go
@RomainMuller RomainMuller force-pushed the romain.marcadier/APPSEC-55160/enhance-pin branch from 7ed5c27 to 058d70a Compare November 6, 2024 08:54
Signed-off-by: Eliott Bouhana <[email protected]>
Comment on lines +58 to +66
&cli.StringFlag{
Category: "Advanced",
Name: "C",
Usage: "Change to the specified directory before proceeding with the rest of the command.",
Hidden: true, // Users don't normally need to use this.
Action: func(_ *cli.Context, dir string) error {
return os.Chdir(dir)
},
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix #161 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure that bug still reproduces today (before this change).

@RomainMuller RomainMuller added this pull request to the merge queue Nov 7, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 7, 2024
@eliottness eliottness added this pull request to the merge queue Nov 8, 2024
Merged via the queue into main with commit 82e1beb Nov 8, 2024
23 checks passed
@eliottness eliottness deleted the romain.marcadier/APPSEC-55160/enhance-pin branch November 8, 2024 00:52
Copy link

codecov bot commented Nov 8, 2024

Codecov Report

Attention: Patch coverage is 64.06250% with 161 lines in your changes missing coverage. Please review.

Project coverage is 60.81%. Comparing base (c91c3e9) to head (a3364d8).
Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
internal/pin/pin.go 61.32% 54 Missing and 16 partials ⚠️
internal/pin/importset.go 64.84% 38 Missing and 7 partials ⚠️
internal/pin/gomod.go 55.73% 23 Missing and 4 partials ⚠️
internal/pin/auto.go 69.35% 17 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #376      +/-   ##
==========================================
+ Coverage   60.28%   60.81%   +0.52%     
==========================================
  Files         166      174       +8     
  Lines       11973    12302     +329     
==========================================
+ Hits         7218     7481     +263     
- Misses       4280     4340      +60     
- Partials      475      481       +6     
Components Coverage Δ
Generators 76.98% <ø> (ø)
Instruments 88.05% <ø> (ø)
Go Driver 73.46% <ø> (+0.32%) ⬆️
Toolexec Driver 70.64% <100.00%> (ø)
Aspects 71.73% <ø> (ø)
Injector 73.72% <ø> (ø)
Job Server 64.02% <ø> (+0.56%) ⬆️
Integration Test Suite 54.24% <ø> (+0.69%) ⬆️
Other 60.81% <64.06%> (+0.52%) ⬆️
Files with missing lines Coverage Δ
internal/cmd/pin.go 100.00% <100.00%> (+100.00%) ⬆️
internal/toolexec/proxy/link.flags.go 100.00% <100.00%> (ø)
main.go 26.35% <100.00%> (+1.15%) ⬆️
internal/pin/auto.go 69.35% <69.35%> (ø)
internal/pin/gomod.go 55.73% <55.73%> (ø)
internal/pin/importset.go 64.84% <64.84%> (ø)
internal/pin/pin.go 57.86% <61.32%> (-0.39%) ⬇️

... and 60 files with indirect coverage changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants