From cb228542696e3a997cf94a5e484560169c4ac376 Mon Sep 17 00:00:00 2001 From: Vibhav Bobade Date: Fri, 27 Mar 2026 19:13:45 +0530 Subject: [PATCH] fix(crafter): use deterministic filename for AI config collector MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace os.CreateTemp (which generates a random suffix) with a deterministic filename derived from the config's content hash. Root cause: although AddMaterialContractFree uses a deterministic map key (Materials[m.Name]), the Artifact struct embedded in each material gets its Name and Digest from fileStats(), which calls os.Stat(filepath).Name(). A random temp filename therefore produces a different Artifact.Name on every call — even when the JSON payload is byte-for-byte identical — causing the CAS to treat each retry as a new, distinct resource. Changes: - Extract aiConfigTempFilePath() helper for deterministic path generation - Use full ConfigHash (no truncation) to avoid collision risk - Write into a private os.MkdirTemp directory (symlink-safe) instead of the shared os.TempDir() - Add unit tests proving same-input-same-output and different-input- different-output invariants Fixes: chainloop-dev/chainloop#2907 Co-Authored-By: Claude Opus 4.6 (1M context) Signed-off-by: Vibhav Bobade --- .../crafter/collector_aiagentconfig.go | 32 +++++++--- .../crafter/collector_aiagentconfig_test.go | 61 +++++++++++++++++++ 2 files changed, 86 insertions(+), 7 deletions(-) create mode 100644 pkg/attestation/crafter/collector_aiagentconfig_test.go diff --git a/pkg/attestation/crafter/collector_aiagentconfig.go b/pkg/attestation/crafter/collector_aiagentconfig.go index 4b67f97e6..c9e51d522 100644 --- a/pkg/attestation/crafter/collector_aiagentconfig.go +++ b/pkg/attestation/crafter/collector_aiagentconfig.go @@ -20,6 +20,7 @@ import ( "encoding/json" "fmt" "os" + "path/filepath" "sort" schemaapi "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1" @@ -27,6 +28,21 @@ import ( "github.com/chainloop-dev/chainloop/pkg/casclient" ) +// aiConfigTempFilePath returns a deterministic filename for the given agent +// name and config hash. Using a deterministic path (instead of a random temp +// file) ensures that retries of the same attestation produce identical +// Artifact.Name and Artifact.Digest values inside the material, which +// prevents duplicate CAS uploads. +// +// Why the filename matters: AddMaterialContractFree stores the material in +// Materials[m.Name] (a deterministic map key), but the Artifact struct inside +// that material gets its Name and Digest from fileStats(), which calls +// os.Stat(filepath).Name(). A random temp filename therefore produces a +// different Artifact on every call, even when the content is identical. +func aiConfigTempFilePath(dir, agentName, configHash string) string { + return filepath.Join(dir, fmt.Sprintf("ai-agent-config-%s-%s.json", agentName, configHash)) +} + // AIAgentConfigCollector discovers AI agent configuration files and attaches them as evidence. type AIAgentConfigCollector struct{} @@ -99,20 +115,22 @@ func (c *AIAgentConfigCollector) uploadAgentConfig( return fmt.Errorf("marshaling AI agent config for %s: %w", agentName, err) } - tmpFile, err := os.CreateTemp("", fmt.Sprintf("ai-agent-config-%s-*.json", agentName)) + // Write to a private temp directory with a deterministic filename so that + // retries produce the same Artifact.Name / Artifact.Digest (see + // aiConfigTempFilePath for the full explanation). + tmpDir, err := os.MkdirTemp("", "chainloop-ai-config-*") if err != nil { - return fmt.Errorf("creating temp file: %w", err) + return fmt.Errorf("creating temp directory: %w", err) } - defer os.Remove(tmpFile.Name()) + defer os.RemoveAll(tmpDir) - if _, err := tmpFile.Write(jsonData); err != nil { - tmpFile.Close() + tmpPath := aiConfigTempFilePath(tmpDir, agentName, data.ConfigHash) + if err := os.WriteFile(tmpPath, jsonData, 0o600); err != nil { return fmt.Errorf("writing temp file: %w", err) } - tmpFile.Close() materialName := fmt.Sprintf("ai-agent-config-%s", agentName) - if _, err := cr.AddMaterialContractFree(ctx, attestationID, schemaapi.CraftingSchema_Material_CHAINLOOP_AI_AGENT_CONFIG.String(), materialName, tmpFile.Name(), casBackend, nil); err != nil { + if _, err := cr.AddMaterialContractFree(ctx, attestationID, schemaapi.CraftingSchema_Material_CHAINLOOP_AI_AGENT_CONFIG.String(), materialName, tmpPath, casBackend, nil); err != nil { return fmt.Errorf("adding AI agent config material for %s: %w", agentName, err) } diff --git a/pkg/attestation/crafter/collector_aiagentconfig_test.go b/pkg/attestation/crafter/collector_aiagentconfig_test.go new file mode 100644 index 000000000..e607b0534 --- /dev/null +++ b/pkg/attestation/crafter/collector_aiagentconfig_test.go @@ -0,0 +1,61 @@ +// +// Copyright 2026 The Chainloop Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package crafter + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const ( + testTempDir = "/tmp/test-dir" + testAgent = "claude" + testConfigHash = "a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2" +) + +func TestAIConfigTempFilePath_Deterministic(t *testing.T) { + t.Run("same inputs produce same filename", func(t *testing.T) { + path1 := aiConfigTempFilePath(testTempDir, testAgent, testConfigHash) + path2 := aiConfigTempFilePath(testTempDir, testAgent, testConfigHash) + + assert.Equal(t, path1, path2, "same inputs must produce identical file paths") + }) + + t.Run("different config hash produces different filename", func(t *testing.T) { + hash2 := "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff" + + path1 := aiConfigTempFilePath(testTempDir, testAgent, testConfigHash) + path2 := aiConfigTempFilePath(testTempDir, testAgent, hash2) + + assert.NotEqual(t, path1, path2, "different hashes must produce different file paths") + }) + + t.Run("different agent name produces different filename", func(t *testing.T) { + path1 := aiConfigTempFilePath(testTempDir, testAgent, testConfigHash) + path2 := aiConfigTempFilePath(testTempDir, "cursor", testConfigHash) + + assert.NotEqual(t, path1, path2, "different agent names must produce different file paths") + }) + + t.Run("filename includes full config hash", func(t *testing.T) { + path := aiConfigTempFilePath(testTempDir, testAgent, testConfigHash) + + require.Contains(t, path, testConfigHash, "path must contain the full config hash (not truncated)") + assert.Equal(t, testTempDir+"/ai-agent-config-"+testAgent+"-"+testConfigHash+".json", path) + }) +}