Repository navigation
Conversation
…nt commit GitStateRecorder.signAndCommit read HEAD, signed it as the record's parent, wrote four fixed-name files into one shared directory and committed, with no lock. Overlapping workflow executions could therefore sign the same parent (false "chain broken"), overwrite each other's files (signature mismatch, lost record) or collide on .git/index.lock (record silently missing). - hold a mutex from the HEAD read until the commit is created - return an error instead of signing parent="" when HEAD cannot be read Tests: concurrent RecordState/RecordDrift must produce exactly one commit per record and a verifiable chain; an unreadable HEAD must fail without creating a commit. Both fail without the fix. The existing "git command fails" case now corrupts the index, because removing .git fails earlier, at the parent read. Refs #161 --- [signing-metadata] key = cic-my-sign-key signature = vault:v1:MEYCIQDxpjq4sJlzUcm2Mw8lh3+VKrqEDkfKM9Ce1nv9sVvsxAIhAKcTkHGh5Cw0vjcByYgMbe1iLwgFD2VoAg4a1A/9VvKn hash-algorithm = sha256 digest = +PQdSK8RK6S+JWKlmkhDgcOrZFhVYV/AXRoTaaCUJj4= [certificate] -----BEGIN CERTIFICATE----- MIICBjCCAaygAwIBAgIUSnRMR6RPnEbg296XWPOqq/u5PCwwCgYIKoZIzj0EAwIw QzELMAkGA1UEBhMCSFUxGTAXBgNVBAoMEENlbnRyYWxJbmZyYUNvcmUxGTAXBgNV BAMMEENJQyBEZXZlbG9wZXIgQ0EwHhcNMjYwMzIwMTMyMjU5WhcNMjYxMjMxMTMy MjU5WjBFMQswCQYDVQQGEwJIVTEZMBcGA1UECgwQQ2VudHJhbEluZnJhQ29yZTEb MBkGA1UEAwwSR2Fib3IgWm9sdGFuIFNpbmtvMFkwEwYHKoZIzj0CAQYIKoZIzj0D AQcDQgAEIG2CVmTfmLB9pLLclj7YmP2eedAjklpy4LGrU2ijoiy6Xqpuybv7OgJe i+ez31s65NEV8+X/ByeX1cstR988z6N8MHowCQYDVR0TBAIwADAdBgNVHQ4EFgQU yZN6AIX/TNnIJ9GwAa/NRN3ujHAwHwYDVR0jBBgwFoAUXn6CHYzPUqU4JVP8g+OS WeDYjhcwDgYDVR0PAQH/BAQDAgeAMB0GA1UdJQQWMBQGCCsGAQUFBwMCBggrBgEF BQcDBDAKBggqhkjOPQQDAgNIADBFAiEA+bFzXRoJ4PCQbhAAtpkcMjt0vNj5rEW0 lOMBGDNyaWkCIB1vmM7PcZzv/c9bIrxF5kqv6QXomouhByUfeNUTbpKW -----END CERTIFICATE-----
…ter lock contract The concurrency test proved the commit count and a verifiable chain, which does not show that each record survived intact. It now also requires each of the 12 run-ids exactly once and that every commit's signed meta.json names the run its commit message names. Without the lock this reports the missing run-ids. The lock comment now states the contract: one recorder instance per repository, and that waiting for the lock ignores the caller's context. Refs #161 --- [signing-metadata] key = cic-my-sign-key signature = vault:v1:MEUCIQCvtM6iuoFjTQKHZ8ufyBpF4At2V5r0rKi4563GhAp4YgIgR5eT0jE50yWtB3u4R8/hVuoxlHYm/fCGnclmeQlwxn0= hash-algorithm = sha256 digest = xfeKYGDYRpwFZ+7ZKzjLQgI4euMdlzZNbWXw7jrTWrg= [certificate] -----BEGIN CERTIFICATE----- MIICBjCCAaygAwIBAgIUSnRMR6RPnEbg296XWPOqq/u5PCwwCgYIKoZIzj0EAwIw QzELMAkGA1UEBhMCSFUxGTAXBgNVBAoMEENlbnRyYWxJbmZyYUNvcmUxGTAXBgNV BAMMEENJQyBEZXZlbG9wZXIgQ0EwHhcNMjYwMzIwMTMyMjU5WhcNMjYxMjMxMTMy MjU5WjBFMQswCQYDVQQGEwJIVTEZMBcGA1UECgwQQ2VudHJhbEluZnJhQ29yZTEb MBkGA1UEAwwSR2Fib3IgWm9sdGFuIFNpbmtvMFkwEwYHKoZIzj0CAQYIKoZIzj0D AQcDQgAEIG2CVmTfmLB9pLLclj7YmP2eedAjklpy4LGrU2ijoiy6Xqpuybv7OgJe i+ez31s65NEV8+X/ByeX1cstR988z6N8MHowCQYDVR0TBAIwADAdBgNVHQ4EFgQU yZN6AIX/TNnIJ9GwAa/NRN3ujHAwHwYDVR0jBBgwFoAUXn6CHYzPUqU4JVP8g+OS WeDYjhcwDgYDVR0PAQH/BAQDAgeAMB0GA1UdJQQWMBQGCCsGAQUFBwMCBggrBgEF BQcDBDAKBggqhkjOPQQDAgNIADBFAiEA+bFzXRoJ4PCQbhAAtpkcMjt0vNj5rEW0 lOMBGDNyaWkCIB1vmM7PcZzv/c9bIrxF5kqv6QXomouhByUfeNUTbpKW -----END CERTIFICATE-----
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #161
What
GitStateRecorder.signAndCommitread HEAD, signed it as the record's parent, wrote four fixed-name files into one shared directory and committed, with no lock. Overlapping workflow executions could sign the same parent, overwrite each other's files, or collide on.git/index.lock(the record is then silently missing).sync.Mutexfrom the HEAD read until the commit is created (core/nexus/recorder/recorder.go).parent = ""when HEAD cannot be read.The lock covers one process only; several writers on one audit repository would need a file lock (not part of this PR). The lock is held across the Vault
Signcall, which is required because the signed manifest contains the parent, so records are written serially.Tests
TestSignAndCommit_ConcurrentRecordsKeepChainVerifiable: 12 concurrentRecordState/RecordDriftcalls (Sign delayed to widen the window) must give exactly one commit per record plus the root, andVerifyChainmust pass.TestSignAndCommit_HeadReadFailureIsAnError: a repository without commits must fail the record and create no commit.git command failscase now corrupts the index instead of removing.git, because removing.gitfails earlier, at the parent read.Both new tests fail without the fix (fix reverted temporarily):
Local gates (Docker
golang:1.26.11):gofmt -s -lempty,go vet ./...clean,staticcheck ./core/nexus/recorder/clean,go test -count=1 ./...allok,go test -race ./core/nexus/recorder/ok. Norecorderentry indeadcode ./....Reachability
cmd/relay/main.go:480NewWorkflowRecorder->main.go:486cabinet.WithRecorder->core/nexus/recorder/workflow_recorder.go:37->recorder.go:98RecordState->signAndCommit. Other callers:core/nexus/operator/bootstrap.go:107,lifecycle.go:130.Not verified
Setxis called concurrently over HTTP is the issue's reading; the handler path was not followed end to end here.chain broken) and 2 (mixed files) of the issue were not triggered separately; theindex.lockcollision masks them in the unfixed run.Notes
The commit carries the developer's Vault signing metadata, added by a global commit hook. No workflow or CI files are touched.
🤖 Generated with Claude Code