diff --git a/app/cli/cmd/trace.go b/app/cli/cmd/trace.go index d4041814d..a653d72be 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; 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 { cmd := &cobra.Command{ diff --git a/app/cli/cmd/trace_init.go b/app/cli/cmd/trace_init.go index a012b349c..f4d141ef5 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 @@ -45,94 +47,33 @@ 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 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. -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.`, RunE: func(cmd *cobra.Command, _ []string) error { gitDir, repoRoot, err := tracegit.FindGitDirAndRoot() if err != nil { 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 +115,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 +125,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 +135,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..3659b717a 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,11 @@ 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. 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 { @@ -42,6 +47,7 @@ func newTraceRunCmd() *cobra.Command { projectFlag string workflowFlag string versionFlag string + contractFlag string claudeFlag bool cursorFlag bool opencodeFlag bool @@ -103,17 +109,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 +131,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..bf1041a5f 100755 --- a/app/cli/documentation/cli-reference.mdx +++ b/app/cli/documentation/cli-reference.mdx @@ -3306,16 +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. -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] @@ -3325,6 +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; 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 @@ -3366,6 +3364,10 @@ 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. 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] ``` @@ -3374,6 +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; 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 06c62d97f..b86a38775 100644 --- a/app/cli/internal/trace/config/config.go +++ b/app/cli/internal/trace/config/config.go @@ -16,9 +16,11 @@ package config import ( + "bytes" "fmt" "os" "path/filepath" + "strings" tracegit "github.com/chainloop-dev/chainloop/app/cli/internal/trace/git" "gopkg.in/yaml.v3" @@ -33,6 +35,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 +53,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 +183,27 @@ 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) - if data, err := os.ReadFile(path); err == nil { - if err := yaml.Unmarshal(data, &existing); err != nil { - return fmt.Errorf("parse %s: %w", filepath.Base(path), err) - } + // A missing file is the same as an empty document: setYAMLField starts one. + data, _ := os.ReadFile(path) + + var doc yaml.Node + 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, yamlIndent(data)) if err != nil { return fmt.Errorf("marshal %s: %w", filepath.Base(path), err) } @@ -180,6 +211,93 @@ func updateChainloopYMLField(dir, key string, value any) error { return os.WriteFile(path, out, 0600) } +// 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(indent) + if err := enc.Encode(doc); err != nil { + return nil, err + } + if err := enc.Close(); err != nil { + return nil, err + } + + 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 +// 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..36c5daeda 100644 --- a/app/cli/internal/trace/config/config_test.go +++ b/app/cli/internal/trace/config/config_test.go @@ -155,6 +155,154 @@ 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 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) { + // 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 +# 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 + want string + }{ + { + 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: "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: "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 +`, + }, + } + + 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) + 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)) + }) + } +} + func TestRequireTrace(t *testing.T) { t.Run("defaults to false when field missing", func(t *testing.T) { dir := t.TempDir() @@ -264,6 +412,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/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..df7744bee --- /dev/null +++ b/app/cli/pkg/action/auth_error_test.go @@ -0,0 +1,168 @@ +// +// 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: authMsgMissing, + }, + { + 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: authMsgExpired, + jwtMiddleware.ErrTokenInvalid: authMsgInvalid, + jwtMiddleware.ErrTokenParseFail: authMsgParseFail, + jwtMiddleware.ErrMissingJwtToken: authMsgMissing, + } { + 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, authMsgExpired, 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 2d1bfdd93..ff1019259 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. @@ -99,12 +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") - } + 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 @@ -194,6 +200,51 @@ func TraceRun(ctx context.Context, log zerolog.Logger, opts TraceRunOpts) error }) } +// 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, + ContractName: opts.ContractName, + 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 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 // 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..d24a169ab --- /dev/null +++ b/app/cli/pkg/action/trace_workflow.go @@ -0,0 +1,168 @@ +// +// 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, 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, 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 "", 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 +} + +// 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..c4ce41da9 --- /dev/null +++ b/app/cli/pkg/action/trace_workflow_test.go @@ -0,0 +1,261 @@ +// +// 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" + + jwtMiddleware "github.com/go-kratos/kratos/v2/middleware/auth/jwt" + "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", + api: &fakeTraceWorkflowAPI{viewErr: status.Error(codes.Unauthenticated, "JWT token is missing")}, + opts: EnsureTraceWorkflowOpts{ProjectName: project, WorkflowName: workflow}, + wantErr: "JWT token is missing", + }, + { + 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)) + }) + + // 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, authMsgMissing, 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}) + 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)