Skip to content

user: GetExecUser: always clone default Sgids#229

Merged
thaJeztah merged 1 commit into
moby:mainfrom
thaJeztah:user_deref_sgids
Jun 30, 2026
Merged

user: GetExecUser: always clone default Sgids#229
thaJeztah merged 1 commit into
moby:mainfrom
thaJeztah:user_deref_sgids

Conversation

@thaJeztah

Copy link
Copy Markdown
Member

Clone the default Sgids slice instead of sharing it to prevent accidentally mutating, and simplify the initialization by creating a non-nil copy directly.

Copilot AI left a comment

Copy link
Copy Markdown

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 updates GetExecUser to avoid sharing the defaults.Sgids backing array with the returned ExecUser, preventing accidental mutation of the defaults. It also simplifies initialization by guaranteeing a non-nil Sgids slice directly during struct construction.

Changes:

  • Clone defaults.Sgids when building the returned ExecUser to avoid slice aliasing.
  • Remove the post-initialization nil check by constructing Sgids as non-nil from the start.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread user/user.go
Uid: defaults.Uid,
Gid: defaults.Gid,
Sgids: defaults.Sgids,
Sgids: append([]int{}, defaults.Sgids...), // Sgids slice *cannot* be nil.
Clone the default Sgids slice instead of sharing it to prevent
accidentally mutating, and simplify the initialization by creating
a non-nil copy directly.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah

Copy link
Copy Markdown
Member Author

rebased after #228 was merged; I'll merge once CI completes

@thaJeztah thaJeztah merged commit 55196b1 into moby:main Jun 30, 2026
20 checks passed
@thaJeztah thaJeztah deleted the user_deref_sgids branch June 30, 2026 11:18
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.

3 participants