diff options
author | Justin Tobler <jtobler@gitlab.com> | 2023-01-25 20:34:51 +0300 |
---|---|---|
committer | Justin Tobler <jtobler@gitlab.com> | 2023-01-27 21:05:53 +0300 |
commit | ea97c42beadef243bb13a52172b175ea396f1e88 (patch) | |
tree | c6342a6b225dbd6555edd331d4e13d373bec05e3 | |
parent | b1841c6d79f62066335ad4d44efa21f3c5f77c03 (diff) |
hooks: Fix custom hooks vote hash
Currently the vote hash is generated using only files names instead of
the full path. This is fine most of the time because if the underlying
path of the file changes the order that the file name gets written to in
the hash usually changes leading to a different vote hash being
generated. However, there are scenarios where changes to the file path
will not change the processing order of entries which could lead to the
same vote hash being generated.
For example the two following directory structures would result in the
same processing order and name.
Structure:
`custom_hooks/foo/`
`custom_hooks/foo/bar`
`custom_hooks/foo/`
`custom_hooks/bar`
Name order:
`custom_hooks`
`foo`
`bar`
This change remediates this issue by leveraging the relative path of
each entry instead of just its name and lexical order.
-rw-r--r-- | internal/gitaly/service/repository/restore_custom_hooks.go | 12 | ||||
-rw-r--r-- | internal/gitaly/service/repository/restore_custom_hooks_test.go | 8 |
2 files changed, 11 insertions, 9 deletions
diff --git a/internal/gitaly/service/repository/restore_custom_hooks.go b/internal/gitaly/service/repository/restore_custom_hooks.go index b221a32e5..9f5639089 100644 --- a/internal/gitaly/service/repository/restore_custom_hooks.go +++ b/internal/gitaly/service/repository/restore_custom_hooks.go @@ -177,11 +177,13 @@ func newDirectoryVote(basePath string) (*voting.VoteHash, error) { return err } - // Write file name to hash. Since `WalkDir()` output is deterministic - // based on lexical order, the path does not need to be included with - // the name written to the hash. Any change to the entry's path will - // result in a different hash due to the change in walked order. - _, _ = voteHash.Write([]byte(entry.Name())) + relPath, err := filepath.Rel(basePath, path) + if err != nil { + return fmt.Errorf("getting relative path: %w", err) + } + + // Write file relative path to hash. + _, _ = voteHash.Write([]byte(relPath)) info, err := entry.Info() if err != nil { diff --git a/internal/gitaly/service/repository/restore_custom_hooks_test.go b/internal/gitaly/service/repository/restore_custom_hooks_test.go index 6596c0301..48afedb17 100644 --- a/internal/gitaly/service/repository/restore_custom_hooks_test.go +++ b/internal/gitaly/service/repository/restore_custom_hooks_test.go @@ -180,7 +180,7 @@ func TestNewDirectoryVote(t *testing.T) { {name: "pre-commit.sample", content: "foo", mode: 0o755}, {name: "pre-push.sample", content: "bar", mode: 0o755}, }, - expectedHash: "b8f99a3012ce12c1a5711e3b94d45b98878cc18d", + expectedHash: "8ca11991268de4c9278488a674fc1a88db449566", }, { desc: "generated hash matches with changed file name", @@ -188,7 +188,7 @@ func TestNewDirectoryVote(t *testing.T) { {name: "pre-commit.sample.diff", content: "foo", mode: 0o755}, {name: "pre-push.sample", content: "bar", mode: 0o755}, }, - expectedHash: "f55ff88d7f84045fce970615ecd04c4fe9bf0a94", + expectedHash: "b5ed58ced84103da1ed9d7813a9e39b3b5daf7d7", }, { desc: "generated hash matches with changed file content", @@ -196,7 +196,7 @@ func TestNewDirectoryVote(t *testing.T) { {name: "pre-commit.sample", content: "foo", mode: 0o755}, {name: "pre-push.sample", content: "bar.diff", mode: 0o755}, }, - expectedHash: "0c628c59e62c351069037aad858a6f900ef1de9b", + expectedHash: "178083848c8a08e36c4f86c2d318a84b0bb845f2", }, { desc: "generated hash matches with changed file mode", @@ -204,7 +204,7 @@ func TestNewDirectoryVote(t *testing.T) { {name: "pre-commit.sample", content: "foo", mode: 0o644}, {name: "pre-push.sample", content: "bar", mode: 0o755}, }, - expectedHash: "9321dde590ef5ffa05d1ddf690b8983f406242e7", + expectedHash: "c69574241b83496bb4005b4f7a0dfcda96cb317e", }, } { t.Run(tc.desc, func(t *testing.T) { |