-
Notifications
You must be signed in to change notification settings - Fork 0
Ddded option to use CLI in server environments #9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdds environment token-based auth and a non-interactive setup path gated by ENKRYPTIFY_TOKEN, including API support to fetch project token details. Updates login help text, bumps version to 0.1.9 in code and installer, and ignores .DS_Store in git. No other behavioral changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant CLI as CLI (setup)
participant Auth as Auth Env
participant API as Enkryptify API
participant Store as Setup Storage
User->>CLI: run setup
CLI->>Auth: check ENKRYPTIFY_TOKEN
alt Token present (non-interactive)
Auth-->>CLI: token value
CLI->>API: GET /auth/project-token
API-->>CLI: Workspace, Project, EnvironmentID
CLI->>Store: Save SetupConfig (silent update if exists)
Store-->>CLI: ok
CLI-->>User: Print success summary
else No token (interactive)
CLI-->>User: Print header/prompts
User->>CLI: Select workspace/project/environment
CLI->>Store: Save SetupConfig
Store-->>CLI: ok
CLI-->>User: Print success summary
end
sequenceDiagram
autonumber
participant Caller as Any Command
participant Auth as Auth Env
participant Keyring as Keyring
Note over Auth,Keyring: Access token resolution
alt ENKRYPTIFY_TOKEN set
Caller->>Auth: GetAccessToken
Auth-->>Caller: Return env token
else Not set
Caller->>Keyring: Lookup token
Keyring-->>Caller: Return stored token (or error)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
.gitignore(1 hunks)cmd/login.go(1 hunks)cmd/root.go(1 hunks)cmd/setup.go(2 hunks)install.sh(1 hunks)internal/auth/enkryptify.go(5 hunks)internal/providers/enkryptify/provider.go(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
internal/auth/enkryptify.go (1)
internal/ui/ui.go (1)
PrintInfo(95-97)
cmd/setup.go (4)
internal/providers/enkryptify/provider.go (4)
NewClient(85-90)Client(16-19)Workspace(21-26)Project(28-36)internal/auth/enkryptify.go (1)
EnvTokenKey(32-32)internal/config/setup.go (2)
SetupStorage(19-21)SetupConfig(12-17)internal/ui/ui.go (2)
ShowBrandHeader(139-151)PrintTitle(70-72)
🔇 Additional comments (5)
install.sh (1)
3-3: LGTM!Version bump is consistent with the changes in cmd/root.go.
cmd/root.go (1)
10-10: LGTM!Version bump aligns with the new features introduced in this PR.
cmd/login.go (1)
32-34: LGTM!The added documentation provides clear guidance for server environments where browser authentication isn't available. This aligns well with the environment-token authentication feature introduced in this PR.
cmd/setup.go (2)
63-99: Verify error handling for incomplete token details.The non-interactive setup flow correctly handles API errors from
GetProjectTokenDetails, but relies on the API returning complete data. Ensure that the validation suggested ininternal/providers/enkryptify/provider.gois implemented to prevent saving incomplete configurations.Additionally, verify that the environment token validation suggested in
internal/auth/enkryptify.gois implemented to prevent this setup flow from running with an invalid token.
51-60: LGTM!The refactoring to separate interactive and non-interactive setup flows is clean and maintains backward compatibility. The routing logic based on the environment token is straightforward and correct.
| fmt.Printf("Updating existing setup for %s\n", currentPath) | ||
| fmt.Printf("Previous: workspace=%s, project=%s, environment=%s\n", | ||
| existingSetup.WorkspaceSlug, existingSetup.ProjectSlug, existingSetup.EnvironmentID) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Use ui package functions for consistent output formatting.
The code uses fmt.Printf directly instead of the ui package functions. For consistency with the rest of the codebase, consider using ui.PrintInfo for informational messages.
Apply this diff:
- fmt.Printf("Updating existing setup for %s\n", currentPath)
- fmt.Printf("Previous: workspace=%s, project=%s, environment=%s\n",
- existingSetup.WorkspaceSlug, existingSetup.ProjectSlug, existingSetup.EnvironmentID)
+ ui.PrintInfo(fmt.Sprintf("Updating existing setup for %s", currentPath))
+ ui.PrintInfo(fmt.Sprintf("Previous: workspace=%s, project=%s, environment=%s",
+ existingSetup.WorkspaceSlug, existingSetup.ProjectSlug, existingSetup.EnvironmentID))🤖 Prompt for AI Agents
In cmd/setup.go around lines 74 to 76, the code uses fmt.Printf for
informational output; replace those calls with ui.PrintInfo to keep output
consistent with the codebase (e.g., ui.PrintInfo("Updating existing setup for
%s", currentPath) and ui.PrintInfo("Previous: workspace=%s, project=%s,
environment=%s", existingSetup.WorkspaceSlug, existingSetup.ProjectSlug,
existingSetup.EnvironmentID)); ensure the ui package is imported and remove the
unused fmt import if no longer needed.
| fmt.Printf("✓ Setup completed successfully!\n") | ||
| fmt.Printf(" Workspace: %s\n", tokenDetails.Workspace.Slug) | ||
| fmt.Printf(" Project: %s\n", tokenDetails.Project.Slug) | ||
| fmt.Printf(" Environment: %s\n", tokenDetails.EnvironmentID) | ||
| fmt.Printf(" Path: %s\n", currentPath) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Use ui package functions for consistent output formatting.
Similar to the previous comment, use ui.PrintSuccess and ui.PrintInfo instead of fmt.Printf for consistent output formatting throughout the CLI.
Apply this diff:
- fmt.Printf("✓ Setup completed successfully!\n")
- fmt.Printf(" Workspace: %s\n", tokenDetails.Workspace.Slug)
- fmt.Printf(" Project: %s\n", tokenDetails.Project.Slug)
- fmt.Printf(" Environment: %s\n", tokenDetails.EnvironmentID)
- fmt.Printf(" Path: %s\n", currentPath)
+ ui.PrintSuccess("Setup completed successfully!")
+ ui.PrintInfo(fmt.Sprintf("Workspace: %s", tokenDetails.Workspace.Slug))
+ ui.PrintInfo(fmt.Sprintf("Project: %s", tokenDetails.Project.Slug))
+ ui.PrintInfo(fmt.Sprintf("Environment: %s", tokenDetails.EnvironmentID))
+ ui.PrintInfo(fmt.Sprintf("Path: %s", currentPath))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fmt.Printf("✓ Setup completed successfully!\n") | |
| fmt.Printf(" Workspace: %s\n", tokenDetails.Workspace.Slug) | |
| fmt.Printf(" Project: %s\n", tokenDetails.Project.Slug) | |
| fmt.Printf(" Environment: %s\n", tokenDetails.EnvironmentID) | |
| fmt.Printf(" Path: %s\n", currentPath) | |
| ui.PrintSuccess("Setup completed successfully!") | |
| ui.PrintInfo(fmt.Sprintf("Workspace: %s", tokenDetails.Workspace.Slug)) | |
| ui.PrintInfo(fmt.Sprintf("Project: %s", tokenDetails.Project.Slug)) | |
| ui.PrintInfo(fmt.Sprintf("Environment: %s", tokenDetails.EnvironmentID)) | |
| ui.PrintInfo(fmt.Sprintf("Path: %s", currentPath)) |
🤖 Prompt for AI Agents
In cmd/setup.go around lines 92 to 96, replace direct fmt.Printf calls with the
CLI ui package helpers to ensure consistent formatting: use ui.PrintSuccess("✓
Setup completed successfully!") for the success line and ui.PrintInfo for each
detail line (Workspace, Project, Environment, Path), passing formatted strings
or using ui.PrintInfof if available; also remove the fmt import if it becomes
unused.
| // Check if using environment variable token | ||
| if token := os.Getenv(EnvTokenKey); token != "" { | ||
| ui.PrintInfo(fmt.Sprintf("Authenticated using %s environment variable", EnvTokenKey)) | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Validate the environment token before accepting it.
The code accepts any non-empty value from ENKRYPTIFY_TOKEN without validation. Consider validating that the token is not empty and optionally verify its format or make a test API call to ensure it's valid before short-circuiting the OAuth flow.
Apply this diff to add basic validation:
func (e *EnkryptifyAuth) Login(ctx context.Context) error {
// Check if using environment variable token
if token := os.Getenv(EnvTokenKey); token != "" {
+ // Validate token is not just whitespace
+ if strings.TrimSpace(token) == "" {
+ return fmt.Errorf("invalid %s: token cannot be empty or whitespace", EnvTokenKey)
+ }
ui.PrintInfo(fmt.Sprintf("Authenticated using %s environment variable", EnvTokenKey))
return nil
}🏁 Script executed:
#!/bin/bash
# Search for all uses of the environment token without validation
rg -nP --type Go 'os\.Getenv\(\s*EnvTokenKey\s*\)' -C3Length of output: 26
🏁 Script executed:
#!/bin/bash
# Locate all occurrences of os.Getenv(EnvTokenKey) in .go files
rg -n 'os\.Getenv\(\s*EnvTokenKey\s*\)' -C3 --glob '*.go'Length of output: 1510
🏁 Script executed:
#!/bin/bash
# Search for existing use of strings.TrimSpace in the repo
rg -n 'TrimSpace' --glob '*.go'Length of output: 144
🏁 Script executed:
#!/bin/bash
# Show import block in enkryptify.go
rg -n 'import \(' -C5 --glob 'internal/auth/enkryptify.go'Length of output: 341
🏁 Script executed:
sed -n '1,20p' internal/auth/enkryptify.goLength of output: 281
Trim and reject whitespace-only ENKRYPTIFY_TOKEN
- Login, IsAuthenticated, and GetAccessToken accept non-empty but whitespace-only tokens.
- Use
token := strings.TrimSpace(os.Getenv(EnvTokenKey))and return an error iftoken == ""to avoid invalid values.
Apply in Login (mirror in the other methods):
func (e *EnkryptifyAuth) Login(ctx context.Context) error {
- if token := os.Getenv(EnvTokenKey); token != "" {
+ if token := strings.TrimSpace(os.Getenv(EnvTokenKey)); token != "" {
ui.PrintInfo(fmt.Sprintf("Authenticated using %s environment variable", EnvTokenKey))
return nil
- }
+ } else if os.Getenv(EnvTokenKey) != "" {
+ return fmt.Errorf("invalid %s: token cannot be empty or whitespace", EnvTokenKey)
+ }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In internal/auth/enkryptify.go around lines 81 to 85, the code reads the
ENKRYPTIFY_TOKEN env var without trimming, allowing whitespace-only values;
update the env var read to use strings.TrimSpace(os.Getenv(EnvTokenKey)) and if
the result is "" return an error (or treat as unauthenticated) instead of
accepting it; apply the same trimmed-check pattern to Login and mirror the
change in IsAuthenticated and GetAccessToken to reject whitespace-only tokens.
| // Check for environment variable token first (for server environments) | ||
| if token := os.Getenv(EnvTokenKey); token != "" { | ||
| return true, nil | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure consistency with GetAccessToken for empty tokens.
If ENKRYPTIFY_TOKEN is set but empty (e.g., export ENKRYPTIFY_TOKEN=""), this function returns true, but GetAccessToken would return an empty string. This inconsistency could lead to confusing error messages downstream. Consider trimming and checking for non-empty values in both places.
Apply this diff to ensure consistency:
func (e *EnkryptifyAuth) IsAuthenticated() (bool, error) {
// Check for environment variable token first (for server environments)
if token := os.Getenv(EnvTokenKey); token != "" {
- return true, nil
+ return strings.TrimSpace(token) != "", nil
}
return e.keyring.IsAuthenticated("enkryptify")
}🤖 Prompt for AI Agents
In internal/auth/enkryptify.go around lines 421 to 424, the code treats
ENKRYPTIFY_TOKEN as present when the environment variable is set even if empty;
change the env check to trim whitespace and ensure non-empty so it mirrors
GetAccessToken behavior: read os.Getenv(EnvTokenKey), strings.TrimSpace it, and
only return true when the trimmed value is not an empty string (otherwise
continue to other checks or return false).
| // Check for environment variable token first (for server environments) | ||
| if token := os.Getenv(EnvTokenKey); token != "" { | ||
| return token, nil | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add validation for the environment token.
Similar to the concern in Login, this code returns the token without validating it's non-empty or properly formatted. An empty or invalid token would cause downstream API calls to fail with unclear error messages.
Apply this diff to add validation:
func (e *EnkryptifyAuth) GetAccessToken() (string, error) {
// Check for environment variable token first (for server environments)
if token := os.Getenv(EnvTokenKey); token != "" {
- return token, nil
+ token = strings.TrimSpace(token)
+ if token == "" {
+ return "", fmt.Errorf("invalid %s: token cannot be empty or whitespace", EnvTokenKey)
+ }
+ return token, nil
}
authInfo, err := e.keyring.GetAuthInfo("enkryptify")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Check for environment variable token first (for server environments) | |
| if token := os.Getenv(EnvTokenKey); token != "" { | |
| return token, nil | |
| } | |
| func (e *EnkryptifyAuth) GetAccessToken() (string, error) { | |
| // Check for environment variable token first (for server environments) | |
| if token := os.Getenv(EnvTokenKey); token != "" { | |
| token = strings.TrimSpace(token) | |
| if token == "" { | |
| return "", fmt.Errorf("invalid %s: token cannot be empty or whitespace", EnvTokenKey) | |
| } | |
| return token, nil | |
| } | |
| authInfo, err := e.keyring.GetAuthInfo("enkryptify") | |
| // ... | |
| } |
🤖 Prompt for AI Agents
In internal/auth/enkryptify.go around lines 441 to 444, the code returns an
environment token without validating it; change this to retrieve the EnvTokenKey
value, check it's non-empty, run it through the same token validation used in
Login (or call the existing validateToken/validateAuthToken helper) and only
return it if validation passes; if empty or invalid, do not return the token and
instead proceed as before or return a clear error indicating the env token is
malformed so downstream calls fail fast with a helpful message.
| type ProjectTokenResponse struct { | ||
| ID string `json:"id"` | ||
| Workspace struct { | ||
| ID string `json:"id"` | ||
| Slug string `json:"slug"` | ||
| } `json:"workspace"` | ||
| Project struct { | ||
| ID string `json:"id"` | ||
| Slug string `json:"slug"` | ||
| } `json:"project"` | ||
| EnvironmentID string `json:"environmentId"` | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Consider removing the unused ID field.
The ID field on line 73 is not used by the consuming code in cmd/setup.go (lines 80-85), which only references Workspace.Slug, Project.Slug, and EnvironmentID. Unless this field serves a future purpose, consider removing it to keep the API surface minimal.
🤖 Prompt for AI Agents
internal/providers/enkryptify/provider.go around lines 72 to 83: remove the
unused top-level ID field from ProjectTokenResponse (the code only consumes
Workspace.Slug, Project.Slug and EnvironmentID), so delete the ID line and its
json tag, keep Workspace, Project and EnvironmentID intact, then run go fmt / go
vet and the test suite to ensure no references remain.
| func (c *Client) GetProjectTokenDetails() (*ProjectTokenResponse, error) { | ||
| var tokenDetails ProjectTokenResponse | ||
|
|
||
| if err := c.makeRequest("GET", "/auth/project-token", &tokenDetails); err != nil { | ||
| return nil, fmt.Errorf("failed to get project token details: %w", err) | ||
| } | ||
|
|
||
| return &tokenDetails, nil | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Consider validating the API response.
The method correctly uses the existing makeRequest helper and error handling pattern. However, consider adding validation for the returned data to ensure that critical fields like Workspace.Slug, Project.Slug, and EnvironmentID are non-empty, as these are required by the consuming setup logic in cmd/setup.go.
Apply this diff to add validation:
func (c *Client) GetProjectTokenDetails() (*ProjectTokenResponse, error) {
var tokenDetails ProjectTokenResponse
if err := c.makeRequest("GET", "/auth/project-token", &tokenDetails); err != nil {
return nil, fmt.Errorf("failed to get project token details: %w", err)
}
+
+ // Validate required fields
+ if tokenDetails.Workspace.Slug == "" || tokenDetails.Project.Slug == "" || tokenDetails.EnvironmentID == "" {
+ return nil, fmt.Errorf("incomplete project token details: missing required fields")
+ }
return &tokenDetails, nil
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func (c *Client) GetProjectTokenDetails() (*ProjectTokenResponse, error) { | |
| var tokenDetails ProjectTokenResponse | |
| if err := c.makeRequest("GET", "/auth/project-token", &tokenDetails); err != nil { | |
| return nil, fmt.Errorf("failed to get project token details: %w", err) | |
| } | |
| return &tokenDetails, nil | |
| } | |
| func (c *Client) GetProjectTokenDetails() (*ProjectTokenResponse, error) { | |
| var tokenDetails ProjectTokenResponse | |
| if err := c.makeRequest("GET", "/auth/project-token", &tokenDetails); err != nil { | |
| return nil, fmt.Errorf("failed to get project token details: %w", err) | |
| } | |
| // Validate required fields | |
| if tokenDetails.Workspace.Slug == "" || tokenDetails.Project.Slug == "" || tokenDetails.EnvironmentID == "" { | |
| return nil, fmt.Errorf("incomplete project token details: missing required fields") | |
| } | |
| return &tokenDetails, nil | |
| } |
🤖 Prompt for AI Agents
In internal/providers/enkryptify/provider.go around lines 172 to 180, the
GetProjectTokenDetails function returns API data without validating required
fields; add post-request validation to check that tokenDetails.Workspace.Slug,
tokenDetails.Project.Slug, and tokenDetails.EnvironmentID are non-empty and
return a descriptive error if any are missing. Keep the existing makeRequest
call and error wrapping, then perform simple nil/empty-string checks and
fmt.Errorf to surface which field is invalid (e.g., "missing workspace slug"),
ensuring callers receive a clear failure instead of corrupt data.
Summary by CodeRabbit
New Features
Documentation
Chores