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.

Sign up for GitHub

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

correctly handle configuration file saving when it is a relative symlink #5282

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

correctly handle configuration file saving when it is a relative symlink #5282

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
c90705a
fix bug with config file being a relative symlink
willww64 Jul 23, 2024
115fdd9
Update cli/config/configfile/file.go
willww64 Jul 23, 2024
c6d1de3
allow for dangling symlinks
willww64 Jul 24, 2024
efaf3c9
extract path from error when NotExist error occurs
willww64 Jul 24, 2024
40df265
use os.PathError to avoid an extra import
willww64 Jul 24, 2024
a85a37e
better to limit the scope of err
willww64 Jul 24, 2024
0118291
update comments
willww64 Jul 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions cli/config/configfile/file.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,10 +167,13 @@ func (configFile *ConfigFile) Save() (retErr error) {
return errors.Wrap(err, "error closing temp file")
}

// Handle situation where the configfile is a symlink
// Handle situation where the configfile is a symlink, and allow for dangling symlinks
cfgFile := configFile.Filename
if f, err := os.Readlink(cfgFile); err == nil {
if f, err := filepath.EvalSymlinks(cfgFile); err == nil {
cfgFile = f
} else if os.IsNotExist(err) {
// extract the path from the error if the configfile does not exist or is a dangling symlink
cfgFile = err.(*os.PathError).Path
Copy link
Member

Choose a reason for hiding this comment

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

Not sure if we can hit that situation, but this could panic, because os.IsNotExist also considers syscall errors; https://go.dev/play/p/5VjH5lK-YDB

package main

import (
	"os"
	"syscall"
	"testing"
)

func TestCheckError(t *testing.T) {
	err := func() error {
		return syscall.ENOENT
	}()
	if os.IsNotExist(err) {
		t.Log(err)
		cfgFile := err.(*os.PathError).Path
		t.Log(cfgFile)
	}
}

}

// Try copying the current config file (if any) ownership and permissions
Expand Down
28 changes: 28 additions & 0 deletions cli/config/configfile/file_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -538,6 +538,34 @@ func TestSaveWithSymlink(t *testing.T) {
assert.Check(t, is.Equal(string(cfg), "{\n \"auths\": {}\n}"))
}

func TestSaveWithRelativeSymlink(t *testing.T) {
dir := fs.NewDir(t, t.Name(), fs.WithFile("real-config.json", `{}`))
defer dir.Remove()

symLink := dir.Join("config.json")
relativeRealFile := "real-config.json"
realFile := dir.Join(relativeRealFile)
err := os.Symlink(relativeRealFile, symLink)
assert.NilError(t, err)

configFile := New(symLink)

err = configFile.Save()
assert.NilError(t, err)

fi, err := os.Lstat(symLink)
assert.NilError(t, err)
assert.Assert(t, fi.Mode()&os.ModeSymlink != 0, "expected %s to be a symlink", symLink)

cfg, err := os.ReadFile(symLink)
assert.NilError(t, err)
assert.Check(t, is.Equal(string(cfg), "{\n \"auths\": {}\n}"))

cfg, err = os.ReadFile(realFile)
assert.NilError(t, err)
assert.Check(t, is.Equal(string(cfg), "{\n \"auths\": {}\n}"))
}

func TestPluginConfig(t *testing.T) {
configFile := New("test-plugin")
defer os.Remove("test-plugin")
Expand Down
Loading