Skip to content

Conversation

@elezar
Copy link
Member

@elezar elezar commented Jul 7, 2025

This change updates the proc mount when updating ldconfig to use secure join to align with the other functions.

This change updates the proc mount when updating ldconfig to
use secure join to align with the other functions.

Signed-off-by: Evan Lezar <[email protected]>
@elezar elezar self-assigned this Jul 7, 2025
@elezar elezar added the must-backport The changes in PR need to be backported to at least one stable release branch. label Jul 7, 2025
@elezar elezar requested a review from ArangoGutierrez July 7, 2025 09:20
@coveralls
Copy link

Pull Request Test Coverage Report for Build 16113025035

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 6 (0.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.005%) to 33.112%

Changes Missing Coverage Covered Lines Changed/Added Lines %
internal/ldconfig/ldconfig_linux.go 0 6 0.0%
Totals Coverage Status
Change from base Build 16054869452: -0.005%
Covered Lines: 4381
Relevant Lines: 13231

💛 - Coveralls

@ArangoGutierrez ArangoGutierrez requested a review from Copilot July 7, 2025 11:00
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR improves the mountProc function in ldconfig_linux.go by using securejoin.SecureJoin to resolve the /proc path and switching directory creation to utils.MkdirAllInRoot for consistency and security.

  • Use securejoin.SecureJoin instead of filepath.Join for the proc mount target
  • Replace os.MkdirAll with utils.MkdirAllInRoot
  • Update error handling in mountProc

return fmt.Errorf("error creating directory: %w", err)
target, err := securejoin.SecureJoin(newroot, "proc")
if err != nil {
return err
Copy link

Copilot AI Jul 7, 2025

Choose a reason for hiding this comment

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

Wrap the error from securejoin.SecureJoin with context to aid debugging. For example: return fmt.Errorf("failed to resolve proc path: %w", err)

Suggested change
return err
return fmt.Errorf("failed to resolve proc path: %w", err)

Copilot uses AI. Check for mistakes.
return err
}
if err := utils.MkdirAllInRoot(newroot, target, 0755); err != nil {
return err
Copy link

Copilot AI Jul 7, 2025

Choose a reason for hiding this comment

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

Provide additional context when returning this error, e.g., return fmt.Errorf("failed to create proc directory: %w", err) to clarify which operation failed.

Suggested change
return err
return fmt.Errorf("failed to create proc directory at %s: %w", target, err)

Copilot uses AI. Check for mistakes.
Copy link
Collaborator

@ArangoGutierrez ArangoGutierrez left a comment

Choose a reason for hiding this comment

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

LGTM

@elezar elezar merged commit d1d1676 into NVIDIA:main Jul 7, 2025
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

must-backport The changes in PR need to be backported to at least one stable release branch.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants