Enrich 401/403 errors with auth identity context#4576
Merged
simonfaltum merged 10 commits intomainfrom Feb 24, 2026
Merged
Conversation
When CLI commands fail with 401 (unauthorized) or 403 (forbidden), append profile, host, and auth type to the error message along with auth-type-aware remediation steps. This helps users quickly identify which identity is being used and how to fix the issue. The enrichment is applied in the centralized Execute error path so it covers both client creation and command execution errors. Commands that bypass MustWorkspaceClient/MustAccountClient (e.g., databricks api) are not enriched as they don't set ConfigUsed. Resolves DECO-26395.
- BuildDescribeCommand now takes *config.Config directly instead of OAuthArgument, so it can include --workspace-id for unified hosts. - For login re-auth hints on unified hosts, append --workspace-id after BuildLoginCommand when profile is not set. - Add cmd/root/root_test.go with integration tests for Execute wiring: with ConfigUsed + 403, without ConfigUsed, and ErrAlreadyPrinted.
- Remove redundant status code iterations in NonAuthStatusCode test (400, 404, 500 all hit the same path — one suffices) - Remove "empty config fields are omitted" test case — covered by "401 without profile" and "401 with unknown auth type" tests - Simplify Execute integration test to verify wiring only (check for "Next steps:") rather than re-testing enrichment content - Gate --account-id emission in BuildDescribeCommand on account or unified host types to avoid emitting it for workspace hosts
When cfg.Profile is set, BuildLoginCommand only uses --profile and ignores the OAuthArgument. Skip the ToOAuthArgument() conversion to avoid unnecessarily degrading to a bare "databricks auth login" fallback if the host happens to be malformed. Also fix the EnrichAuthError doc comment to accurately describe its behavior — it always returns a wrapped error for 401/403.
Collaborator
|
Commit: 3ebd312
15 interesting tests: 7 KNOWN, 7 SKIP, 1 RECOVERED
Top 20 slowest tests (at least 2 minutes):
|
- Replace fmt.Errorf with errors.New for static strings (perfsprint) - Replace require.True(errors.As(...)) with require.ErrorAs (testifylint)
- Add AuthTypeDisplayName mapping so error messages show "Personal Access Token (pat)" instead of raw "pat", etc. - Simplify BuildDescribeCommand to always emit bare "databricks auth describe" when no profile is set, since it resolves DATABRICKS_HOST/TOKEN env vars automatically. This avoids nudging users toward --host flags.
- Add constants for all auth types, not just the 5 used in switch - Use fmt.Fprint consistently instead of mixing with b.WriteString - Switch tests to exact output comparison (wantMsg + assert.Equal) instead of substring contains/notContain checks
andrewnester
approved these changes
Feb 24, 2026
Collaborator
|
Commit: 61a1131
32 interesting tests: 16 KNOWN, 8 RECOVERED, 4 flaky, 3 FAIL, 1 SKIP
Top 50 slowest tests (at least 2 minutes):
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
When CLI commands fail with 401 (unauthorized) or 403 (forbidden), the error message shows only the raw API error (e.g.,
PERMISSION_DENIED: ...) with no indication of which profile, host, or auth method was used. This makes debugging auth issues unnecessarily difficult — the first thing users and support ask is "which identity am I using?"Changes
libs/auth/error.go— NewEnrichAuthErrorfunction that detectsapierr.APIErrorwith status 401 or 403 and appends:auth login, PAT → regenerate token, Azure CLI →az login, M2M → check credentials, etc.)auth describecommand with correct flags for workspace/account/unified contexts (including--workspace-id)libs/auth/error.go— NewBuildDescribeCommandthat buildsdatabricks auth describeflags directly from*config.Config(avoids information loss from OAuthArgument conversion, e.g., workspace-id)cmd/root/root.go— CallEnrichAuthErrorin the centralizedExecuteerror path, gated oncmdctx.HasConfigUsed. Covers both client creation errors (PersistentPreRunE) and command execution errors (RunE)acceptance/workspace/jobs/create-error/output.txt— Regenerated golden fileScope limitations
Commands that bypass
MustWorkspaceClient/MustAccountClientand don't callSetConfigUsed(e.g.,databricks api) won't get enrichment. This can be addressed in a follow-up.Example output
better-auth-debugging-errors.mov
403 with profile:
401 without profile (env var auth):
Test plan
Automated
go test ./libs/auth/...— unit tests forEnrichAuthError,BuildDescribeCommand, error preservation, all auth types, unified host workspace-idgo test ./cmd/root/...— integration tests for Execute wiring (with ConfigUsed, without, ErrAlreadyPrinted)go test ./acceptance -run TestAccept/workspace/jobs/create-error— acceptance test with updated golden filemake checks— whitespace, tidy, linksManual testing
Force a 401 (expired/invalid token):
Verify non-auth errors are unaffected: