Skip to content

Do not follow destination symlink when copying user configuration#1957

Merged
jglogan merged 1 commit into
apple:mainfrom
katiewasnothere:config_load_copy_rename
Jul 20, 2026
Merged

Do not follow destination symlink when copying user configuration#1957
jglogan merged 1 commit into
apple:mainfrom
katiewasnothere:config_load_copy_rename

Conversation

@katiewasnothere

@katiewasnothere katiewasnothere commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Type of Change

  • Bug fix

Motivation and Context

This PR fixes a narrow case where a user may have modified the user configuration file in the application root to use a symlink. In this case, upon restarting the service, we would follow the symlink, potentially replacing files unintentionally. This PR fixes this issue by using the rename(2) which will override the destPath regardless of its file permissions and status as a symlink or otherwise.

Testing

  • Tested locally
  • Added/updated tests

Signed-off-by: Kathryn Baldauf <k_baldauf@apple.com>
@katiewasnothere
katiewasnothere force-pushed the config_load_copy_rename branch from 650ff17 to dcfdad3 Compare July 16, 2026 21:46
@github-actions

Copy link
Copy Markdown

Code Coverage

Tier Line Coverage
Unit 23.38%
Integration 66.49%
Combined 75.46%

@github-actions

Copy link
Copy Markdown

Code Coverage

Tier Line Coverage
Unit 23.37%
Integration 66.48%
Combined 75.45%


let fm = FileManager.default

if fm.fileExists(atPath: destPath.string) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm not sure I understand. Was removeItem following the symlink and removing the file at the target path?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Or that the symlink was pointing nowhere and fileExists follows the symlink and then returns false?

@jglogan
jglogan merged commit 3310f34 into apple:main Jul 20, 2026
3 checks passed
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