From 048d15c97192e4e864905031ef9e0fd03f0b8d84 Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Tue, 8 Sep 2026 17:28:44 +0200 Subject: [PATCH 1/3] feat(cli): create the trace workflow on init, attached to the AI coding session contract `chainloop trace init` and `trace run` now create the Chainloop workflow the trace attestations target, attached to the `chainloop-ai-coding-session` contract when the organization has it and to the control plane's default contract otherwise. Until now the workflow was created implicitly by the pre-push hook, always with an empty contract and with nobody reading the output, so trace sessions were never governed by a contract. The contract name can be overridden with the new `--contract` flag on both commands. It is resolved when the workflow is created and not persisted to .chainloop.yml, so the push path is unchanged. Existing workflows are never modified. `trace init` fails before writing anything when the workflow cannot be created; `trace run` warns and runs the session anyway. Updating .chainloop.yml now preserves the user's comments, key order and indentation instead of re-marshalling the file from a map. Assisted-by: Claude Code Signed-off-by: Miguel Martinez Trivino Chainloop-Trace-Sessions: 90463ef4-5d43-4c6b-8369-d85c131c0ba8 --- app/cli/cmd/trace.go | 4 + app/cli/cmd/trace_init.go | 226 ++++++++++++----- app/cli/cmd/trace_init_test.go | 158 ++++++++++++ app/cli/cmd/trace_run.go | 34 ++- app/cli/cmd/workflow_contract_describe.go | 6 +- app/cli/cmd/workflow_create.go | 2 +- app/cli/documentation/cli-reference.mdx | 13 + app/cli/internal/trace/config/config.go | 101 +++++++- app/cli/internal/trace/config/config_test.go | 89 +++++++ app/cli/pkg/action/trace_run.go | 30 +++ app/cli/pkg/action/trace_workflow.go | 178 +++++++++++++ app/cli/pkg/action/trace_workflow_test.go | 237 ++++++++++++++++++ .../pkg/action/workflow_contract_describe.go | 6 +- app/cli/pkg/action/workflow_create.go | 4 +- .../pkg/biz/workflow_integration_test.go | 19 +- 15 files changed, 1011 insertions(+), 96 deletions(-) create mode 100644 app/cli/cmd/trace_init_test.go create mode 100644 app/cli/pkg/action/trace_workflow.go create mode 100644 app/cli/pkg/action/trace_workflow_test.go diff --git a/app/cli/cmd/trace.go b/app/cli/cmd/trace.go index d4041814d..2273910cf 100644 --- a/app/cli/cmd/trace.go +++ b/app/cli/cmd/trace.go @@ -19,6 +19,10 @@ import ( "github.com/spf13/cobra" ) +// traceContractFlagDesc documents the --contract flag `trace init` and +// `trace run` share. +const traceContractFlagDesc = `workflow contract to attach when creating the workflow (defaults to "chainloop-ai-coding-session" when the organization has it)` + // newTraceCmd creates the trace parent command with all subcommands. func newTraceCmd() *cobra.Command { cmd := &cobra.Command{ diff --git a/app/cli/cmd/trace_init.go b/app/cli/cmd/trace_init.go index a012b349c..270fd0597 100644 --- a/app/cli/cmd/trace_init.go +++ b/app/cli/cmd/trace_init.go @@ -16,6 +16,7 @@ package cmd import ( + "context" "fmt" "github.com/chainloop-dev/chainloop/app/cli/internal/trace/claude" @@ -34,6 +35,7 @@ import ( func newTraceInitCmd() *cobra.Command { var ( project string + contract string claudeFlag bool cursorFlag bool opencodeFlag bool @@ -49,6 +51,12 @@ This installs the managed git hooks plus agent-specific hooks for the selected providers. Pass --claude, --cursor, and/or --opencode to pick providers; when none is set, Claude Code is used as the default. +It also creates the Chainloop workflow the attestations target, attached to +the "chainloop-ai-coding-session" contract when your organization has it and +to a default empty contract otherwise. Pass --contract to attach a different +one. Nothing is written to the repository until the workflow exists, so you +need to be logged in with permission to create workflows. + Pass --org to pin the Chainloop organization trace attestations target; the value is saved to .chainloop.yml and overrides the CLI default on every push. @@ -61,78 +69,20 @@ attestations. The value is saved to .chainloop.yml and defaults to return err } - // If --project was given, persist it to .chainloop.yml - if project != "" { - if err := config.SaveProjectToYML(repoRoot, project); err != nil { - return err - } - logger.Info().Str("project", project).Msg("project name saved to .chainloop.yml") - } else { - project = config.LoadProjectFromYML(repoRoot) - } - if project == "" { - return fmt.Errorf("--project is required (or add projectName to .chainloop.yml)") - } - - // When the user explicitly passes the inherited persistent --org flag, - // persist it to .chainloop.yml so trace hooks target it on every push. - if cmd.Flags().Changed("org") { - orgFlag, err := cmd.Flags().GetString("org") - if err != nil { - return fmt.Errorf("reading --org flag: %w", err) - } - if err := config.SaveOrganizationToYML(repoRoot, orgFlag); err != nil { - return err - } - logger.Info().Str("organization", orgFlag).Msg("organization saved to .chainloop.yml") + cfg, err := resolveTraceInitConfig(cmd, repoRoot, project) + if err != nil { + return err } - if cmd.Flags().Changed("workflow") { - workflowFlag, err := cmd.Flags().GetString("workflow") - if err != nil { - return fmt.Errorf("reading --workflow flag: %w", err) - } - if err := config.SaveWorkflowToYML(repoRoot, workflowFlag); err != nil { - return err - } - logger.Info().Str("workflow", workflowFlag).Msg("workflow saved to .chainloop.yml") - } - workflowName := config.ResolveWorkflowName(config.LoadWorkflowFromYML(repoRoot)) - - // Persist --require-trace when explicitly set, otherwise - // load the persisted value from .chainloop.yml. - requireTrace := config.LoadRequireTraceFromYML(repoRoot) - if cmd.Flags().Changed("require-trace") { - val, err := cmd.Flags().GetBool("require-trace") - if err != nil { - return fmt.Errorf("reading --require-trace flag: %w", err) - } - requireTrace = val - if err := config.SaveRequireTraceToYML(repoRoot, requireTrace); err != nil { - return err - } + if err := ensureTraceInitWorkflow(cmd.Context(), cfg, contract); err != nil { + return err } - // Verify authentication. If the repo pins an organization, use it so - // the auth check reflects the org the hooks will actually target. - var authExecOpts []action.ExecutorOption - if forcedOrg := config.LoadOrganizationFromYML(repoRoot); forcedOrg != "" { - authExecOpts = append(authExecOpts, action.WithForcedOrganization(forcedOrg)) - } - executor, err := action.NewAttestationExecutor(ActionOpts, Version, authExecOpts...) - if err != nil { + // The workflow exists: from here on it is safe to leave hooks and + // configuration behind. + if err := cfg.save(repoRoot); err != nil { return err } - defer func() { _ = executor.Close() }() - - if err := executor.CheckAuth(cmd.Context()); err != nil { - logger.Warn().Err(err).Msg("authentication check failed") - if requireTrace { - logger.Warn().Msg("require-trace is enabled: pushes with AI-assisted commits will fail until authenticated") - } else { - logger.Warn().Msg("hooks will be installed, but attestation commands will fail until authenticated") - } - } // Create trace directory structure store := state.NewGitStore(gitDir) @@ -174,8 +124,8 @@ attestations. The value is saved to .chainloop.yml and defaults to } logger.Info(). - Str("project", project). - Str("workflow", workflowName). + Str("project", cfg.project). + Str("workflow", cfg.workflow). Strs("providers", selected). Msg("trace initialized") @@ -184,6 +134,7 @@ attestations. The value is saved to .chainloop.yml and defaults to } cmd.Flags().StringVar(&project, "project", "", "chainloop project name") + cmd.Flags().StringVar(&contract, "contract", "", traceContractFlagDesc) cmd.Flags().String("workflow", "", "chainloop workflow name used for trace attestations (defaults to \"ai-coding-session\")") cmd.Flags().Bool("require-trace", false, "block pushes when attestation fails for AI-assisted commits") cmd.Flags().BoolVar(&claudeFlag, "claude", false, "install Claude Code hooks (default when no provider flag is set)") @@ -193,6 +144,145 @@ attestations. The value is saved to .chainloop.yml and defaults to return cmd } +// ensureTraceInitWorkflow makes sure the workflow the trace attestations target +// exists, creating it up front while someone is watching the output. Left to +// the pre-push hook, the control plane creates it implicitly with an empty +// contract and no one reads the result. Anything other than the workflow +// already existing is fatal, so a failed init leaves nothing behind. +func ensureTraceInitWorkflow(ctx context.Context, cfg *traceInitConfig, contractFlag string) error { + // If the repo pins an organization, use it so the workflow lands where the + // hooks will actually attest. An empty name keeps the default connection. + executor, err := action.NewAttestationExecutor(ActionOpts, Version, action.WithForcedOrganization(cfg.organization)) + if err != nil { + return err + } + defer func() { _ = executor.Close() }() + + if err := executor.CheckAuth(ctx); err != nil { + return err + } + + contractName, contractRequired := config.ResolveContract(contractFlag) + wf, err := executor.EnsureWorkflow(ctx, action.EnsureTraceWorkflowOpts{ + ProjectName: cfg.project, + WorkflowName: cfg.workflow, + ContractName: contractName, + ContractRequired: contractRequired, + }) + if err != nil { + return err + } + + if wf.Created { + logger.Info(). + Str("workflow", cfg.workflow). + Str("contract", wf.ContractName). + Msg("workflow created") + } + + return nil +} + +// traceInitConfig is the trace setup resolved from the command flags and the +// existing .chainloop.yml. Resolving and saving are separate steps so nothing +// lands in the user's repository before the Chainloop workflow exists. +type traceInitConfig struct { + project string + workflow string + organization string + requireTrace bool + + // The save* fields mark the values the user set on this run. Only those + // are written back to .chainloop.yml, leaving the rest of the file alone. + saveProject, saveWorkflow, saveOrganization, saveRequireTrace bool +} + +// resolveTraceInitConfig reads the trace settings from the flags, falling back +// to the values already in .chainloop.yml. +func resolveTraceInitConfig(cmd *cobra.Command, repoRoot, projectFlag string) (*traceInitConfig, error) { + // The organization is read back even when the user did not pass --org, + // since it pins where the workflow is created. The other persisted values + // are only needed when they are being written. + cfg := &traceInitConfig{ + project: projectFlag, + saveProject: projectFlag != "", + organization: config.LoadOrganizationFromYML(repoRoot), + } + + if cfg.project == "" { + cfg.project = config.LoadProjectFromYML(repoRoot) + } + if cfg.project == "" { + return nil, fmt.Errorf("--project is required (or add projectName to .chainloop.yml)") + } + + // --org is inherited from the root command, so it is only meant for this + // repository when the user passed it explicitly. + if cmd.Flags().Changed("org") { + org, err := cmd.Flags().GetString("org") + if err != nil { + return nil, fmt.Errorf("reading --org flag: %w", err) + } + cfg.organization = org + cfg.saveOrganization = true + } + + workflow := config.LoadWorkflowFromYML(repoRoot) + if cmd.Flags().Changed("workflow") { + workflowFlag, err := cmd.Flags().GetString("workflow") + if err != nil { + return nil, fmt.Errorf("reading --workflow flag: %w", err) + } + workflow = workflowFlag + cfg.saveWorkflow = true + } + cfg.workflow = config.ResolveWorkflowName(workflow) + + if cmd.Flags().Changed("require-trace") { + requireTrace, err := cmd.Flags().GetBool("require-trace") + if err != nil { + return nil, fmt.Errorf("reading --require-trace flag: %w", err) + } + cfg.requireTrace = requireTrace + cfg.saveRequireTrace = true + } + + return cfg, nil +} + +// save persists the values the user passed to .chainloop.yml so the trace +// hooks target the same organization, project and workflow on every push. +func (c *traceInitConfig) save(repoRoot string) error { + if c.saveProject { + if err := config.SaveProjectToYML(repoRoot, c.project); err != nil { + return err + } + logger.Info().Str("project", c.project).Msg("project name saved to .chainloop.yml") + } + + if c.saveOrganization { + if err := config.SaveOrganizationToYML(repoRoot, c.organization); err != nil { + return err + } + logger.Info().Str("organization", c.organization).Msg("organization saved to .chainloop.yml") + } + + if c.saveWorkflow { + if err := config.SaveWorkflowToYML(repoRoot, c.workflow); err != nil { + return err + } + logger.Info().Str("workflow", c.workflow).Msg("workflow saved to .chainloop.yml") + } + + if c.saveRequireTrace { + if err := config.SaveRequireTraceToYML(repoRoot, c.requireTrace); err != nil { + return err + } + } + + return nil +} + // selectedTraceProviders resolves the provider names to install based on the // --claude, --cursor, and --opencode flags. When none is set, Claude Code is // used as the default so existing users get the same behavior. diff --git a/app/cli/cmd/trace_init_test.go b/app/cli/cmd/trace_init_test.go new file mode 100644 index 000000000..1b0bb0f69 --- /dev/null +++ b/app/cli/cmd/trace_init_test.go @@ -0,0 +1,158 @@ +// +// 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 cmd + +import ( + "os" + "path/filepath" + "slices" + "testing" + + "github.com/spf13/cobra" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// traceInitTestCmd builds the trace init command with the root command's +// persistent --org flag, which resolveTraceInitConfig reads. +func traceInitTestCmd(t *testing.T, args ...string) *cobra.Command { + t.Helper() + + cmd := newTraceInitCmd() + cmd.Flags().String("org", "", "organization name") + cmd.SetArgs(args) + require.NoError(t, cmd.ParseFlags(args)) + + return cmd +} + +func TestResolveTraceInitConfig(t *testing.T) { + const existingYML = "projectName: yml-project\norganization: yml-org\nworkflowName: yml-workflow\nrequireTrace: true\n" + + testCases := []struct { + name string + yml string + args []string + projectFlag string + + wantErr string + wantProject string + wantWorkflow string + wantOrganization string + wantRequireTrace bool + wantSaved []string + }{ + { + name: "values come from .chainloop.yml when no flag is passed", + yml: existingYML, + wantProject: "yml-project", + wantWorkflow: "yml-workflow", + // The organization pins where the workflow is created, so it is + // read back even when the user did not pass --org. requireTrace is + // not, since it is only needed when it is being written. + wantOrganization: "yml-org", + }, + { + name: "the workflow name falls back to the trace default", + yml: "projectName: yml-project\n", + wantProject: "yml-project", + wantWorkflow: "ai-coding-session", + }, + { + name: "flags win and are marked for saving", + yml: existingYML, + args: []string{"--workflow", "flag-workflow", "--org", "flag-org", "--require-trace"}, + projectFlag: "flag-project", + wantProject: "flag-project", + wantWorkflow: "flag-workflow", + wantOrganization: "flag-org", + wantRequireTrace: true, + wantSaved: []string{"project", "workflow", "organization", "requireTrace"}, + }, + { + name: "require-trace can be turned off explicitly", + yml: existingYML, + args: []string{"--require-trace=false"}, + wantProject: "yml-project", + wantWorkflow: "yml-workflow", + wantOrganization: "yml-org", + wantSaved: []string{"requireTrace"}, + }, + { + name: "a project name is required", + yml: "projectVersion: v1.0.0\n", + wantErr: "--project is required", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, ".chainloop.yml") + require.NoError(t, os.WriteFile(path, []byte(tc.yml), 0600)) + + cfg, err := resolveTraceInitConfig(traceInitTestCmd(t, tc.args...), dir, tc.projectFlag) + + // Resolving must not touch the repository: the workflow is created + // in between, and a failed init leaves nothing behind. + data, readErr := os.ReadFile(path) + require.NoError(t, readErr) + assert.Equal(t, tc.yml, string(data), "resolving must not write to .chainloop.yml") + + if tc.wantErr != "" { + assert.ErrorContains(t, err, tc.wantErr) + return + } + + require.NoError(t, err) + assert.Equal(t, tc.wantProject, cfg.project) + assert.Equal(t, tc.wantWorkflow, cfg.workflow) + assert.Equal(t, tc.wantOrganization, cfg.organization) + assert.Equal(t, tc.wantRequireTrace, cfg.requireTrace) + + saved := map[string]bool{ + "project": cfg.saveProject, + "workflow": cfg.saveWorkflow, + "organization": cfg.saveOrganization, + "requireTrace": cfg.saveRequireTrace, + } + for field, got := range saved { + assert.Equal(t, slices.Contains(tc.wantSaved, field), got, "save flag for %s", field) + } + }) + } +} + +func TestTraceInitConfigSave(t *testing.T) { + t.Run("only the values the user passed are written", func(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, ".chainloop.yml") + require.NoError(t, os.WriteFile(path, []byte("projectName: yml-project\n"), 0600)) + + cfg := &traceInitConfig{ + project: "yml-project", + workflow: "ai-coding-session", + organization: "yml-org", + saveWorkflow: true, + } + require.NoError(t, cfg.save(dir)) + + data, err := os.ReadFile(path) + require.NoError(t, err) + assert.Contains(t, string(data), "workflowName: ai-coding-session") + assert.NotContains(t, string(data), "organization:") + }) +} diff --git a/app/cli/cmd/trace_run.go b/app/cli/cmd/trace_run.go index 474e7ecba..d68187f47 100644 --- a/app/cli/cmd/trace_run.go +++ b/app/cli/cmd/trace_run.go @@ -20,6 +20,7 @@ import ( "fmt" "os" + "github.com/chainloop-dev/chainloop/app/cli/internal/trace/config" tracegit "github.com/chainloop-dev/chainloop/app/cli/internal/trace/git" "github.com/chainloop-dev/chainloop/app/cli/internal/trace/state" "github.com/chainloop-dev/chainloop/app/cli/pkg/action" @@ -34,7 +35,12 @@ trace run is isolated: it ignores .chainloop.yml entirely. The attestation identity must come from the --org, --project, --workflow flags (mandatory) and the optional --version flag. It does not read from or write to .chainloop.yml, and removes every hook and local trace artifact when the -wrapped command exits, so a session never leaks setup into the next one.` +wrapped command exits, so a session never leaks setup into the next one. + +The workflow is created before the command runs, attached to the +"chainloop-ai-coding-session" contract when your organization has it, or to +the one given with --contract. Creating it is best-effort: the session runs +either way and the attestation creates the workflow if this did not.` // newTraceRunCmd creates the `trace run` subcommand. func newTraceRunCmd() *cobra.Command { @@ -42,6 +48,7 @@ func newTraceRunCmd() *cobra.Command { projectFlag string workflowFlag string versionFlag string + contractFlag string claudeFlag bool cursorFlag bool opencodeFlag bool @@ -103,17 +110,21 @@ func newTraceRunCmd() *cobra.Command { return fmt.Errorf("--workflow is required for trace run") } + contractName, contractRequired := config.ResolveContract(contractFlag) + return action.TraceRun(cmd.Context(), logger, action.TraceRunOpts{ - Store: store, - RepoRoot: repoRoot, - Providers: selectedTraceProviders(claudeFlag, cursorFlag, opencodeFlag), - Command: args, - ProjectName: projectFlag, - Organization: organization, - WorkflowName: workflowFlag, - ProjectVersion: versionFlag, - ActionOpts: ActionOpts, - CLIVersion: Version, + Store: store, + RepoRoot: repoRoot, + Providers: selectedTraceProviders(claudeFlag, cursorFlag, opencodeFlag), + Command: args, + ProjectName: projectFlag, + Organization: organization, + WorkflowName: workflowFlag, + ProjectVersion: versionFlag, + ContractName: contractName, + ContractRequired: contractRequired, + ActionOpts: ActionOpts, + CLIVersion: Version, }) }, } @@ -121,6 +132,7 @@ func newTraceRunCmd() *cobra.Command { cmd.Flags().StringVar(&projectFlag, "project", "", "chainloop project name (required; .chainloop.yml is ignored)") cmd.Flags().StringVar(&workflowFlag, "workflow", "", "chainloop workflow name used for trace attestations (required; .chainloop.yml is ignored)") cmd.Flags().StringVar(&versionFlag, "version", "", "chainloop project version (optional; defaults to the latest version)") + cmd.Flags().StringVar(&contractFlag, "contract", "", traceContractFlagDesc) cmd.Flags().BoolVar(&claudeFlag, "claude", false, "install Claude Code hooks (default when no provider flag is set)") cmd.Flags().BoolVar(&cursorFlag, "cursor", false, "install Cursor hooks") cmd.Flags().BoolVar(&opencodeFlag, "opencode", false, "install opencode hooks") diff --git a/app/cli/cmd/workflow_contract_describe.go b/app/cli/cmd/workflow_contract_describe.go index debd9e3d8..74c3d753b 100644 --- a/app/cli/cmd/workflow_contract_describe.go +++ b/app/cli/cmd/workflow_contract_describe.go @@ -1,5 +1,5 @@ // -// Copyright 2024-2025 The Chainloop Authors. +// Copyright 2024-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. @@ -38,8 +38,8 @@ func newWorkflowContractDescribeCmd() *cobra.Command { cmd := &cobra.Command{ Use: "describe", Short: "Describe the information of the contract", - RunE: func(_ *cobra.Command, _ []string) error { - res, err := action.NewWorkflowContractDescribe(ActionOpts).Run(name, revision) + RunE: func(cmd *cobra.Command, _ []string) error { + res, err := action.NewWorkflowContractDescribe(ActionOpts).Run(cmd.Context(), name, revision) if err != nil { return err } diff --git a/app/cli/cmd/workflow_create.go b/app/cli/cmd/workflow_create.go index a7ac2b036..8e5274f06 100644 --- a/app/cli/cmd/workflow_create.go +++ b/app/cli/cmd/workflow_create.go @@ -59,7 +59,7 @@ func newWorkflowCreateCmd() *cobra.Command { } } - wf, err := action.NewWorkflowCreate(ActionOpts).Run(opts) + wf, err := action.NewWorkflowCreate(ActionOpts).Run(cmd.Context(), opts) if err != nil { if s, ok := status.FromError(err); ok { if s.Code() == codes.AlreadyExists { diff --git a/app/cli/documentation/cli-reference.mdx b/app/cli/documentation/cli-reference.mdx index 3093ca274..1572d58cd 100755 --- a/app/cli/documentation/cli-reference.mdx +++ b/app/cli/documentation/cli-reference.mdx @@ -3310,6 +3310,12 @@ This installs the managed git hooks plus agent-specific hooks for the selected providers. Pass --claude, --cursor, and/or --opencode to pick providers; when none is set, Claude Code is used as the default. +It also creates the Chainloop workflow the attestations target, attached to +the "chainloop-ai-coding-session" contract when your organization has it and +to a default empty contract otherwise. Pass --contract to attach a different +one. Nothing is written to the repository until the workflow exists, so you +need to be logged in with permission to create workflows. + Pass --org to pin the Chainloop organization trace attestations target; the value is saved to .chainloop.yml and overrides the CLI default on every push. @@ -3325,6 +3331,7 @@ Options ``` --claude install Claude Code hooks (default when no provider flag is set) +--contract string workflow contract to attach when creating the workflow (defaults to "chainloop-ai-coding-session" when the organization has it) --cursor install Cursor hooks -h, --help help for init --opencode install opencode hooks @@ -3366,6 +3373,11 @@ and the optional --version flag. It does not read from or write to .chainloop.yml, and removes every hook and local trace artifact when the wrapped command exits, so a session never leaks setup into the next one. +The workflow is created before the command runs, attached to the +"chainloop-ai-coding-session" contract when your organization has it, or to +the one given with --contract. Creating it is best-effort: the session runs +either way and the attestation creates the workflow if this did not. + ``` chainloop trace run -- [args...] [flags] ``` @@ -3374,6 +3386,7 @@ Options ``` --claude install Claude Code hooks (default when no provider flag is set) +--contract string workflow contract to attach when creating the workflow (defaults to "chainloop-ai-coding-session" when the organization has it) --cursor install Cursor hooks -h, --help help for run --opencode install opencode hooks diff --git a/app/cli/internal/trace/config/config.go b/app/cli/internal/trace/config/config.go index 06c62d97f..8d5d97ad3 100644 --- a/app/cli/internal/trace/config/config.go +++ b/app/cli/internal/trace/config/config.go @@ -16,6 +16,7 @@ package config import ( + "bytes" "fmt" "os" "path/filepath" @@ -33,6 +34,13 @@ const ( // when no override is configured in .chainloop.yml. const traceWorkflowName = "ai-coding-session" +// traceContractName is the workflow contract the trace workflow is attached to +// when the organization has it. It is the name of the library contract shipped +// with Chainloop, prefixed so it never collides with a contract of the user's +// own named after the workflow. Organizations without it get the control +// plane's default empty contract instead. +const traceContractName = "chainloop-ai-coding-session" + // ResolveWorkflowName returns the persisted workflow name when non-empty, // otherwise falls back to the trace default. Centralizes the default so the // CLI command and the hook handler stay in sync. @@ -44,6 +52,22 @@ func ResolveWorkflowName(persisted string) string { return traceWorkflowName } +// ResolveContract returns the contract to attach when the trace workflow is +// created: the one given on the command line, or the trace default. required +// reports that the name came from the user, where a missing contract is an +// error instead of something to fall back from, because a typo must not +// silently bind another contract. +// +// Unlike the workflow name, the contract is not persisted to .chainloop.yml: +// it is only needed when the workflow is created, never on the push path. +func ResolveContract(flag string) (name string, required bool) { + if flag != "" { + return flag, true + } + + return traceContractName, false +} + // ChainloopYML represents the relevant fields in .chainloop.yml. type ChainloopYML struct { ProjectName string `yaml:"projectName"` @@ -158,21 +182,26 @@ func LoadWorkflowFromYML(dir string) string { return cfg.WorkflowName } -// updateChainloopYMLField reads, updates a single field, and writes back -// the .chainloop.yml file, preserving all other fields. +// updateChainloopYMLField reads, updates a single field, and writes back the +// .chainloop.yml file. The file is edited as a YAML node tree rather than +// re-marshalled from a map, so the user's comments, key order and indentation +// survive: .chainloop.yml is checked into their repository and a rewrite that +// reformats it turns `chainloop trace init` into a noisy diff. func updateChainloopYMLField(dir, key string, value any) error { path := resolveChainloopYMLPath(dir) - existing := make(map[string]any) + var doc yaml.Node if data, err := os.ReadFile(path); err == nil { - if err := yaml.Unmarshal(data, &existing); err != nil { + if err := yaml.Unmarshal(data, &doc); err != nil { return fmt.Errorf("parse %s: %w", filepath.Base(path), err) } } - existing[key] = value + if err := setYAMLField(&doc, key, value); err != nil { + return fmt.Errorf("update %s in %s: %w", key, filepath.Base(path), err) + } - out, err := yaml.Marshal(existing) + out, err := encodeYAML(&doc) if err != nil { return fmt.Errorf("marshal %s: %w", filepath.Base(path), err) } @@ -180,6 +209,66 @@ func updateChainloopYMLField(dir, key string, value any) error { return os.WriteFile(path, out, 0600) } +// encodeYAML renders a node tree with the 2-space indentation .chainloop.yml +// uses; the encoder defaults to 4. +func encodeYAML(doc *yaml.Node) ([]byte, error) { + var out bytes.Buffer + enc := yaml.NewEncoder(&out) + enc.SetIndent(2) + if err := enc.Encode(doc); err != nil { + return nil, err + } + if err := enc.Close(); err != nil { + return nil, err + } + + return out.Bytes(), nil +} + +// setYAMLField sets key to value in the document's top-level mapping, adding +// the key at the end when it is absent. An empty document (missing or empty +// file) is initialized to a mapping. Comments attached to an overwritten value +// are kept, since they document the field rather than the value. +func setYAMLField(doc *yaml.Node, key string, value any) error { + var encoded yaml.Node + if err := encoded.Encode(value); err != nil { + return err + } + + // A missing or empty file leaves the document node zero-valued. + if len(doc.Content) == 0 { + doc.Kind = yaml.DocumentNode + doc.Content = []*yaml.Node{{Kind: yaml.MappingNode, Tag: "!!map"}} + } + + mapping := doc.Content[0] + if mapping.Kind != yaml.MappingNode { + return fmt.Errorf("expected a top-level YAML mapping") + } + + // A mapping's Content alternates key, value. + for i := 0; i+1 < len(mapping.Content); i += 2 { + if mapping.Content[i].Value != key { + continue + } + + old := mapping.Content[i+1] + encoded.HeadComment = old.HeadComment + encoded.LineComment = old.LineComment + encoded.FootComment = old.FootComment + mapping.Content[i+1] = &encoded + + return nil + } + + mapping.Content = append(mapping.Content, + &yaml.Node{Kind: yaml.ScalarNode, Tag: "!!str", Value: key}, + &encoded, + ) + + return nil +} + // LoadRequireTraceFromYML looks for .chainloop.yml starting from dir // and returns the requireTrace value. Returns false when the field is // absent or nil (default-off). diff --git a/app/cli/internal/trace/config/config_test.go b/app/cli/internal/trace/config/config_test.go index 735d55e65..563b641ec 100644 --- a/app/cli/internal/trace/config/config_test.go +++ b/app/cli/internal/trace/config/config_test.go @@ -19,6 +19,7 @@ import ( "os" "os/exec" "path/filepath" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -155,6 +156,74 @@ func TestSaveProjectToYML(t *testing.T) { }) } +// TestUpdateChainloopYMLPreservesFormatting pins that updating one field keeps +// the rest of the file as the user wrote it: comments, key order and +// indentation. .chainloop.yml is checked into the user's repository, so a +// rewrite that drops their comments shows up as noise in their next diff. +func TestUpdateChainloopYMLPreservesFormatting(t *testing.T) { + const existing = `# organization and projectName are used for Chainloop Trace +# https://docs.chainloop.dev/reference/operator/trace +organization: chainloop +projectName: old-project +# This indicates the [current version]+next +projectVersion: v1.0.0+next + +# Maps material names to a location on disk +scorecards: + - name: sarif-results + path: metadata/results.sarif +` + + testCases := []struct { + name string + update func(dir string) error + wantContain []string + }{ + { + name: "updating an existing field", + update: func(dir string) error { return SaveProjectToYML(dir, "new-project") }, + wantContain: []string{"projectName: new-project"}, + }, + { + name: "adding a new field", + update: func(dir string) error { return SaveWorkflowToYML(dir, "my-workflow") }, + wantContain: []string{"projectName: old-project", "workflowName: my-workflow"}, + }, + { + name: "adding a boolean field", + update: func(dir string) error { return SaveRequireTraceToYML(dir, true) }, + wantContain: []string{"requireTrace: true"}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, ".chainloop.yml") + require.NoError(t, os.WriteFile(path, []byte(existing), 0600)) + + require.NoError(t, tc.update(dir)) + + data, err := os.ReadFile(path) + require.NoError(t, err) + got := string(data) + + for _, want := range tc.wantContain { + assert.Contains(t, got, want) + } + + // Comments, key order and list indentation are the user's, not ours. + assert.Contains(t, got, "# organization and projectName are used for Chainloop Trace") + assert.Contains(t, got, "# https://docs.chainloop.dev/reference/operator/trace") + assert.Contains(t, got, "# This indicates the [current version]+next") + assert.Contains(t, got, "# Maps material names to a location on disk") + assert.Contains(t, got, " - name: sarif-results\n path: metadata/results.sarif") + assert.Less(t, strings.Index(got, "organization:"), strings.Index(got, "projectName:")) + assert.Less(t, strings.Index(got, "projectName:"), strings.Index(got, "scorecards:")) + }) + } +} + func TestRequireTrace(t *testing.T) { t.Run("defaults to false when field missing", func(t *testing.T) { dir := t.TempDir() @@ -264,6 +333,26 @@ func TestWorkflow(t *testing.T) { }) } +func TestResolveContract(t *testing.T) { + testCases := []struct { + name string + flag string + want string + wantRequired bool + }{ + {name: "defaults to the shipped contract, which may be absent", want: "chainloop-ai-coding-session"}, + {name: "the flag wins and is required", flag: "my-contract", want: "my-contract", wantRequired: true}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + name, required := ResolveContract(tc.flag) + assert.Equal(t, tc.want, name) + assert.Equal(t, tc.wantRequired, required) + }) + } +} + func TestFindChainloopYML(t *testing.T) { t.Run("reads both fields from .chainloop.yml", func(t *testing.T) { dir := t.TempDir() diff --git a/app/cli/pkg/action/trace_run.go b/app/cli/pkg/action/trace_run.go index 2d1bfdd93..a4f334744 100644 --- a/app/cli/pkg/action/trace_run.go +++ b/app/cli/pkg/action/trace_run.go @@ -67,6 +67,11 @@ type TraceRunOpts struct { // ProjectVersion, when set, targets a specific project version. // Empty means use the latest version. ProjectVersion string + // ContractName is the contract to attach when the workflow has to be + // created, and ContractRequired reports whether the user named it + // explicitly. See EnsureTraceWorkflowOpts. + ContractName string + ContractRequired bool // ActionOpts is the root command's initialized options, used to build // the attestation executor. Required. @@ -101,6 +106,8 @@ func TraceRun(ctx context.Context, log zerolog.Logger, opts TraceRunOpts) error } if err := executor.CheckAuth(ctx); err != nil { log.Warn().Err(err).Msg("authentication check failed; attestation will fail after the session") + } else { + ensureTraceRunWorkflow(ctx, log, executor, opts) } if err := executor.Close(); err != nil { log.Debug().Err(err).Msg("closing auth-check executor") @@ -194,6 +201,29 @@ func TraceRun(ctx context.Context, log zerolog.Logger, opts TraceRunOpts) error }) } +// ensureTraceRunWorkflow creates the workflow the session will be attested to, +// so it gets the trace contract attached instead of the empty one the control +// plane creates implicitly at push time. Unlike `trace init` this is +// best-effort: a session the user is about to run is worth more than the +// contract binding, and the attestation still creates the workflow implicitly +// if this could not. +func ensureTraceRunWorkflow(ctx context.Context, log zerolog.Logger, executor *AttestationExecutor, opts TraceRunOpts) { + wf, err := executor.EnsureWorkflow(ctx, EnsureTraceWorkflowOpts{ + ProjectName: opts.ProjectName, + WorkflowName: opts.WorkflowName, + ContractName: opts.ContractName, + ContractRequired: opts.ContractRequired, + }) + if err != nil { + log.Warn().Err(err).Msg("could not create the workflow up front; it will be created when the session is attested") + return + } + + if wf.Created { + log.Info().Str("workflow", opts.WorkflowName).Str("contract", wf.ContractName).Msg("workflow created") + } +} + // traceRunOwnsState reports whether the store's trace state is trace run's // to wipe. A pre-existing install (from trace init or an agent hook's // auto-install) owns the git hooks and the trace state dir, which holds diff --git a/app/cli/pkg/action/trace_workflow.go b/app/cli/pkg/action/trace_workflow.go new file mode 100644 index 000000000..71fd65f6c --- /dev/null +++ b/app/cli/pkg/action/trace_workflow.go @@ -0,0 +1,178 @@ +// +// 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 action + +import ( + "context" + "fmt" + + "github.com/rs/zerolog" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" +) + +// EnsureTraceWorkflowOpts identifies the workflow `chainloop trace` attests to +// and the contract it should be attached to when it has to be created. +type EnsureTraceWorkflowOpts struct { + ProjectName string + WorkflowName string + // ContractName is the contract to attach when the workflow is created. + // Empty, or a contract the organization does not have, leaves the control + // plane to create its default one. + ContractName string + // ContractRequired fails the call when ContractName cannot be found instead + // of falling back to that default. Set it when the name came from the user + // (--contract), where a typo must not silently bind another contract. + ContractRequired bool +} + +// TraceWorkflowResult reports what EnsureWorkflow did. +type TraceWorkflowResult struct { + // Created is true when this call created the workflow. + Created bool + // ContractName is the contract the workflow is attached to. It is empty + // when the workflow was created concurrently by someone else, since the + // call does not read it back. + ContractName string +} + +// traceWorkflowAPI is the slice of the control-plane API that +// ensureTraceWorkflow drives, so the decision logic can be tested without a +// live control plane. +type traceWorkflowAPI interface { + // viewWorkflow returns the workflow, or a NotFound error when the workflow + // or its project does not exist. + viewWorkflow(ctx context.Context, project, workflow string) (*WorkflowItem, error) + // contractExists reports whether a contract with that name exists in the + // organization. A lookup failure is returned as an error, not as false. + contractExists(ctx context.Context, name string) (bool, error) + // createWorkflow creates the workflow, attached to contract when non-empty. + createWorkflow(ctx context.Context, project, workflow, contract string) (*WorkflowItem, error) +} + +// EnsureWorkflow makes sure the workflow trace attestations target exists, +// creating it attached to opts.ContractName when the organization has that +// contract. `chainloop trace` calls it so the contract decision happens once, +// with a human watching, instead of on the push path: the pre-push hook would +// otherwise let the control plane create the workflow implicitly with an empty +// contract, with nobody reading the output. +// +// It runs over the executor's connection, so it targets the organization pinned +// with WithForcedOrganization like every other trace operation. +func (e *AttestationExecutor) EnsureWorkflow(ctx context.Context, opts EnsureTraceWorkflowOpts) (*TraceWorkflowResult, error) { + return ensureTraceWorkflow(ctx, &cpTraceWorkflowAPI{cfg: e.actionOpts}, e.actionOpts.Logger, opts) +} + +func ensureTraceWorkflow(ctx context.Context, api traceWorkflowAPI, log zerolog.Logger, opts EnsureTraceWorkflowOpts) (*TraceWorkflowResult, error) { + if opts.ProjectName == "" || opts.WorkflowName == "" { + return nil, fmt.Errorf("project and workflow names are required") + } + + wf, err := api.viewWorkflow(ctx, opts.ProjectName, opts.WorkflowName) + if err == nil { + log.Debug().Str("workflow", opts.WorkflowName).Str("contract", wf.ContractName).Msg("trace workflow already exists") + return &TraceWorkflowResult{ContractName: wf.ContractName}, nil + } + if status.Code(err) != codes.NotFound { + return nil, authAwareErr(fmt.Errorf("looking up workflow %q in project %q: %w", opts.WorkflowName, opts.ProjectName, err)) + } + + contract, err := resolveTraceContract(ctx, api, log, opts) + if err != nil { + return nil, err + } + + created, err := api.createWorkflow(ctx, opts.ProjectName, opts.WorkflowName, contract) + if err != nil { + // Someone else created it in the meantime, which is all this call + // promises. Their contract choice stands. + if status.Code(err) == codes.AlreadyExists { + log.Debug().Str("workflow", opts.WorkflowName).Msg("trace workflow was created concurrently") + return &TraceWorkflowResult{}, nil + } + + return nil, authAwareErr(fmt.Errorf("creating workflow %q in project %q: %w", opts.WorkflowName, opts.ProjectName, err)) + } + + return &TraceWorkflowResult{Created: true, ContractName: created.ContractName}, nil +} + +// resolveTraceContract returns the contract name to attach on creation: the +// requested one when the organization has it, or an empty string to let the +// control plane create its per-project default. A contract that cannot be found +// is only an error when the caller marked it required, because the shipped +// default is absent in organizations that never imported it. +func resolveTraceContract(ctx context.Context, api traceWorkflowAPI, log zerolog.Logger, opts EnsureTraceWorkflowOpts) (string, error) { + if opts.ContractName == "" { + return "", nil + } + + exists, err := api.contractExists(ctx, opts.ContractName) + switch { + case err != nil && opts.ContractRequired: + return "", authAwareErr(fmt.Errorf("looking up contract %q: %w", opts.ContractName, err)) + case err != nil: + log.Debug().Err(err).Str("contract", opts.ContractName).Msg("could not look up the contract; falling back to the default one") + return "", nil + case !exists && opts.ContractRequired: + return "", fmt.Errorf("contract %q does not exist in this organization", opts.ContractName) + case !exists: + log.Debug().Str("contract", opts.ContractName).Msg("contract not found; falling back to the default one") + return "", nil + } + + return opts.ContractName, nil +} + +// authAwareErr prefixes an unauthenticated failure with the way out, since +// running trace before logging in is the most likely reason to get one. +func authAwareErr(err error) error { + if status.Code(err) != codes.Unauthenticated { + return err + } + + return fmt.Errorf("chainloop is not authenticated; run 'chainloop auth login' first: %w", err) +} + +// cpTraceWorkflowAPI implements traceWorkflowAPI against the control plane, +// reusing the actions the equivalent commands run. +type cpTraceWorkflowAPI struct { + cfg *ActionsOpts +} + +func (a *cpTraceWorkflowAPI) viewWorkflow(ctx context.Context, project, workflow string) (*WorkflowItem, error) { + return NewWorkflowDescribe(a.cfg).Run(ctx, workflow, project) +} + +func (a *cpTraceWorkflowAPI) contractExists(ctx context.Context, name string) (bool, error) { + if _, err := NewWorkflowContractDescribe(a.cfg).Run(ctx, name, 0); err != nil { + if status.Code(err) == codes.NotFound { + return false, nil + } + + return false, err + } + + return true, nil +} + +func (a *cpTraceWorkflowAPI) createWorkflow(ctx context.Context, project, workflow, contract string) (*WorkflowItem, error) { + return NewWorkflowCreate(a.cfg).Run(ctx, &NewWorkflowCreateOpts{ + Name: workflow, + Project: project, + ContractName: contract, + }) +} diff --git a/app/cli/pkg/action/trace_workflow_test.go b/app/cli/pkg/action/trace_workflow_test.go new file mode 100644 index 000000000..8fa2caacb --- /dev/null +++ b/app/cli/pkg/action/trace_workflow_test.go @@ -0,0 +1,237 @@ +// +// 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 action + +import ( + "context" + "errors" + "testing" + + "github.com/rs/zerolog" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" +) + +// fakeTraceWorkflowAPI records the calls ensureTraceWorkflow makes and returns +// canned answers, so the decision logic is testable without a control plane. +type fakeTraceWorkflowAPI struct { + viewResult *WorkflowItem + viewErr error + + contractFound bool + contractErr error + + createErr error + + contractCalls int + createCalls int + createdWith string +} + +func (f *fakeTraceWorkflowAPI) viewWorkflow(_ context.Context, _, _ string) (*WorkflowItem, error) { + return f.viewResult, f.viewErr +} + +func (f *fakeTraceWorkflowAPI) contractExists(_ context.Context, _ string) (bool, error) { + f.contractCalls++ + return f.contractFound, f.contractErr +} + +// defaultContract stands in for the empty contract the control plane creates +// when a create request carries no contract name. +const defaultContract = "default-contract" + +func (f *fakeTraceWorkflowAPI) createWorkflow(_ context.Context, _, _, contract string) (*WorkflowItem, error) { + f.createCalls++ + f.createdWith = contract + if f.createErr != nil { + return nil, f.createErr + } + + // The control plane echoes back the contract it attached, naming its own + // default when the request carried none. + if contract == "" { + contract = defaultContract + } + + return &WorkflowItem{ContractName: contract}, nil +} + +func TestEnsureTraceWorkflow(t *testing.T) { + const ( + project = "my-project" + workflow = "ai-coding-session" + contract = "chainloop-ai-coding-session" + ) + + notFound := status.Error(codes.NotFound, "not found") + const forbidden = "forbidden" + denied := status.Error(codes.PermissionDenied, forbidden) + + testCases := []struct { + name string + api *fakeTraceWorkflowAPI + opts EnsureTraceWorkflowOpts + + wantErr string + wantCreated bool + wantContract string + wantContractCall bool + wantCreateCall bool + // wantCreatedWith is the contract name sent on create; empty means the + // control plane picks its default. + wantCreatedWith string + }{ + { + name: "existing workflow is left alone", + api: &fakeTraceWorkflowAPI{viewResult: &WorkflowItem{ContractName: "some-contract"}}, + opts: EnsureTraceWorkflowOpts{ProjectName: project, WorkflowName: workflow, ContractName: contract}, + wantContract: "some-contract", + }, + { + name: "missing workflow with an existing contract is created attached to it", + api: &fakeTraceWorkflowAPI{viewErr: notFound, contractFound: true}, + opts: EnsureTraceWorkflowOpts{ProjectName: project, WorkflowName: workflow, ContractName: contract}, + wantCreated: true, + wantContract: contract, + wantContractCall: true, + wantCreateCall: true, + wantCreatedWith: contract, + }, + { + name: "missing contract falls back to the control plane default", + api: &fakeTraceWorkflowAPI{viewErr: notFound}, + opts: EnsureTraceWorkflowOpts{ProjectName: project, WorkflowName: workflow, ContractName: contract}, + wantCreated: true, + wantContract: defaultContract, + wantContractCall: true, + wantCreateCall: true, + }, + { + name: "a contract lookup failure also falls back", + api: &fakeTraceWorkflowAPI{viewErr: notFound, contractErr: denied}, + opts: EnsureTraceWorkflowOpts{ProjectName: project, WorkflowName: workflow, ContractName: contract}, + wantCreated: true, + wantContract: defaultContract, + wantContractCall: true, + wantCreateCall: true, + }, + { + name: "a required contract that does not exist fails", + api: &fakeTraceWorkflowAPI{viewErr: notFound}, + opts: EnsureTraceWorkflowOpts{ProjectName: project, WorkflowName: workflow, ContractName: "typo", ContractRequired: true}, + wantErr: `contract "typo" does not exist`, + wantContractCall: true, + }, + { + name: "a required contract that cannot be looked up fails", + api: &fakeTraceWorkflowAPI{viewErr: notFound, contractErr: denied}, + opts: EnsureTraceWorkflowOpts{ProjectName: project, WorkflowName: workflow, ContractName: "typo", ContractRequired: true}, + wantErr: forbidden, + wantContractCall: true, + }, + { + name: "no contract requested skips the lookup", + api: &fakeTraceWorkflowAPI{viewErr: notFound}, + opts: EnsureTraceWorkflowOpts{ProjectName: project, WorkflowName: workflow}, + wantCreated: true, + wantContract: defaultContract, + wantCreateCall: true, + }, + { + name: "a workflow created concurrently is not an error", + api: &fakeTraceWorkflowAPI{viewErr: notFound, contractFound: true, createErr: status.Error(codes.AlreadyExists, "already exists")}, + opts: EnsureTraceWorkflowOpts{ProjectName: project, WorkflowName: workflow, ContractName: contract}, + wantContractCall: true, + wantCreateCall: true, + wantCreatedWith: contract, + }, + { + name: "a view failure other than not-found fails", + api: &fakeTraceWorkflowAPI{viewErr: denied}, + opts: EnsureTraceWorkflowOpts{ProjectName: project, WorkflowName: workflow, ContractName: contract}, + wantErr: forbidden, + }, + { + name: "an unauthenticated view fails with a hint", + api: &fakeTraceWorkflowAPI{viewErr: status.Error(codes.Unauthenticated, "no token")}, + opts: EnsureTraceWorkflowOpts{ProjectName: project, WorkflowName: workflow}, + wantErr: "not authenticated", + }, + { + name: "a create failure fails", + api: &fakeTraceWorkflowAPI{viewErr: notFound, contractFound: true, createErr: denied}, + opts: EnsureTraceWorkflowOpts{ProjectName: project, WorkflowName: workflow, ContractName: contract}, + wantErr: forbidden, + wantContractCall: true, + wantCreateCall: true, + wantCreatedWith: contract, + }, + { + name: "the project name is required", + api: &fakeTraceWorkflowAPI{}, + opts: EnsureTraceWorkflowOpts{WorkflowName: workflow}, + wantErr: "project and workflow names are required", + }, + { + name: "the workflow name is required", + api: &fakeTraceWorkflowAPI{}, + opts: EnsureTraceWorkflowOpts{ProjectName: project}, + wantErr: "project and workflow names are required", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + got, err := ensureTraceWorkflow(context.Background(), tc.api, zerolog.Nop(), tc.opts) + + if tc.wantErr != "" { + require.Error(t, err) + assert.ErrorContains(t, err, tc.wantErr) + assert.Nil(t, got) + } else { + require.NoError(t, err) + require.NotNil(t, got) + assert.Equal(t, tc.wantCreated, got.Created) + assert.Equal(t, tc.wantContract, got.ContractName) + } + + assert.Equal(t, tc.wantContractCall, tc.api.contractCalls > 0, "contract lookup calls") + assert.Equal(t, tc.wantCreateCall, tc.api.createCalls > 0, "create calls") + if tc.wantCreateCall { + assert.Equal(t, tc.wantCreatedWith, tc.api.createdWith) + } + }) + } + + t.Run("errors keep the underlying gRPC status", func(t *testing.T) { + api := &fakeTraceWorkflowAPI{viewErr: denied} + _, err := ensureTraceWorkflow(context.Background(), api, zerolog.Nop(), EnsureTraceWorkflowOpts{ProjectName: project, WorkflowName: workflow}) + require.Error(t, err) + assert.Equal(t, codes.PermissionDenied, status.Code(err)) + }) + + t.Run("a non-status contract error still falls back", func(t *testing.T) { + api := &fakeTraceWorkflowAPI{viewErr: status.Error(codes.NotFound, "not found"), contractErr: errors.New("boom")} + got, err := ensureTraceWorkflow(context.Background(), api, zerolog.Nop(), EnsureTraceWorkflowOpts{ProjectName: project, WorkflowName: workflow, ContractName: contract}) + require.NoError(t, err) + assert.True(t, got.Created) + assert.Equal(t, 1, api.createCalls) + assert.Empty(t, api.createdWith) + }) +} diff --git a/app/cli/pkg/action/workflow_contract_describe.go b/app/cli/pkg/action/workflow_contract_describe.go index 8759ba821..71b301075 100644 --- a/app/cli/pkg/action/workflow_contract_describe.go +++ b/app/cli/pkg/action/workflow_contract_describe.go @@ -1,5 +1,5 @@ // -// Copyright 2024 The Chainloop Authors. +// Copyright 2024-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. @@ -34,12 +34,12 @@ func NewWorkflowContractDescribe(cfg *ActionsOpts) *WorkflowContractDescribe { return &WorkflowContractDescribe{cfg} } -func (action *WorkflowContractDescribe) Run(name string, rev int32) (*WorkflowContractWithVersionItem, error) { +func (action *WorkflowContractDescribe) Run(ctx context.Context, name string, rev int32) (*WorkflowContractWithVersionItem, error) { client := pb.NewWorkflowContractServiceClient(action.cfg.CPConnection) request := &pb.WorkflowContractServiceDescribeRequest{Name: name, Revision: rev} - resp, err := client.Describe(context.Background(), request) + resp, err := client.Describe(ctx, request) if err != nil { action.cfg.Logger.Debug().Err(err).Msg("making the API request") return nil, err diff --git a/app/cli/pkg/action/workflow_create.go b/app/cli/pkg/action/workflow_create.go index b224d67eb..ed00f2035 100644 --- a/app/cli/pkg/action/workflow_create.go +++ b/app/cli/pkg/action/workflow_create.go @@ -37,9 +37,9 @@ type NewWorkflowCreateOpts struct { WorkflowTemplateID string } -func (action *WorkflowCreate) Run(opts *NewWorkflowCreateOpts) (*WorkflowItem, error) { +func (action *WorkflowCreate) Run(ctx context.Context, opts *NewWorkflowCreateOpts) (*WorkflowItem, error) { client := pb.NewWorkflowServiceClient(action.cfg.CPConnection) - resp, err := client.Create(context.Background(), newWorkflowCreateRequest(opts)) + resp, err := client.Create(ctx, newWorkflowCreateRequest(opts)) if err != nil { return nil, err } diff --git a/app/controlplane/pkg/biz/workflow_integration_test.go b/app/controlplane/pkg/biz/workflow_integration_test.go index 2e2cbb684..b2ce76de2 100644 --- a/app/controlplane/pkg/biz/workflow_integration_test.go +++ b/app/controlplane/pkg/biz/workflow_integration_test.go @@ -124,6 +124,10 @@ func (s *workflowIntegrationTestSuite) TestCreate() { name string opts *biz.WorkflowCreateOpts wantErrMsg string + // wantContractName asserts which contract the workflow ends up + // attached to. Empty skips the check, since the default contract name + // is derived from the project and workflow names. + wantContractName string }{ { name: "org missing", @@ -160,8 +164,16 @@ func (s *workflowIntegrationTestSuite) TestCreate() { opts: &biz.WorkflowCreateOpts{OrgID: s.org.ID, Name: "name", Project: project}, }, { - name: "it can connect to existing contract by providing its name", - opts: &biz.WorkflowCreateOpts{OrgID: s.org.ID, Name: "name2", Project: project, ContractName: "contract-1"}, + name: "it can connect to existing contract by providing its name", + opts: &biz.WorkflowCreateOpts{OrgID: s.org.ID, Name: "name2", Project: project, ContractName: "contract-1"}, + wantContractName: "contract-1", + }, + { + // What `chainloop trace init` relies on: an org-level contract can + // be attached while the project is still being created. + name: "an org-level contract can be attached to a workflow in a brand-new project", + opts: &biz.WorkflowCreateOpts{OrgID: s.org.ID, Name: "name4", Project: "brand-new-project", ContractName: "contract-1"}, + wantContractName: "contract-1", }, { name: "can create it with just the name, the project and the org", @@ -210,6 +222,9 @@ func (s *workflowIntegrationTestSuite) TestCreate() { s.Equal(tc.opts.Project, got.Project) s.NotEmpty(got.ContractID) s.NotEmpty(got.ContractName) + if tc.wantContractName != "" { + s.Equal(tc.wantContractName, got.ContractName) + } // There is a project version created pv, err := s.ProjectVersion.FindByProjectAndVersion(ctx, got.ProjectID.String(), biz.DefaultVersionName) s.NoError(err) From e7b8163b906faf01eb959afe079986ea4dc9e4a7 Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Tue, 8 Sep 2026 18:01:52 +0200 Subject: [PATCH 2/3] fix(cli): render authentication failures the same way everywhere `chainloop trace init` reported a missing credential as "authentication error: chainloop is not authenticated; run 'chainloop config save' first: rpc error: ..." while `chainloop workflow list` reported the same condition as `authentication required, please run "chainloop auth login"`. The mapping in main.go matched on the gRPC status message, which grpc rewrites with the whole error chain as soon as the error is wrapped, so the trace path never matched and fell through to the generic branch. The advice was wrong too: `config save` sets the endpoint, not the credentials. The mapping now lives in action.AuthErrorMessage, which digs the status out of a wrapped error, and is the only place that renders a credential problem: the top-level handler uses it, and so does `trace run`, which logs the failure rather than returning it. Also addresses the review of the previous commit: - `trace run` fails instead of warning when a contract named with --contract cannot be attached, since the attestation path carries no contract name and would silently bind the default one. - .chainloop.yml keeps the indentation width it already uses instead of being rewritten with two spaces. - The formatting test uses a non-alphabetical fixture and compares the whole file, so a map round-trip cannot pass unnoticed. - Shorter command help; the flag descriptions carry the details. Assisted-by: Claude Code Signed-off-by: Miguel Martinez Trivino Chainloop-Trace-Sessions: 90463ef4-5d43-4c6b-8369-d85c131c0ba8 Signed-off-by: Miguel Martinez Trivino --- app/cli/cmd/trace.go | 2 +- app/cli/cmd/trace_init.go | 23 +- app/cli/cmd/trace_run.go | 7 +- app/cli/documentation/cli-reference.mdx | 32 +-- app/cli/internal/trace/config/config.go | 47 +++- app/cli/internal/trace/config/config_test.go | 145 ++++++++--- app/cli/main.go | 47 +--- app/cli/main_test.go | 246 ------------------- app/cli/pkg/action/auth_error.go | 67 +++++ app/cli/pkg/action/auth_error_test.go | 169 +++++++++++++ app/cli/pkg/action/trace_attestation.go | 6 +- app/cli/pkg/action/trace_run.go | 47 +++- app/cli/pkg/action/trace_workflow.go | 16 +- app/cli/pkg/action/trace_workflow_test.go | 30 ++- 14 files changed, 486 insertions(+), 398 deletions(-) delete mode 100644 app/cli/main_test.go create mode 100644 app/cli/pkg/action/auth_error.go create mode 100644 app/cli/pkg/action/auth_error_test.go diff --git a/app/cli/cmd/trace.go b/app/cli/cmd/trace.go index 2273910cf..a653d72be 100644 --- a/app/cli/cmd/trace.go +++ b/app/cli/cmd/trace.go @@ -21,7 +21,7 @@ import ( // traceContractFlagDesc documents the --contract flag `trace init` and // `trace run` share. -const traceContractFlagDesc = `workflow contract to attach when creating the workflow (defaults to "chainloop-ai-coding-session" when the organization has it)` +const traceContractFlagDesc = `workflow contract to attach when creating the workflow; it must already exist (default: "chainloop-ai-coding-session" when the organization has it, an empty contract otherwise)` // newTraceCmd creates the trace parent command with all subcommands. func newTraceCmd() *cobra.Command { diff --git a/app/cli/cmd/trace_init.go b/app/cli/cmd/trace_init.go index 270fd0597..f4d141ef5 100644 --- a/app/cli/cmd/trace_init.go +++ b/app/cli/cmd/trace_init.go @@ -47,22 +47,13 @@ func newTraceInitCmd() *cobra.Command { Long: `Initialize git hooks that automatically trace AI coding sessions and create Chainloop attestations when you push. -This installs the managed git hooks plus agent-specific hooks for the -selected providers. Pass --claude, --cursor, and/or --opencode to pick -providers; when none is set, Claude Code is used as the default. - -It also creates the Chainloop workflow the attestations target, attached to -the "chainloop-ai-coding-session" contract when your organization has it and -to a default empty contract otherwise. Pass --contract to attach a different -one. Nothing is written to the repository until the workflow exists, so you -need to be logged in with permission to create workflows. - -Pass --org to pin the Chainloop organization trace attestations target; the -value is saved to .chainloop.yml and overrides the CLI default on every push. - -Pass --workflow to override the workflow name used when initializing trace -attestations. The value is saved to .chainloop.yml and defaults to -"ai-coding-session" when unset.`, +It installs the managed git hooks plus the hooks of the selected agent +providers (Claude Code when none is given), and creates the Chainloop workflow +the attestations target. Nothing is written to the repository until that +workflow exists, so you need to be logged in. + +The organization, project, workflow and require-trace values are saved to +.chainloop.yml, and every push reads them from there.`, RunE: func(cmd *cobra.Command, _ []string) error { gitDir, repoRoot, err := tracegit.FindGitDirAndRoot() if err != nil { diff --git a/app/cli/cmd/trace_run.go b/app/cli/cmd/trace_run.go index d68187f47..3659b717a 100644 --- a/app/cli/cmd/trace_run.go +++ b/app/cli/cmd/trace_run.go @@ -37,10 +37,9 @@ and the optional --version flag. It does not read from or write to .chainloop.yml, and removes every hook and local trace artifact when the wrapped command exits, so a session never leaks setup into the next one. -The workflow is created before the command runs, attached to the -"chainloop-ai-coding-session" contract when your organization has it, or to -the one given with --contract. Creating it is best-effort: the session runs -either way and the attestation creates the workflow if this did not.` +The workflow is created before the command runs. That part is best-effort: the +session runs either way and the attestation creates the workflow if this did +not, except with --contract, which trace run cannot honor later.` // newTraceRunCmd creates the `trace run` subcommand. func newTraceRunCmd() *cobra.Command { diff --git a/app/cli/documentation/cli-reference.mdx b/app/cli/documentation/cli-reference.mdx index 1572d58cd..bf1041a5f 100755 --- a/app/cli/documentation/cli-reference.mdx +++ b/app/cli/documentation/cli-reference.mdx @@ -3306,22 +3306,13 @@ Synopsis Initialize git hooks that automatically trace AI coding sessions and create Chainloop attestations when you push. -This installs the managed git hooks plus agent-specific hooks for the -selected providers. Pass --claude, --cursor, and/or --opencode to pick -providers; when none is set, Claude Code is used as the default. +It installs the managed git hooks plus the hooks of the selected agent +providers (Claude Code when none is given), and creates the Chainloop workflow +the attestations target. Nothing is written to the repository until that +workflow exists, so you need to be logged in. -It also creates the Chainloop workflow the attestations target, attached to -the "chainloop-ai-coding-session" contract when your organization has it and -to a default empty contract otherwise. Pass --contract to attach a different -one. Nothing is written to the repository until the workflow exists, so you -need to be logged in with permission to create workflows. - -Pass --org to pin the Chainloop organization trace attestations target; the -value is saved to .chainloop.yml and overrides the CLI default on every push. - -Pass --workflow to override the workflow name used when initializing trace -attestations. The value is saved to .chainloop.yml and defaults to -"ai-coding-session" when unset. +The organization, project, workflow and require-trace values are saved to +.chainloop.yml, and every push reads them from there. ``` chainloop trace init [flags] @@ -3331,7 +3322,7 @@ Options ``` --claude install Claude Code hooks (default when no provider flag is set) ---contract string workflow contract to attach when creating the workflow (defaults to "chainloop-ai-coding-session" when the organization has it) +--contract string workflow contract to attach when creating the workflow; it must already exist (default: "chainloop-ai-coding-session" when the organization has it, an empty contract otherwise) --cursor install Cursor hooks -h, --help help for init --opencode install opencode hooks @@ -3373,10 +3364,9 @@ and the optional --version flag. It does not read from or write to .chainloop.yml, and removes every hook and local trace artifact when the wrapped command exits, so a session never leaks setup into the next one. -The workflow is created before the command runs, attached to the -"chainloop-ai-coding-session" contract when your organization has it, or to -the one given with --contract. Creating it is best-effort: the session runs -either way and the attestation creates the workflow if this did not. +The workflow is created before the command runs. That part is best-effort: the +session runs either way and the attestation creates the workflow if this did +not, except with --contract, which trace run cannot honor later. ``` chainloop trace run -- [args...] [flags] @@ -3386,7 +3376,7 @@ Options ``` --claude install Claude Code hooks (default when no provider flag is set) ---contract string workflow contract to attach when creating the workflow (defaults to "chainloop-ai-coding-session" when the organization has it) +--contract string workflow contract to attach when creating the workflow; it must already exist (default: "chainloop-ai-coding-session" when the organization has it, an empty contract otherwise) --cursor install Cursor hooks -h, --help help for run --opencode install opencode hooks diff --git a/app/cli/internal/trace/config/config.go b/app/cli/internal/trace/config/config.go index 8d5d97ad3..b86a38775 100644 --- a/app/cli/internal/trace/config/config.go +++ b/app/cli/internal/trace/config/config.go @@ -20,6 +20,7 @@ import ( "fmt" "os" "path/filepath" + "strings" tracegit "github.com/chainloop-dev/chainloop/app/cli/internal/trace/git" "gopkg.in/yaml.v3" @@ -190,18 +191,19 @@ func LoadWorkflowFromYML(dir string) string { func updateChainloopYMLField(dir, key string, value any) error { path := resolveChainloopYMLPath(dir) + // A missing file is the same as an empty document: setYAMLField starts one. + data, _ := os.ReadFile(path) + var doc yaml.Node - if data, err := os.ReadFile(path); err == nil { - if err := yaml.Unmarshal(data, &doc); err != nil { - return fmt.Errorf("parse %s: %w", filepath.Base(path), err) - } + if err := yaml.Unmarshal(data, &doc); err != nil { + return fmt.Errorf("parse %s: %w", filepath.Base(path), err) } if err := setYAMLField(&doc, key, value); err != nil { return fmt.Errorf("update %s in %s: %w", key, filepath.Base(path), err) } - out, err := encodeYAML(&doc) + out, err := encodeYAML(&doc, yamlIndent(data)) if err != nil { return fmt.Errorf("marshal %s: %w", filepath.Base(path), err) } @@ -209,12 +211,11 @@ func updateChainloopYMLField(dir, key string, value any) error { return os.WriteFile(path, out, 0600) } -// encodeYAML renders a node tree with the 2-space indentation .chainloop.yml -// uses; the encoder defaults to 4. -func encodeYAML(doc *yaml.Node) ([]byte, error) { +// encodeYAML renders a node tree with the given indentation. +func encodeYAML(doc *yaml.Node, indent int) ([]byte, error) { var out bytes.Buffer enc := yaml.NewEncoder(&out) - enc.SetIndent(2) + enc.SetIndent(indent) if err := enc.Encode(doc); err != nil { return nil, err } @@ -225,6 +226,34 @@ func encodeYAML(doc *yaml.Node) ([]byte, error) { return out.Bytes(), nil } +const ( + // defaultYAMLIndent is what .chainloop.yml uses; the encoder would + // otherwise widen to 4. + defaultYAMLIndent = 2 + // maxYAMLIndent bounds what is read as indentation, so a deeply indented + // continuation line inside a block scalar cannot widen the whole file. + maxYAMLIndent = 8 +) + +// yamlIndent reports the indentation width src already uses, taken from its +// first indented line, so nested blocks are written back the way the user +// wrote them rather than reindented to our own taste. Files with nothing +// nested (and new ones) get defaultYAMLIndent. +func yamlIndent(src []byte) int { + for _, line := range strings.Split(string(src), "\n") { + // YAML forbids tabs in indentation, so counting spaces is enough. + body := strings.TrimLeft(line, " ") + width := len(line) - len(body) + if body == "" || width == 0 || width > maxYAMLIndent { + continue + } + + return width + } + + return defaultYAMLIndent +} + // setYAMLField sets key to value in the document's top-level mapping, adding // the key at the end when it is absent. An empty document (missing or empty // file) is initialized to a mapping. Comments attached to an overwritten value diff --git a/app/cli/internal/trace/config/config_test.go b/app/cli/internal/trace/config/config_test.go index 563b641ec..36c5daeda 100644 --- a/app/cli/internal/trace/config/config_test.go +++ b/app/cli/internal/trace/config/config_test.go @@ -19,7 +19,6 @@ import ( "os" "os/exec" "path/filepath" - "strings" "testing" "github.com/stretchr/testify/assert" @@ -159,15 +158,17 @@ func TestSaveProjectToYML(t *testing.T) { // TestUpdateChainloopYMLPreservesFormatting pins that updating one field keeps // the rest of the file as the user wrote it: comments, key order and // indentation. .chainloop.yml is checked into the user's repository, so a -// rewrite that drops their comments shows up as noise in their next diff. +// rewrite that reformats it shows up as noise in their next diff. The fixture +// is deliberately in non-alphabetical order, so re-marshalling through a map +// cannot pass unnoticed, and the whole output is compared. func TestUpdateChainloopYMLPreservesFormatting(t *testing.T) { - const existing = `# organization and projectName are used for Chainloop Trace + // projectVersion, projectName, organization: reverse-alphabetical. + const existing = `# This indicates the [current version]+next +projectVersion: v1.0.0+next +projectName: old-project +# organization and projectName are used for Chainloop Trace # https://docs.chainloop.dev/reference/operator/trace organization: chainloop -projectName: old-project -# This indicates the [current version]+next -projectVersion: v1.0.0+next - # Maps material names to a location on disk scorecards: - name: sarif-results @@ -175,24 +176,56 @@ scorecards: ` testCases := []struct { - name string - update func(dir string) error - wantContain []string + name string + update func(dir string) error + want string }{ { - name: "updating an existing field", - update: func(dir string) error { return SaveProjectToYML(dir, "new-project") }, - wantContain: []string{"projectName: new-project"}, + name: "updating an existing field touches only that value", + update: func(dir string) error { return SaveProjectToYML(dir, "new-project") }, + want: `# This indicates the [current version]+next +projectVersion: v1.0.0+next +projectName: new-project +# organization and projectName are used for Chainloop Trace +# https://docs.chainloop.dev/reference/operator/trace +organization: chainloop +# Maps material names to a location on disk +scorecards: + - name: sarif-results + path: metadata/results.sarif +`, }, { - name: "adding a new field", - update: func(dir string) error { return SaveWorkflowToYML(dir, "my-workflow") }, - wantContain: []string{"projectName: old-project", "workflowName: my-workflow"}, + name: "a new field is appended", + update: func(dir string) error { return SaveWorkflowToYML(dir, "my-workflow") }, + want: `# This indicates the [current version]+next +projectVersion: v1.0.0+next +projectName: old-project +# organization and projectName are used for Chainloop Trace +# https://docs.chainloop.dev/reference/operator/trace +organization: chainloop +# Maps material names to a location on disk +scorecards: + - name: sarif-results + path: metadata/results.sarif +workflowName: my-workflow +`, }, { - name: "adding a boolean field", - update: func(dir string) error { return SaveRequireTraceToYML(dir, true) }, - wantContain: []string{"requireTrace: true"}, + name: "a new boolean field is appended", + update: func(dir string) error { return SaveRequireTraceToYML(dir, true) }, + want: `# This indicates the [current version]+next +projectVersion: v1.0.0+next +projectName: old-project +# organization and projectName are used for Chainloop Trace +# https://docs.chainloop.dev/reference/operator/trace +organization: chainloop +# Maps material names to a location on disk +scorecards: + - name: sarif-results + path: metadata/results.sarif +requireTrace: true +`, }, } @@ -206,20 +239,66 @@ scorecards: data, err := os.ReadFile(path) require.NoError(t, err) - got := string(data) - - for _, want := range tc.wantContain { - assert.Contains(t, got, want) - } - - // Comments, key order and list indentation are the user's, not ours. - assert.Contains(t, got, "# organization and projectName are used for Chainloop Trace") - assert.Contains(t, got, "# https://docs.chainloop.dev/reference/operator/trace") - assert.Contains(t, got, "# This indicates the [current version]+next") - assert.Contains(t, got, "# Maps material names to a location on disk") - assert.Contains(t, got, " - name: sarif-results\n path: metadata/results.sarif") - assert.Less(t, strings.Index(got, "organization:"), strings.Index(got, "projectName:")) - assert.Less(t, strings.Index(got, "projectName:"), strings.Index(got, "scorecards:")) + assert.Equal(t, tc.want, string(data)) + }) + } +} + +// TestUpdateChainloopYMLKeepsIndentation pins that nested blocks keep the +// indentation width the file already uses, rather than being reindented to +// whatever this package prefers. +func TestUpdateChainloopYMLKeepsIndentation(t *testing.T) { + testCases := []struct { + name string + existing string + want string + }{ + { + name: "two spaces", + existing: `projectName: p +scorecards: + - name: sarif + path: results.sarif +`, + want: `projectName: p +scorecards: + - name: sarif + path: results.sarif +workflowName: w +`, + }, + { + name: "four spaces", + existing: `projectName: p +scorecards: + - name: sarif + path: results.sarif +`, + want: `projectName: p +scorecards: + - name: sarif + path: results.sarif +workflowName: w +`, + }, + { + name: "nothing nested falls back to two", + existing: "projectName: p\n", + want: "projectName: p\nworkflowName: w\n", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, ".chainloop.yml") + require.NoError(t, os.WriteFile(path, []byte(tc.existing), 0600)) + + require.NoError(t, SaveWorkflowToYML(dir, "w")) + + data, err := os.ReadFile(path) + require.NoError(t, err) + assert.Equal(t, tc.want, string(data)) }) } } diff --git a/app/cli/main.go b/app/cli/main.go index 2cff30ea1..231ee96b9 100644 --- a/app/cli/main.go +++ b/app/cli/main.go @@ -16,14 +16,12 @@ package main import ( - "fmt" "os" "github.com/chainloop-dev/chainloop/app/cli/cmd" "github.com/chainloop-dev/chainloop/app/cli/pkg/action" v1 "github.com/chainloop-dev/chainloop/app/controlplane/api/controlplane/v1" "github.com/go-kratos/kratos/v2/errors" - jwtMiddleware "github.com/go-kratos/kratos/v2/middleware/auth/jwt" "github.com/rs/zerolog" _ "github.com/sigstore/sigstore/pkg/signature/kms/aws" _ "github.com/sigstore/sigstore/pkg/signature/kms/azure" @@ -96,6 +94,10 @@ func errorInfo(err error, logger zerolog.Logger) (string, int) { var gateErr *cmd.GateError var subprocessErr *action.SubprocessExitError + // The same helper the commands that only log a credential failure use, so + // the CLI says the same thing wherever one surfaces. + authMsg, authMatched := action.AuthErrorMessage(err) + // Make overrides switch { case errors.As(err, &subprocessErr): @@ -107,25 +109,15 @@ func errorInfo(err error, logger zerolog.Logger) (string, int) { msg = "you need to enable a CAS backend first. Refer to `chainloop cas-backend` command or contact your administrator." case v1.IsCasBackendErrorReasonInvalid(err): msg = "the CAS backend you provided is invalid. Refer to `chainloop cas-backend update` command or contact your administrator." - case isWrappedErr(st, jwtMiddleware.ErrTokenExpired): - msg = "your authentication token has expired, please run \"chainloop auth login\" again" - case isWrappedErr(st, jwtMiddleware.ErrTokenInvalid): - msg = "your authentication token is invalid, please run \"chainloop auth login\" again" - case isWrappedErr(st, jwtMiddleware.ErrTokenParseFail): - msg = "failed to parse authentication token, please run \"chainloop auth login\" again" - case isWrappedErr(st, jwtMiddleware.ErrMissingJwtToken): - msg = "authentication required, please run \"chainloop auth login\"" case v1.IsUserNotMemberOfOrgErrorNotInOrg(err): msg = "the organization you are trying to access does not exist or you are not part of it, please run \"chainloop auth login\"" case v1.IsUserWithNoMembershipErrorNotInOrg(err): msg = "you are not part of any organization, please run \"chainloop organization create --name ORG_NAME\" to create one" - case isUnmatchedAuthErr(st): - // Fallback for any other 401/Unauthenticated errors not matched above. - // Org-membership errors (IsUserNotMemberOfOrgErrorNotInOrg, - // IsUserWithNoMembershipErrorNotInOrg) use gRPC code 7 (PermissionDenied), - // not code 16 (Unauthenticated), so shadowing is structurally impossible. - // We keep this case ordered after them purely for readability/clarity. - msg = fmt.Sprintf("authentication error: %s", st.Message()) + case authMatched: + // Ordered after the org-membership cases, which use gRPC code 7 + // (PermissionDenied) rather than 16 (Unauthenticated), so shadowing + // them is structurally impossible either way. + msg = authMsg case errors.As(err, &cmd.GracefulError{}): // Graceful recovery if the flag is set and the received error is marked as recoverable if cmd.GracefulExit { @@ -139,24 +131,3 @@ func errorInfo(err error, logger zerolog.Logger) (string, int) { return msg, exitCode } - -// target is the expected error -// grpcStatus is the actual error that might be wrapped in both the status and the error -// -// NOTE: We intentionally do NOT use kratos errors.Is() here because it only -// compares Code and Reason. Since all JWT errors share the same Code (401) and -// Reason ("UNAUTHORIZED"), errors.Is() would match any 401 error against any -// JWT sentinel — causing e.g. "token invalid" to be reported as "token expired". -// Instead we compare Code and Message, which carry the distinguishing text. -func isWrappedErr(grpcStatus *status.Status, target *errors.Error) bool { - err := errors.FromError(grpcStatus.Err()) - return target.Code == err.Code && err.Message == target.Message -} - -// isUnmatchedAuthErr returns true when the gRPC status represents an -// Unauthenticated (401-equivalent) error that was not matched by any of the -// specific JWT sentinel checks above. This lets us surface the server's -// original message instead of silently dropping it. -func isUnmatchedAuthErr(grpcStatus *status.Status) bool { - return grpcStatus.Code() == codes.Unauthenticated -} diff --git a/app/cli/main_test.go b/app/cli/main_test.go deleted file mode 100644 index f63bb209b..000000000 --- a/app/cli/main_test.go +++ /dev/null @@ -1,246 +0,0 @@ -// -// Copyright 2023-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 main - -import ( - "testing" - - kratosErrors "github.com/go-kratos/kratos/v2/errors" - jwtMiddleware "github.com/go-kratos/kratos/v2/middleware/auth/jwt" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - "google.golang.org/grpc/codes" - "google.golang.org/grpc/status" -) - -// toGRPCStatus converts a Kratos *errors.Error into a *status.Status the same -// way it would travel over the wire: Kratos Error -> GRPCStatus() -> gRPC Status. -func toGRPCStatus(err *kratosErrors.Error) *status.Status { - return err.GRPCStatus() -} - -func TestIsWrappedErr(t *testing.T) { - tests := []struct { - name string - actual *kratosErrors.Error // the error that "came over the wire" - target *kratosErrors.Error // the sentinel we are checking against - want bool - }{ - { - name: "ErrTokenExpired matches itself", - actual: jwtMiddleware.ErrTokenExpired, - target: jwtMiddleware.ErrTokenExpired, - want: true, - }, - { - name: "ErrTokenInvalid matches itself", - actual: jwtMiddleware.ErrTokenInvalid, - target: jwtMiddleware.ErrTokenInvalid, - want: true, - }, - { - name: "ErrTokenParseFail matches itself", - actual: jwtMiddleware.ErrTokenParseFail, - target: jwtMiddleware.ErrTokenParseFail, - want: true, - }, - { - name: "ErrMissingJwtToken matches itself", - actual: jwtMiddleware.ErrMissingJwtToken, - target: jwtMiddleware.ErrMissingJwtToken, - want: true, - }, - { - name: "ErrTokenExpired does NOT match ErrTokenInvalid", - actual: jwtMiddleware.ErrTokenExpired, - target: jwtMiddleware.ErrTokenInvalid, - want: false, - }, - { - name: "ErrTokenInvalid does NOT match ErrTokenExpired", - actual: jwtMiddleware.ErrTokenInvalid, - target: jwtMiddleware.ErrTokenExpired, - want: false, - }, - { - name: "ErrTokenParseFail does NOT match ErrTokenExpired", - actual: jwtMiddleware.ErrTokenParseFail, - target: jwtMiddleware.ErrTokenExpired, - want: false, - }, - { - name: "ErrTokenExpired does NOT match ErrMissingJwtToken", - actual: jwtMiddleware.ErrTokenExpired, - target: jwtMiddleware.ErrMissingJwtToken, - want: false, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - st := toGRPCStatus(tt.actual) - got := isWrappedErr(st, tt.target) - assert.Equal(t, tt.want, got, "isWrappedErr(%v, %v)", tt.actual.Message, tt.target.Message) - }) - } -} - -func TestIsUnmatchedAuthErr(t *testing.T) { - tests := []struct { - name string - st *status.Status - want bool - }{ - { - name: "generic 401/Unauthenticated error is caught", - st: status.New(codes.Unauthenticated, "some auth error"), - want: true, - }, - { - name: "JWT token expired (Unauthenticated) is caught", - st: toGRPCStatus(jwtMiddleware.ErrTokenExpired), - want: true, - }, - { - name: "PermissionDenied is NOT caught", - st: status.New(codes.PermissionDenied, "forbidden"), - want: false, - }, - { - name: "OK status is NOT caught", - st: status.New(codes.OK, ""), - want: false, - }, - { - name: "Internal error is NOT caught", - st: status.New(codes.Internal, "internal server error"), - want: false, - }, - { - name: "NotFound is NOT caught", - st: status.New(codes.NotFound, "not found"), - want: false, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got := isUnmatchedAuthErr(tt.st) - assert.Equal(t, tt.want, got, "isUnmatchedAuthErr()") - }) - } -} - -// TestKratosErrorsIsMasksJWTErrors demonstrates the bug that motivates our -// Message-based comparison: Kratos errors.Is() only compares Code and Reason. -// Since all JWT errors share Code=401 and Reason="UNAUTHORIZED", errors.Is() -// incorrectly matches ANY JWT error against ANY other JWT sentinel. -func TestKratosErrorsIsMasksJWTErrors(t *testing.T) { - // This test proves that the naive errors.Is approach is broken: - // ErrTokenExpired would incorrectly match ErrTokenInvalid via Kratos errors.Is. - if !kratosErrors.Is(jwtMiddleware.ErrTokenExpired, jwtMiddleware.ErrTokenInvalid) { - t.Skip("Kratos errors.Is behavior has changed; this test documents the original masking bug") - } - - // Now verify that our isWrappedErr correctly distinguishes them - st := toGRPCStatus(jwtMiddleware.ErrTokenExpired) - assert.False(t, isWrappedErr(st, jwtMiddleware.ErrTokenInvalid), - "isWrappedErr should NOT match ErrTokenExpired against ErrTokenInvalid") - assert.True(t, isWrappedErr(st, jwtMiddleware.ErrTokenExpired), - "isWrappedErr should match ErrTokenExpired against itself") -} - -// TestIsWrappedErrGRPCWireRoundTrip verifies that isWrappedErr works after a -// full gRPC wire round-trip: KratosError -> GRPCStatus -> proto bytes -> gRPC -// status.FromError -> isWrappedErr. This simulates the actual path an error -// takes from the server through a gRPC transport to the CLI client. -func TestIsWrappedErrGRPCWireRoundTrip(t *testing.T) { - sentinels := []*kratosErrors.Error{ - jwtMiddleware.ErrTokenExpired, - jwtMiddleware.ErrTokenInvalid, - jwtMiddleware.ErrTokenParseFail, - jwtMiddleware.ErrMissingJwtToken, - } - - for _, sentinel := range sentinels { - t.Run(sentinel.Message, func(t *testing.T) { - // Step 1: Convert Kratos error to gRPC status (server side) - grpcSt := sentinel.GRPCStatus() - - // Step 2: Serialize to the wire format (proto bytes) - proto := grpcSt.Proto() - require.NotNil(t, proto, "gRPC status proto should not be nil") - - // Step 3: Deserialize from proto back into a gRPC status (client side) - roundTripped := status.FromProto(proto) - require.NotNil(t, roundTripped, "round-tripped status should not be nil") - - // Step 4: Verify isWrappedErr still matches after the full round-trip - assert.True(t, isWrappedErr(roundTripped, sentinel), - "isWrappedErr should match %q after gRPC wire round-trip", sentinel.Message) - - // Step 5: Verify it does NOT match a different sentinel after round-trip - for _, other := range sentinels { - if other == sentinel { - continue - } - assert.False(t, isWrappedErr(roundTripped, other), - "isWrappedErr should NOT match %q against %q after round-trip", - sentinel.Message, other.Message) - } - }) - } -} - -// TestJWTSentinelMessageCanary asserts the exact Message strings of the JWT -// sentinel errors we depend on. If a Kratos update changes these strings -// (e.g. ErrTokenParseFail's trailing space), this test will fail and alert us -// that our Message-based matching in isWrappedErr needs updating. -func TestJWTSentinelMessageCanary(t *testing.T) { - tests := []struct { - name string - err *kratosErrors.Error - wantMsg string - }{ - { - name: "ErrTokenExpired", - err: jwtMiddleware.ErrTokenExpired, - wantMsg: "JWT token has expired", - }, - { - name: "ErrTokenInvalid", - err: jwtMiddleware.ErrTokenInvalid, - wantMsg: "Token is invalid", - }, - { - name: "ErrTokenParseFail (note trailing space)", - err: jwtMiddleware.ErrTokenParseFail, - wantMsg: "Fail to parse JWT token ", // trailing space is intentional - }, - { - name: "ErrMissingJwtToken", - err: jwtMiddleware.ErrMissingJwtToken, - wantMsg: "JWT token is missing", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - require.Equal(t, tt.wantMsg, tt.err.Message, - "Kratos sentinel %s Message has changed — update isWrappedErr matching and case branches in errorInfo", tt.name) - }) - } -} diff --git a/app/cli/pkg/action/auth_error.go b/app/cli/pkg/action/auth_error.go new file mode 100644 index 000000000..6ea2a4d29 --- /dev/null +++ b/app/cli/pkg/action/auth_error.go @@ -0,0 +1,67 @@ +// +// 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 action + +import ( + "errors" + "fmt" + + jwtMiddleware "github.com/go-kratos/kratos/v2/middleware/auth/jwt" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" +) + +// authMessages maps the control plane's authentication failures, keyed by the +// message Kratos sends for each, to what the user should do about them. +// +// NOTE: kratos errors.Is() is deliberately not used here because it compares +// only Code and Reason, which are 401 and "UNAUTHORIZED" for all of these, so +// every 401 would match the first entry. The message is the only part that +// tells them apart, which is why it is the key. +var authMessages = map[string]string{ + jwtMiddleware.ErrTokenExpired.Message: `your authentication token has expired, please run "chainloop auth login" again`, + jwtMiddleware.ErrTokenInvalid.Message: `your authentication token is invalid, please run "chainloop auth login" again`, + jwtMiddleware.ErrTokenParseFail.Message: `failed to parse authentication token, please run "chainloop auth login" again`, + jwtMiddleware.ErrMissingJwtToken.Message: `authentication required, please run "chainloop auth login"`, +} + +// AuthErrorMessage renders the message the CLI shows for an authentication +// failure, and reports whether err was one. Every place that surfaces a +// credential problem goes through it — the top-level error handler and the +// commands that only log the failure, like `chainloop trace run` — so the CLI +// says the same thing wherever it comes up. +// +// It digs the gRPC status out of a wrapped error, so callers are free to add +// context: grpc's status.FromError replaces a wrapped error's message with the +// whole error chain, which nothing in the table above would match. +func AuthErrorMessage(err error) (string, bool) { + var grpcErr interface{ GRPCStatus() *status.Status } + if !errors.As(err, &grpcErr) { + return "", false + } + + st := grpcErr.GRPCStatus() + if st.Code() != codes.Unauthenticated { + return "", false + } + + if msg, known := authMessages[st.Message()]; known { + return msg, true + } + + // Any other 401: the control plane's own message says more than a guess. + return fmt.Sprintf("authentication error: %s", st.Message()), true +} diff --git a/app/cli/pkg/action/auth_error_test.go b/app/cli/pkg/action/auth_error_test.go new file mode 100644 index 000000000..d81073a23 --- /dev/null +++ b/app/cli/pkg/action/auth_error_test.go @@ -0,0 +1,169 @@ +// +// Copyright 2023-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 action + +import ( + "errors" + "fmt" + "testing" + + kerrors "github.com/go-kratos/kratos/v2/errors" + jwtMiddleware "github.com/go-kratos/kratos/v2/middleware/auth/jwt" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" +) + +// The messages AuthErrorMessage is expected to render for each Kratos JWT +// sentinel. Spelled out here rather than read from authMessages, so a change to +// what the CLI tells the user has to be made in both places deliberately. +const ( + authMsgExpired = `your authentication token has expired, please run "chainloop auth login" again` + authMsgInvalid = `your authentication token is invalid, please run "chainloop auth login" again` + authMsgParseFail = `failed to parse authentication token, please run "chainloop auth login" again` + authMsgMissing = `authentication required, please run "chainloop auth login"` +) + +func TestAuthErrorMessage(t *testing.T) { + testCases := []struct { + name string + err error + + wantMatch bool + wantMsg string + }{ + { + name: "a missing token asks the user to log in", + err: jwtMiddleware.ErrMissingJwtToken.GRPCStatus().Err(), + wantMatch: true, + wantMsg: authMsgMissing, + }, + { + name: "an expired token says so", + err: jwtMiddleware.ErrTokenExpired.GRPCStatus().Err(), + wantMatch: true, + wantMsg: authMsgExpired, + }, + { + name: "an invalid token says so", + err: jwtMiddleware.ErrTokenInvalid.GRPCStatus().Err(), + wantMatch: true, + wantMsg: authMsgInvalid, + }, + { + name: "an unparseable token says so", + err: jwtMiddleware.ErrTokenParseFail.GRPCStatus().Err(), + wantMatch: true, + wantMsg: authMsgParseFail, + }, + { + // The whole point of matching on the message: every JWT sentinel + // shares Code 401 and Reason "UNAUTHORIZED", so a Reason-only + // comparison (kratos errors.Is) would report all of them as the + // first one. See TestKratosErrorsIsMasksJWTErrors. + name: "any other 401 keeps the control plane's own message", + err: status.Error(codes.Unauthenticated, "token belongs to a deleted user"), + wantMatch: true, + wantMsg: "authentication error: token belongs to a deleted user", + }, + { + // Commands add context on the way up, and grpc's status.FromError + // would replace the status message with the whole chain, so the + // status is dug out with errors.As instead. + name: "a wrapped authentication error is still matched", + err: fmt.Errorf("looking up workflow %q: %w", "ai-coding-session", jwtMiddleware.ErrMissingJwtToken.GRPCStatus().Err()), + wantMatch: true, + wantMsg: `authentication required, please run "chainloop auth login"`, + }, + { + name: "a permission error is not an authentication error", + err: status.Error(codes.PermissionDenied, "forbidden"), + }, + { + name: "a not-found error is not an authentication error", + err: status.Error(codes.NotFound, "not found"), + }, + { + name: "a plain error is not an authentication error", + err: errors.New("boom"), + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + msg, matched := AuthErrorMessage(tc.err) + assert.Equal(t, tc.wantMatch, matched) + assert.Equal(t, tc.wantMsg, msg) + }) + } +} + +// TestAuthErrorMessageGRPCWireRoundTrip verifies the mapping survives a full +// gRPC wire round-trip: kratos error -> GRPCStatus -> proto bytes -> status -> +// AuthErrorMessage. This is the path an error actually takes from the server +// through the transport to the CLI. +func TestAuthErrorMessageGRPCWireRoundTrip(t *testing.T) { + for sentinel, want := range map[*kerrors.Error]string{ + jwtMiddleware.ErrTokenExpired: `your authentication token has expired, please run "chainloop auth login" again`, + jwtMiddleware.ErrTokenInvalid: `your authentication token is invalid, please run "chainloop auth login" again`, + jwtMiddleware.ErrTokenParseFail: `failed to parse authentication token, please run "chainloop auth login" again`, + jwtMiddleware.ErrMissingJwtToken: `authentication required, please run "chainloop auth login"`, + } { + t.Run(sentinel.Message, func(t *testing.T) { + proto := sentinel.GRPCStatus().Proto() + require.NotNil(t, proto) + + msg, matched := AuthErrorMessage(status.FromProto(proto).Err()) + require.True(t, matched) + assert.Equal(t, want, msg, "each sentinel must keep its own message after a wire round-trip") + }) + } +} + +// TestKratosErrorsIsMasksJWTErrors documents the bug that motivates comparing +// the message: kratos errors.Is() only compares Code and Reason, which every +// JWT error shares, so it matches any JWT error against any other sentinel. +func TestKratosErrorsIsMasksJWTErrors(t *testing.T) { + if !kerrors.Is(jwtMiddleware.ErrTokenExpired, jwtMiddleware.ErrTokenInvalid) { + t.Skip("Kratos errors.Is behavior has changed; this test documents the original masking bug") + } + + msg, matched := AuthErrorMessage(jwtMiddleware.ErrTokenExpired.GRPCStatus().Err()) + require.True(t, matched) + assert.Equal(t, `your authentication token has expired, please run "chainloop auth login" again`, msg, + "an expired token must not be reported as an invalid one") +} + +// TestJWTSentinelMessageCanary asserts the exact Message strings of the JWT +// sentinels the mapping depends on. If a Kratos update changes them, this fails +// and points at authMessages. +func TestJWTSentinelMessageCanary(t *testing.T) { + for name, tc := range map[string]struct { + err *kerrors.Error + wantMsg string + }{ + "ErrTokenExpired": {jwtMiddleware.ErrTokenExpired, "JWT token has expired"}, + "ErrTokenInvalid": {jwtMiddleware.ErrTokenInvalid, "Token is invalid"}, + "ErrTokenParseFail (trailing space)": {jwtMiddleware.ErrTokenParseFail, "Fail to parse JWT token "}, + "ErrMissingJwtToken": {jwtMiddleware.ErrMissingJwtToken, "JWT token is missing"}, + } { + t.Run(name, func(t *testing.T) { + require.Equal(t, tc.wantMsg, tc.err.Message, + "Kratos sentinel %s Message has changed — update authMessages", name) + }) + } +} diff --git a/app/cli/pkg/action/trace_attestation.go b/app/cli/pkg/action/trace_attestation.go index 1a87dd102..945c94f17 100644 --- a/app/cli/pkg/action/trace_attestation.go +++ b/app/cli/pkg/action/trace_attestation.go @@ -197,7 +197,11 @@ func (e *AttestationExecutor) CheckAuth(_ context.Context) error { AuthTokenRaw: e.actionOpts.AuthTokenRaw, }).Run() if err != nil { - return fmt.Errorf("chainloop is not authenticated; run 'chainloop config save' first: %w", err) + // The wrap keeps the gRPC status reachable with errors.As, so an + // authentication failure still renders through AuthErrorMessage; + // anything else is the control plane being unreachable, and its own + // error says more than a guess would. + return fmt.Errorf("reaching the chainloop control plane: %w", err) } return nil diff --git a/app/cli/pkg/action/trace_run.go b/app/cli/pkg/action/trace_run.go index a4f334744..ff1019259 100644 --- a/app/cli/pkg/action/trace_run.go +++ b/app/cli/pkg/action/trace_run.go @@ -104,14 +104,13 @@ func TraceRun(ctx context.Context, log zerolog.Logger, opts TraceRunOpts) error if err != nil { return err } - if err := executor.CheckAuth(ctx); err != nil { - log.Warn().Err(err).Msg("authentication check failed; attestation will fail after the session") - } else { - ensureTraceRunWorkflow(ctx, log, executor, opts) - } + prepErr := prepareTraceRunWorkflow(ctx, log, executor, opts) if err := executor.Close(); err != nil { log.Debug().Err(err).Msg("closing auth-check executor") } + if prepErr != nil { + return prepErr + } // Snapshot the agent settings files before we touch anything else // so we can restore them bit-for-bit on exit, even if the user had @@ -201,13 +200,29 @@ func TraceRun(ctx context.Context, log zerolog.Logger, opts TraceRunOpts) error }) } -// ensureTraceRunWorkflow creates the workflow the session will be attested to, -// so it gets the trace contract attached instead of the empty one the control -// plane creates implicitly at push time. Unlike `trace init` this is -// best-effort: a session the user is about to run is worth more than the -// contract binding, and the attestation still creates the workflow implicitly -// if this could not. -func ensureTraceRunWorkflow(ctx context.Context, log zerolog.Logger, executor *AttestationExecutor, opts TraceRunOpts) { +// prepareTraceRunWorkflow checks the credentials and creates the workflow the +// session will be attested to, so it gets the trace contract attached instead +// of the empty one the control plane creates implicitly at push time. +// +// It is best-effort: the session a user is about to run is worth more than the +// contract binding, and the attestation creates the workflow anyway. The one +// exception is a contract named with --contract: the attestation path cannot +// carry a contract name, so continuing there would ignore the flag and bind the +// default contract instead. +func prepareTraceRunWorkflow(ctx context.Context, log zerolog.Logger, executor *AttestationExecutor, opts TraceRunOpts) error { + if err := executor.CheckAuth(ctx); err != nil { + if opts.ContractRequired { + return err + } + if authMsg, isAuth := AuthErrorMessage(err); isAuth { + log.Warn().Msg(authMsg + "; the session is recorded but its attestation will fail") + } else { + log.Warn().Err(err).Msg("authentication check failed; attestation will fail after the session") + } + + return nil + } + wf, err := executor.EnsureWorkflow(ctx, EnsureTraceWorkflowOpts{ ProjectName: opts.ProjectName, WorkflowName: opts.WorkflowName, @@ -215,13 +230,19 @@ func ensureTraceRunWorkflow(ctx context.Context, log zerolog.Logger, executor *A ContractRequired: opts.ContractRequired, }) if err != nil { + if opts.ContractRequired { + return err + } log.Warn().Err(err).Msg("could not create the workflow up front; it will be created when the session is attested") - return + + return nil } if wf.Created { log.Info().Str("workflow", opts.WorkflowName).Str("contract", wf.ContractName).Msg("workflow created") } + + return nil } // traceRunOwnsState reports whether the store's trace state is trace run's diff --git a/app/cli/pkg/action/trace_workflow.go b/app/cli/pkg/action/trace_workflow.go index 71fd65f6c..d24a169ab 100644 --- a/app/cli/pkg/action/trace_workflow.go +++ b/app/cli/pkg/action/trace_workflow.go @@ -87,7 +87,7 @@ func ensureTraceWorkflow(ctx context.Context, api traceWorkflowAPI, log zerolog. return &TraceWorkflowResult{ContractName: wf.ContractName}, nil } if status.Code(err) != codes.NotFound { - return nil, authAwareErr(fmt.Errorf("looking up workflow %q in project %q: %w", opts.WorkflowName, opts.ProjectName, err)) + return nil, fmt.Errorf("looking up workflow %q in project %q: %w", opts.WorkflowName, opts.ProjectName, err) } contract, err := resolveTraceContract(ctx, api, log, opts) @@ -104,7 +104,7 @@ func ensureTraceWorkflow(ctx context.Context, api traceWorkflowAPI, log zerolog. return &TraceWorkflowResult{}, nil } - return nil, authAwareErr(fmt.Errorf("creating workflow %q in project %q: %w", opts.WorkflowName, opts.ProjectName, err)) + return nil, fmt.Errorf("creating workflow %q in project %q: %w", opts.WorkflowName, opts.ProjectName, err) } return &TraceWorkflowResult{Created: true, ContractName: created.ContractName}, nil @@ -123,7 +123,7 @@ func resolveTraceContract(ctx context.Context, api traceWorkflowAPI, log zerolog exists, err := api.contractExists(ctx, opts.ContractName) switch { case err != nil && opts.ContractRequired: - return "", authAwareErr(fmt.Errorf("looking up contract %q: %w", opts.ContractName, err)) + return "", fmt.Errorf("looking up contract %q: %w", opts.ContractName, err) case err != nil: log.Debug().Err(err).Str("contract", opts.ContractName).Msg("could not look up the contract; falling back to the default one") return "", nil @@ -137,16 +137,6 @@ func resolveTraceContract(ctx context.Context, api traceWorkflowAPI, log zerolog return opts.ContractName, nil } -// authAwareErr prefixes an unauthenticated failure with the way out, since -// running trace before logging in is the most likely reason to get one. -func authAwareErr(err error) error { - if status.Code(err) != codes.Unauthenticated { - return err - } - - return fmt.Errorf("chainloop is not authenticated; run 'chainloop auth login' first: %w", err) -} - // cpTraceWorkflowAPI implements traceWorkflowAPI against the control plane, // reusing the actions the equivalent commands run. type cpTraceWorkflowAPI struct { diff --git a/app/cli/pkg/action/trace_workflow_test.go b/app/cli/pkg/action/trace_workflow_test.go index 8fa2caacb..1fd94ae4c 100644 --- a/app/cli/pkg/action/trace_workflow_test.go +++ b/app/cli/pkg/action/trace_workflow_test.go @@ -20,6 +20,7 @@ import ( "errors" "testing" + jwtMiddleware "github.com/go-kratos/kratos/v2/middleware/auth/jwt" "github.com/rs/zerolog" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -168,10 +169,10 @@ func TestEnsureTraceWorkflow(t *testing.T) { wantErr: forbidden, }, { - name: "an unauthenticated view fails with a hint", - api: &fakeTraceWorkflowAPI{viewErr: status.Error(codes.Unauthenticated, "no token")}, + name: "an unauthenticated view fails", + api: &fakeTraceWorkflowAPI{viewErr: status.Error(codes.Unauthenticated, "JWT token is missing")}, opts: EnsureTraceWorkflowOpts{ProjectName: project, WorkflowName: workflow}, - wantErr: "not authenticated", + wantErr: "JWT token is missing", }, { name: "a create failure fails", @@ -226,6 +227,29 @@ func TestEnsureTraceWorkflow(t *testing.T) { assert.Equal(t, codes.PermissionDenied, status.Code(err)) }) + // Whichever call hits it, an authentication failure has to stay + // recognizable through the context added on the way up, so the CLI renders + // its own "run chainloop auth login" message instead of this plumbing. + t.Run("authentication errors stay recognizable", func(t *testing.T) { + authErr := jwtMiddleware.ErrMissingJwtToken.GRPCStatus().Err() + + for name, api := range map[string]*fakeTraceWorkflowAPI{ + "on view": {viewErr: authErr}, + "on contract": {viewErr: notFound, contractErr: authErr}, + "on create": {viewErr: notFound, contractFound: true, createErr: authErr}, + } { + t.Run(name, func(t *testing.T) { + _, err := ensureTraceWorkflow(context.Background(), api, zerolog.Nop(), + EnsureTraceWorkflowOpts{ProjectName: project, WorkflowName: workflow, ContractName: contract, ContractRequired: true}) + require.Error(t, err) + + msg, matched := AuthErrorMessage(err) + require.True(t, matched) + assert.Equal(t, `authentication required, please run "chainloop auth login"`, msg) + }) + } + }) + t.Run("a non-status contract error still falls back", func(t *testing.T) { api := &fakeTraceWorkflowAPI{viewErr: status.Error(codes.NotFound, "not found"), contractErr: errors.New("boom")} got, err := ensureTraceWorkflow(context.Background(), api, zerolog.Nop(), EnsureTraceWorkflowOpts{ProjectName: project, WorkflowName: workflow, ContractName: contract}) From 4c6b2d8940498faa5955ce3bfb135d0c48b7e216 Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Tue, 8 Sep 2026 18:04:14 +0200 Subject: [PATCH 3/3] refactor(cli): name the expected authentication messages in tests The four user-visible strings were written out nine times across three tests. Assisted-by: Claude Code Signed-off-by: Miguel Martinez Trivino Chainloop-Trace-Sessions: 90463ef4-5d43-4c6b-8369-d85c131c0ba8 --- app/cli/pkg/action/auth_error_test.go | 13 ++++++------- app/cli/pkg/action/trace_workflow_test.go | 2 +- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/app/cli/pkg/action/auth_error_test.go b/app/cli/pkg/action/auth_error_test.go index d81073a23..df7744bee 100644 --- a/app/cli/pkg/action/auth_error_test.go +++ b/app/cli/pkg/action/auth_error_test.go @@ -87,7 +87,7 @@ func TestAuthErrorMessage(t *testing.T) { name: "a wrapped authentication error is still matched", err: fmt.Errorf("looking up workflow %q: %w", "ai-coding-session", jwtMiddleware.ErrMissingJwtToken.GRPCStatus().Err()), wantMatch: true, - wantMsg: `authentication required, please run "chainloop auth login"`, + wantMsg: authMsgMissing, }, { name: "a permission error is not an authentication error", @@ -118,10 +118,10 @@ func TestAuthErrorMessage(t *testing.T) { // through the transport to the CLI. func TestAuthErrorMessageGRPCWireRoundTrip(t *testing.T) { for sentinel, want := range map[*kerrors.Error]string{ - jwtMiddleware.ErrTokenExpired: `your authentication token has expired, please run "chainloop auth login" again`, - jwtMiddleware.ErrTokenInvalid: `your authentication token is invalid, please run "chainloop auth login" again`, - jwtMiddleware.ErrTokenParseFail: `failed to parse authentication token, please run "chainloop auth login" again`, - jwtMiddleware.ErrMissingJwtToken: `authentication required, please run "chainloop auth login"`, + jwtMiddleware.ErrTokenExpired: authMsgExpired, + jwtMiddleware.ErrTokenInvalid: authMsgInvalid, + jwtMiddleware.ErrTokenParseFail: authMsgParseFail, + jwtMiddleware.ErrMissingJwtToken: authMsgMissing, } { t.Run(sentinel.Message, func(t *testing.T) { proto := sentinel.GRPCStatus().Proto() @@ -144,8 +144,7 @@ func TestKratosErrorsIsMasksJWTErrors(t *testing.T) { msg, matched := AuthErrorMessage(jwtMiddleware.ErrTokenExpired.GRPCStatus().Err()) require.True(t, matched) - assert.Equal(t, `your authentication token has expired, please run "chainloop auth login" again`, msg, - "an expired token must not be reported as an invalid one") + assert.Equal(t, authMsgExpired, msg, "an expired token must not be reported as an invalid one") } // TestJWTSentinelMessageCanary asserts the exact Message strings of the JWT diff --git a/app/cli/pkg/action/trace_workflow_test.go b/app/cli/pkg/action/trace_workflow_test.go index 1fd94ae4c..c4ce41da9 100644 --- a/app/cli/pkg/action/trace_workflow_test.go +++ b/app/cli/pkg/action/trace_workflow_test.go @@ -245,7 +245,7 @@ func TestEnsureTraceWorkflow(t *testing.T) { msg, matched := AuthErrorMessage(err) require.True(t, matched) - assert.Equal(t, `authentication required, please run "chainloop auth login"`, msg) + assert.Equal(t, authMsgMissing, msg) }) } })