feat: add Parseable Cloud API key login#110
Conversation
📝 WalkthroughWalkthroughAdds Parseable Cloud profile creation and browser login, expands profile authentication modes, centralizes request authentication, updates PromQL commands, and adds JSON output support across root, status, logout, and tail commands. ChangesCloud authentication and profile management
Estimated code review effort: 4 (Complex) | ~60 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
cmd/login.go (1)
60-77: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated cloud-profile construction.
cloudProfileFromAPIKeybuilds aconfig.Profilefrom the validation result identically toCloudProfileAddCmd.RunEincmd/cloud.go(lines 96-105). Consider extracting a single helper (e.g.profileFromValidation(apiKey, orchestratorURL, result)) used by both paths so the field mapping stays in sync.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/login.go` around lines 60 - 77, The cloud-profile construction logic is duplicated between cloudProfileFromAPIKey and CloudProfileAddCmd.RunE, so extract the shared mapping into a single helper such as profileFromValidation that accepts the apiKey, orchestratorURL, and validation result and returns the config.Profile. Update both call sites to use that helper so the field assignments stay consistent in one place.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/cloud.go`:
- Around line 184-200: saveCloudProfile currently treats every
ReadConfigFromFile error as an empty config and then writes it back, which can
wipe existing profiles, and it also overwrites an existing profile when the
derived or provided name already exists. Update saveCloudProfile to distinguish
a missing config file from other read errors so non-fatal parse/read failures
are surfaced instead of replacing the config, and add an overwrite check before
assigning fileConfig.Profiles[name]. Use the existing stepConfirmReplace pattern
from the interactive login flow to warn or confirm when a profile name collision
would replace an existing entry, then only call WriteConfigToFile after the user
has explicitly accepted the overwrite.
- Around line 33-37: The default Cloud orchestrator URL is currently pointing at
the staging endpoint, so `defaultCloudOrchestratorURL` in `cmd/cloud.go` should
be changed to the production orchestrator unless staging is explicitly intended.
Update the default used by the Cloud validation flow, and make sure the
`envCloudOrchestratorURL` override still works so users can opt into a different
endpoint if needed.
In `@cmd/tail.go`:
- Around line 120-132: The tailAuthMetadata helper is missing bearer-token
handling and currently falls back to Basic auth for token-only profiles. Update
tailAuthMetadata to mirror the same auth precedence used by AddAuthHeaders:
handle profile.Token before the Basic fallback, returning an Authorization
header with Bearer <token> when a token is present, while keeping the existing
cloud x-api-key and tenant behavior unchanged.
In `@pkg/config/config.go`:
- Line 101: The config write path in the file-opening logic does not harden
permissions for already-existing files, so broader modes can persist after
writing sensitive values like api_key. Update the config save flow around
os.OpenFile in the config write function to explicitly set restrictive
permissions on the opened file after it is created/opened, using
file.Chmod(0600) or equivalent, so both new and existing config files end up
with 0600.
---
Nitpick comments:
In `@cmd/login.go`:
- Around line 60-77: The cloud-profile construction logic is duplicated between
cloudProfileFromAPIKey and CloudProfileAddCmd.RunE, so extract the shared
mapping into a single helper such as profileFromValidation that accepts the
apiKey, orchestratorURL, and validation result and returns the config.Profile.
Update both call sites to use that helper so the field assignments stay
consistent in one place.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 19834b75-b4c7-47e5-8e5b-cdc99993cf39
📒 Files selected for processing (13)
cmd/cloud.gocmd/login.gocmd/promql.gocmd/queryList.gocmd/tail.gomain.gopkg/config/config.gopkg/datasets/datasets.gopkg/http/http.gopkg/model/login/login.gopkg/model/promql.gopkg/model/query.gopkg/model/savedQueries.go
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
pkg/model/login/login.go (1)
579-593: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRender the actual Cloud authentication mode.
OAuth browser profiles are currently labeled as Cloud API-key profiles, while Cloud API-key profiles render two
AUTHrows.Proposed fix
if m.Profile.Cloud { + auth := "Cloud API key (stored after validation)" + if m.CloudBrowserLogin { + auth = "Cloud OAuth (browser)" + } b.WriteString(" " + labelStyle.Render("AUTH ")) - b.WriteString(normalStyle.Render("Cloud API key (stored after validation)")) + b.WriteString(normalStyle.Render(auth)) b.WriteString("\n") } - if m.Profile.APIKey != "" { + if !m.Profile.Cloud && m.Profile.APIKey != "" {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/model/login/login.go` around lines 579 - 593, Update the profile rendering logic around the Cloud and APIKey checks to distinguish OAuth browser profiles from Cloud API-key profiles: render the Cloud authentication label according to the actual authentication mode, and avoid emitting a second AUTH row for Cloud API-key profiles. Preserve the existing username rendering and stored-key messaging for non-Cloud profiles.cmd/queryList.go (1)
53-69: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPropagate saved-query request failures.
When
NewRequestrejects an invalid profile,fetchFiltersprints the error to stdout and returns nil. JSON mode then emits[]and exits successfully, producing invalid mixed output and masking authentication failures.Change
fetchFiltersto return([]Item, error)and letRunEreturn the error without printing inside the data layer.Also applies to: 181-191
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/queryList.go` around lines 53 - 69, The fetchFilters data path currently suppresses request errors, causing JSON mode to output an empty array and succeed. Update fetchFilters to return ([]Item, error), propagate NewRequest failures without printing, and update RunE and all callers to handle the returned error by returning it before formatting output; preserve successful JSON and non-JSON behavior.
🧹 Nitpick comments (3)
cmd/dataset_test.go (1)
22-22: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the authentication header in at least one handler.
Supplying
APIKeyonly satisfiesAuthMode; these tests still pass ifx-api-keyis never sent.Proposed assertion
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if got := r.Header.Get("x-api-key"); got != "test-api-key" { + t.Fatalf("x-api-key mismatch: got %q", got) + }Also applies to: 66-66
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/dataset_test.go` at line 22, Update the handlers tested in cmd/dataset_test.go to assert that requests include the expected x-api-key authentication header, using the configured "test-api-key" value. Add this assertion in at least one handler, including the additionally referenced handler, while preserving the existing test behavior.cmd/promql.go (2)
384-393: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRepeated flag/positional-arg override boilerplate across ~7 subcommands.
The same pattern (
val, _ := cmd.Flags().GetString(...); if len(args) > idx { val = args[idx] }) is duplicated inpromqlLabelsCmd,promqlLabelValuesCmd,promqlSeriesCmd,promqlCardinalityLabelNamesCmd,promqlCardinalityLabelValuesCmd,promqlCardinalityActiveSeriesCmd, andpromqlTSDBCmd. Consider extracting a small helper to centralize this.♻️ Proposed helper and example usage
// argOrFlag returns args[idx] if present, otherwise the current value of the given string flag. func argOrFlag(cmd *cobra.Command, args []string, idx int, flagName string) string { val, _ := cmd.Flags().GetString(flagName) if len(args) > idx { return args[idx] } return val }- stream, _ := cmd.Flags().GetString("dataset") - if len(args) > 0 { - stream = args[0] - } + stream := argOrFlag(cmd, args, 0, "dataset")Apply the same substitution at the other six call sites.
Also applies to: 438-448, 494-503, 569-578, 620-633, 717-732, 843-852
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/promql.go` around lines 384 - 393, The positional-argument-overrides-flag logic is duplicated across the PromQL subcommands. Add an argOrFlag helper near the command implementations, then update promqlLabelsCmd and the corresponding promqlLabelValuesCmd, promqlSeriesCmd, promqlCardinalityLabelNamesCmd, promqlCardinalityLabelValuesCmd, promqlCardinalityActiveSeriesCmd, and promqlTSDBCmd call sites to use it with the appropriate argument index and flag name.
620-633: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winPositional order for
label-valuesis reversed between sibling commands.
promql label-valuestakes[label_name] [stream](Line 438), whilepromql cardinality label-valuestakes[stream] [label](Line 620) — same subcommand name, swapped order, no type validation on either positional. A user transposing the args between the two contexts would silently query the wrong stream/label instead of getting an error.Worth a deliberate decision (and doc note) on whether to align ordering with the top-level command or keep it consistent with the other
cardinalitysubcommands (which are all stream-first).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/promql.go` around lines 620 - 633, Deliberately resolve the positional argument order mismatch between the top-level label-values command and the cardinality label-values command. Update the cardinality label-values definition, including its Use, Example, and RunE argument mapping around stream and labelName, to follow the chosen ordering consistently; document the supported order and preserve compatibility expectations where applicable.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/cloud.go`:
- Around line 216-218: Update the direct exchange call in
cloudProfileFromDirectCodeExchange’s caller to pass the explicit clerk session
token from the --clerk-session-token option instead of the browser token
currently supplied twice. Apply the same token-source correction to the
corresponding direct-exchange paths at the other referenced locations,
preserving browser-token behavior only when no explicit flag is provided.
- Line 41: Update cloudDefaultOrchestratorAuthToken to an empty value so the
access-token fallback is used instead of sending the placeholder bearer token.
Preserve the existing fallback behavior in the default browser login and
interactive pb login Cloud flow, allowing organization resolution without an
explicit token override.
- Around line 858-864: Update cloudClerkScriptURL to replace the moving `@latest`
Clerk SDK tag with one fixed, tested version in both the CDN fallback URL and
the host-based URL, keeping the existing path selection behavior unchanged.
In `@cmd/logout.go`:
- Around line 61-64: Ensure the logout flow remains non-interactive when --yes
is set: before calling selectLogoutProfile, detect the
no-default/multiple-profile case and return an ambiguity error or require an
explicit profile. Preserve normal selector behavior for interactive invocations
and update the existing condition around outputFormat accordingly.
In `@cmd/promql.go`:
- Around line 384-393: Add tests in cmd/promql_test.go covering optional
positional-argument parsing for promqlLabelsCmd, promqlLabelValuesCmd,
promqlSeriesCmd, promqlCardinalityLabelValuesCmd,
promqlCardinalityActiveSeriesCmd, and promqlTSDBCmd, verifying each command uses
the positional argument when provided and preserves the dataset flag/default
behavior otherwise.
In `@cmd/status.go`:
- Around line 35-39: Update the status command’s RunE flow to route preflight
failures such as missing configuration or an active profile through statusOutput
when the output format is JSON, while still returning an error so the command
exits nonzero. Preserve the existing human-readable error behavior for other
formats and use the established statusOutput rendering path.
In `@cmd/tail.go`:
- Around line 154-161: Update the Flight client credential setup in the tail
command to use TLS for config.AuthCloudAPIKey and config.AuthCloudOAuth
profiles, ensuring API keys and session cookies are transmitted over encrypted
gRPC. Preserve insecure.NewCredentials() only for the intentional self-hosted
path, and keep the existing metadata construction unchanged.
In `@pkg/config/config.go`:
- Around line 85-137: The authentication model currently conflates migrated
legacy token values with API keys. Update Profile, AuthMode, and AddAuthHeaders
to retain a distinct legacy token field and authentication mode, classify token
profiles separately from API-key profiles, and continue emitting Authorization:
Bearer for them. Preserve existing cloud, basic, and API-key validation behavior
while rejecting ambiguous combinations consistently.
In `@pkg/datasets/datasets.go`:
- Around line 116-123: Update the dataset-building loop around fetchDatasetType
to avoid serial HTTP requests for every name. Fetch dataset types with bounded
concurrency, ensuring all results are safely associated with their corresponding
Dataset and the client’s request limits are respected, while preserving the
existing behavior when a type lookup fails.
In `@pkg/http/http.go`:
- Around line 61-62: Update NewRequest to validate client.Profile before
constructing the request URL or invoking baseAPIURL, returning the existing
“profile is nil” error through the normal error path. Ensure
DefaultClient(nil).NewRequest(...) does not dereference the nil profile, while
preserving the existing AddAuthHeaders behavior for valid profiles.
In `@pkg/model/login/login.go`:
- Line 110: Configure apiKeyInput to use masked/no-echo input, matching the
existing password input behavior, while preserving its prompt and length limit.
Update the newInput invocation or input configuration associated with
apiKeyInput so pasted API keys are not displayed during entry.
---
Outside diff comments:
In `@cmd/queryList.go`:
- Around line 53-69: The fetchFilters data path currently suppresses request
errors, causing JSON mode to output an empty array and succeed. Update
fetchFilters to return ([]Item, error), propagate NewRequest failures without
printing, and update RunE and all callers to handle the returned error by
returning it before formatting output; preserve successful JSON and non-JSON
behavior.
In `@pkg/model/login/login.go`:
- Around line 579-593: Update the profile rendering logic around the Cloud and
APIKey checks to distinguish OAuth browser profiles from Cloud API-key profiles:
render the Cloud authentication label according to the actual authentication
mode, and avoid emitting a second AUTH row for Cloud API-key profiles. Preserve
the existing username rendering and stored-key messaging for non-Cloud profiles.
---
Nitpick comments:
In `@cmd/dataset_test.go`:
- Line 22: Update the handlers tested in cmd/dataset_test.go to assert that
requests include the expected x-api-key authentication header, using the
configured "test-api-key" value. Add this assertion in at least one handler,
including the additionally referenced handler, while preserving the existing
test behavior.
In `@cmd/promql.go`:
- Around line 384-393: The positional-argument-overrides-flag logic is
duplicated across the PromQL subcommands. Add an argOrFlag helper near the
command implementations, then update promqlLabelsCmd and the corresponding
promqlLabelValuesCmd, promqlSeriesCmd, promqlCardinalityLabelNamesCmd,
promqlCardinalityLabelValuesCmd, promqlCardinalityActiveSeriesCmd, and
promqlTSDBCmd call sites to use it with the appropriate argument index and flag
name.
- Around line 620-633: Deliberately resolve the positional argument order
mismatch between the top-level label-values command and the cardinality
label-values command. Update the cardinality label-values definition, including
its Use, Example, and RunE argument mapping around stream and labelName, to
follow the chosen ordering consistently; document the supported order and
preserve compatibility expectations where applicable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bf1c6043-814d-4416-9f93-e62991edae58
📒 Files selected for processing (17)
cmd/cloud.gocmd/dataset_test.gocmd/login.gocmd/logout.gocmd/profile.gocmd/promql.gocmd/queryList.gocmd/status.gocmd/tail.gomain.gopkg/config/config.gopkg/datasets/datasets.gopkg/http/http.gopkg/model/login/login.gopkg/model/promql.gopkg/model/query.gopkg/model/savedQueries.go
🚧 Files skipped from review as they are similar to previous changes (2)
- cmd/login.go
- pkg/model/promql.go
|
|
||
| const ( | ||
| cloudDefaultOrchestratorURL = "https://orchestrator.cloud-staging.parseable.com" | ||
| cloudDefaultOrchestratorAuthToken = "Add Authbearer token" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Do not send the placeholder bearer token.
The non-empty default makes the access-token fallback unreachable, so default browser login sends Bearer Add Authbearer token. Interactive pb login provides no override, making its Cloud browser flow fail during organization resolution.
Proposed fix
- cloudDefaultOrchestratorAuthToken = "Add Authbearer token"
+ cloudDefaultOrchestratorAuthToken = ""Also applies to: 379-387
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cmd/cloud.go` at line 41, Update cloudDefaultOrchestratorAuthToken to an
empty value so the access-token fallback is used instead of sending the
placeholder bearer token. Preserve the existing fallback behavior in the default
browser login and interactive pb login Cloud flow, allowing organization
resolution without an explicit token override.
| if cloudDirectCodeExchange { | ||
| return cloudProfileFromDirectCodeExchange(cloudWorkspaceURL, cloudTenantID, clerkSessionToken, clerkSessionToken) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Honor --clerk-session-token during direct exchange.
Line 217 always supplies a non-empty browser token, so the explicit flag is never read at Lines 439–441.
Proposed fix
if cloudDirectCodeExchange {
- return cloudProfileFromDirectCodeExchange(cloudWorkspaceURL, cloudTenantID, clerkSessionToken, clerkSessionToken)
+ bearerToken := strings.TrimSpace(cloudClerkSessionTokenFlag)
+ if bearerToken == "" {
+ bearerToken = clerkSessionToken
+ }
+ return cloudProfileFromDirectCodeExchange(cloudWorkspaceURL, cloudTenantID, clerkSessionToken, bearerToken)
}Also applies to: 292-292, 439-441
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cmd/cloud.go` around lines 216 - 218, Update the direct exchange call in
cloudProfileFromDirectCodeExchange’s caller to pass the explicit clerk session
token from the --clerk-session-token option instead of the browser token
currently supplied twice. Apply the same token-source correction to the
corresponding direct-exchange paths at the other referenced locations,
preserving browser-token behavior only when no explicit flag is provided.
| func cloudClerkScriptURL(publishableKey string) string { | ||
| host := cloudClerkFrontendHost(publishableKey) | ||
| if host == "" { | ||
| return "https://cdn.jsdelivr.net/npm/@clerk/clerk-js@latest/dist/clerk.browser.js" | ||
| } | ||
| return "https://" + host + "/npm/@clerk/clerk-js@latest/dist/clerk.browser.js" | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
What versioned URL format does Clerk recommend for loading clerk-js, and which stable version supports mountSignIn, handleRedirectCallback, and session.getToken?
💡 Result:
Clerk recommends loading clerk-js using a versioned URL format that specifies the exact version to ensure stability and predictability. Typically, this is achieved by pinning the version in your import path, for example via a CDN like jsDelivr: https://cdn.jsdelivr.net/npm/@clerk/clerk-js@/dist/clerk.browser.js [1][2]. Regarding the requested methods mountSignIn, handleRedirectCallback, and session.getToken: 1. mountSignIn: This method has been a stable part of the Clerk class API for rendering the SignIn component and is available in current versions of @clerk/clerk-js, including the v6.x series [3][1]. 2. handleRedirectCallback: This method is used to complete authentication flows (such as OAuth or SAML) and is available in current v6.x versions [1][4]. 3. session.getToken: The getToken() method on the Session object is a core, stable feature of the Clerk SDK used for retrieving session tokens or JWT templates and is available in current v6.x versions [5][6]. As of July 2026, these methods are supported in all stable versions within the @clerk/clerk-js v6.x release line [1][2]. It is recommended to use the latest stable version (e.g., v6.7.7 or higher) to ensure access to the most recent features and security updates [2]. [7].
Citations:
- 1: https://cdn.jsdelivr.net/npm/@clerk/clerk-js@6.4.0/dist/types/core/clerk.d.ts
- 2: https://registry.npmjs.org/@clerk/clerk-js
- 3: https://clerk.com/docs/js-frontend/reference/components/authentication/sign-in
- 4: https://clerk.com/docs/nuxt/reference/objects/clerk
- 5: https://clerk.com/docs/js-frontend/reference/objects/session
- 6: https://clerk.com/docs/guides/development/upgrading/upgrading-from-v2-to-v3
- 7: https://clerk.com/docs/guides/development/upgrading/versioning
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant file and find usages of the Clerk script URL helper/version literals.
git ls-files cmd/cloud.go
printf '\n--- outline ---\n'
ast-grep outline cmd/cloud.go --view expanded || true
printf '\n--- occurrences ---\n'
rg -n 'cloudClerkScriptURL|clerk-js@latest|clerk.browser.js|cloudClerkFrontendHost|session.getToken|handleRedirectCallback|mountSignIn' cmd/cloud.go .Repository: parseablehq/pb
Length of output: 7232
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '707,845p' cmd/cloud.goRepository: parseablehq/pb
Length of output: 5164
Pin the Clerk JavaScript version on both URL paths.
These pages load the SDK in the sign-in and redirect flow, so @latest can introduce breaking behavior in login or token handling. Use one tested version instead of the moving tag.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cmd/cloud.go` around lines 858 - 864, Update cloudClerkScriptURL to replace
the moving `@latest` Clerk SDK tag with one fixed, tested version in both the CDN
fallback URL and the host-based URL, keeping the existing path selection
behavior unchanged.
| if outputFormat == "json" && len(fileConfig.Profiles) > 1 { | ||
| return fmt.Errorf("no active profile found") | ||
| } | ||
| selectedProfile, err := selectLogoutProfile(fileConfig.Profiles) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Keep --yes fully non-interactive.
When no default exists and multiple profiles remain, pb logout --yes still opens the selector. Return an ambiguity error or require an explicit profile instead of blocking automation.
Proposed fix
- if outputFormat == "json" && len(fileConfig.Profiles) > 1 {
- return fmt.Errorf("no active profile found")
+ if yes && len(fileConfig.Profiles) > 1 {
+ return fmt.Errorf("no active profile found; set a default profile before using --yes")
}📝 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.
| if outputFormat == "json" && len(fileConfig.Profiles) > 1 { | |
| return fmt.Errorf("no active profile found") | |
| } | |
| selectedProfile, err := selectLogoutProfile(fileConfig.Profiles) | |
| if yes && len(fileConfig.Profiles) > 1 { | |
| return fmt.Errorf("no active profile found; set a default profile before using --yes") | |
| } | |
| selectedProfile, err := selectLogoutProfile(fileConfig.Profiles) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cmd/logout.go` around lines 61 - 64, Ensure the logout flow remains
non-interactive when --yes is set: before calling selectLogoutProfile, detect
the no-default/multiple-profile case and return an ambiguity error or require an
explicit profile. Preserve normal selector behavior for interactive invocations
and update the existing condition around outputFormat accordingly.
| Use: "labels [stream]", | ||
| Short: "List all label names in a metrics stream", | ||
| Example: " pb promql labels --dataset otel_metrics", | ||
| Args: cobra.NoArgs, | ||
| Example: " pb promql labels otel_metrics\n pb promql labels --dataset otel_metrics", | ||
| Args: cobra.MaximumNArgs(1), | ||
| PreRunE: PreRunDefaultProfile, | ||
| RunE: func(cmd *cobra.Command, _ []string) error { | ||
| RunE: func(cmd *cobra.Command, args []string) error { | ||
| stream, _ := cmd.Flags().GetString("dataset") | ||
| if len(args) > 0 { | ||
| stream = args[0] | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd promql_test.go
rg -n 'SetArgs|Args:|promqlLabelsCmd|promqlLabelValuesCmd|promqlSeriesCmd|promqlCardinalityLabelValuesCmd|promqlCardinalityActiveSeriesCmd|promqlTSDBCmd' cmd/promql_test.goRepository: parseablehq/pb
Length of output: 196
🏁 Script executed:
#!/bin/bash
set -euo pipefail
wc -l cmd/promql_test.go
ast-grep outline cmd/promql_test.go --view expanded
printf '\n--- SEARCH ---\n'
rg -n 'SetArgs|Args:|promqlLabelsCmd|promqlLabelValuesCmd|promqlSeriesCmd|promqlCardinalityLabelValuesCmd|promqlCardinalityActiveSeriesCmd|promqlTSDBCmd|label_name|selector|stream' cmd/promql_test.go
printf '\n--- SAMPLE ---\n'
cat -n cmd/promql_test.go | sed -n '1,260p'Repository: parseablehq/pb
Length of output: 440
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline cmd/promql.go --view expanded
printf '\n--- POSitional args in promql.go ---\n'
rg -n 'MaximumNArgs|ExactArgs|MinimumNArgs|Args:' cmd/promql.go
printf '\n--- promql_test.go full file ---\n'
cat -n cmd/promql_test.goRepository: parseablehq/pb
Length of output: 3553
Add tests for the new positional-argument parsing — cmd/promql_test.go only covers promqlInteractiveFromDefault; it doesn’t exercise the optional args on promqlLabelsCmd, promqlLabelValuesCmd, promqlSeriesCmd, promqlCardinalityLabelValuesCmd, promqlCardinalityActiveSeriesCmd, or promqlTSDBCmd.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cmd/promql.go` around lines 384 - 393, Add tests in cmd/promql_test.go
covering optional positional-argument parsing for promqlLabelsCmd,
promqlLabelValuesCmd, promqlSeriesCmd, promqlCardinalityLabelValuesCmd,
promqlCardinalityActiveSeriesCmd, and promqlTSDBCmd, verifying each command uses
the positional argument when provided and preserves the dataset flag/default
behavior otherwise.
| case config.AuthCloudAPIKey: | ||
| values := map[string]string{"x-api-key": profile.APIKey} | ||
| values["x-p-tenant"] = profile.TenantID | ||
| return metadata.New(values), nil | ||
| case config.AuthCloudOAuth: | ||
| values := map[string]string{"cookie": "session=" + profile.SessionToken} | ||
| values["x-p-tenant"] = profile.TenantID | ||
| return metadata.New(values), nil |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C4 '\bGrpcAddr\s*\(' --type go .
rg -n -C4 'NewClientWithMiddleware|WithTransportCredentials|insecure\.NewCredentials|credentials\.NewTLS' --type go .Repository: parseablehq/pb
Length of output: 1394
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant files first
ast-grep outline cmd/tail.go --view expanded
ast-grep outline pkg/config/config.go --view expanded
# Read the relevant sections with line numbers
sed -n '1,220p' cmd/tail.go | cat -n
printf '\n----\n'
sed -n '1,220p' pkg/config/config.go | cat -nRepository: parseablehq/pb
Length of output: 17457
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Look for any TLS / credential selection around the flight client and profile auth paths
rg -n -C3 'insecure\.NewCredentials|WithTransportCredentials|tls\.|credentials\.' cmd pkg --type go
rg -n -C3 'AuthCloudAPIKey|AuthCloudOAuth|AuthSelfHosted|SessionToken|APIKey|TenantID' cmd pkg --type goRepository: parseablehq/pb
Length of output: 25986
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check whether the project ever documents gRPC/Flight TLS for Cloud or self-hosted
rg -n -C2 'gRPC|Flight|TLS|insecure|certificate|credentials' README.md docs cmd pkg --type go --type mdRepository: parseablehq/pb
Length of output: 6650
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the exact call chain for tailing/auth metadata
sed -n '1,240p' cmd/tail.go | cat -n
printf '\n----\n'
sed -n '1,220p' pkg/config/config.go | cat -n
printf '\n----\n'
rg -n -C4 'metadata\.New|WithTransportCredentials|NewClientWithMiddleware|GrpcAddr\(' cmd pkg --type goRepository: parseablehq/pb
Length of output: 17452
Use TLS for Cloud gRPC traffic. tail always creates the Flight client with insecure.NewCredentials(), so Cloud API keys and session cookies are sent over plaintext gRPC. Switch Cloud profiles to TLS credentials; keep self-hosted insecure only if that path is intentional.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cmd/tail.go` around lines 154 - 161, Update the Flight client credential
setup in the tail command to use TLS for config.AuthCloudAPIKey and
config.AuthCloudOAuth profiles, ensuring API keys and session cookies are
transmitted over encrypted gRPC. Preserve insecure.NewCredentials() only for the
intentional self-hosted path, and keep the existing metadata construction
unchanged.
| type AuthMode string | ||
|
|
||
| const ( | ||
| // AuthSelfHostedBasic identifies username/password authentication. | ||
| AuthSelfHostedBasic AuthMode = "self_hosted_basic" | ||
| // AuthSelfHostedAPIKey identifies API-key authentication for a self-hosted server. | ||
| AuthSelfHostedAPIKey AuthMode = "self_hosted_api_key" | ||
| // AuthCloudAPIKey identifies API-key authentication for Parseable Cloud. | ||
| AuthCloudAPIKey AuthMode = "cloud_api_key" | ||
| // AuthCloudOAuth identifies browser-based OAuth authentication for Parseable Cloud. | ||
| AuthCloudOAuth AuthMode = "cloud_oauth" | ||
| ) | ||
|
|
||
| func (p Profile) AuthMode() (AuthMode, error) { | ||
| hasBasic := p.Username != "" || p.Password != "" | ||
| hasAPIKey := p.APIKey != "" | ||
| hasCloudOAuth := p.SessionToken != "" | ||
|
|
||
| if p.Cloud { | ||
| if p.TenantID == "" { | ||
| return "", errors.New("cloud profile missing tenant_id") | ||
| } | ||
| if hasBasic { | ||
| return "", errors.New("cloud profile cannot use self-hosted credentials") | ||
| } | ||
| switch { | ||
| case hasAPIKey && !hasCloudOAuth: | ||
| return AuthCloudAPIKey, nil | ||
| case hasCloudOAuth && !hasAPIKey: | ||
| return AuthCloudOAuth, nil | ||
| case hasAPIKey && hasCloudOAuth: | ||
| return "", errors.New("cloud profile has both apiKey and session_token") | ||
| default: | ||
| return "", errors.New("cloud profile missing apiKey or session_token") | ||
| } | ||
| } | ||
|
|
||
| if hasCloudOAuth || p.TenantID != "" { | ||
| return "", errors.New("self-hosted profile cannot use cloud credentials") | ||
| } | ||
| switch { | ||
| case hasBasic && !hasAPIKey: | ||
| if p.Username == "" || p.Password == "" { | ||
| return "", errors.New("self-hosted basic profile missing username or password") | ||
| } | ||
| return AuthSelfHostedBasic, nil | ||
| case hasAPIKey && !hasBasic: | ||
| return AuthSelfHostedAPIKey, nil | ||
| case hasBasic && hasAPIKey: | ||
| return "", errors.New("self-hosted profile has both basic credentials and api key") | ||
| default: | ||
| return "", errors.New("self-hosted profile missing credentials") | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Preserve legacy Bearer-token authentication separately.
Legacy token values are migrated into APIKey, causing AddAuthHeaders to send them as x-api-key instead of the previous Authorization: Bearer .... Existing token profiles will stop authenticating. Retain a distinct token field/auth mode and continue emitting the Bearer header.
Also applies to: 196-196, 201-225
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 90-90: A credential is hard-coded as a string literal. Secrets stored in source code, such as passwords, API keys, and tokens, can be leaked through version control or binaries and used by internal or external malicious actors. Rotate the exposed secret and load it at runtime from a secure secret vault, a Hardware Security Module (HSM), or an environment variable if permitted by your company policy (e.g. password := os.Getenv("APP_PASSWORD")).
Context: AuthSelfHostedAPIKey AuthMode = "self_hosted_api_key"
Note: [CWE-798] Use of Hard-coded Credentials.
(hardcoded-credentials-string-literal-go)
[warning] 92-92: A credential is hard-coded as a string literal. Secrets stored in source code, such as passwords, API keys, and tokens, can be leaked through version control or binaries and used by internal or external malicious actors. Rotate the exposed secret and load it at runtime from a secure secret vault, a Hardware Security Module (HSM), or an environment variable if permitted by your company policy (e.g. password := os.Getenv("APP_PASSWORD")).
Context: AuthCloudAPIKey AuthMode = "cloud_api_key"
Note: [CWE-798] Use of Hard-coded Credentials.
(hardcoded-credentials-string-literal-go)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/config/config.go` around lines 85 - 137, The authentication model
currently conflates migrated legacy token values with API keys. Update Profile,
AuthMode, and AddAuthHeaders to retain a distinct legacy token field and
authentication mode, classify token profiles separately from API-key profiles,
and continue emitting Authorization: Bearer for them. Preserve existing cloud,
basic, and API-key validation behavior while rejecting ambiguous combinations
consistently.
| items := make([]Dataset, 0, len(names)) | ||
| for _, name := range names { | ||
| item := Dataset{Title: name} | ||
| if datasetType, err := fetchDatasetType(&client, name); err == nil { | ||
| item.DatasetType = datasetType | ||
| } | ||
| items = append(items, item) | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
Avoid serial per-dataset HTTP requests.
This introduces an O(N) request waterfall, with each information request potentially consuming the client’s 60-second timeout. Fetch types with bounded concurrency, or obtain/lazily load type metadata without blocking the complete dataset list.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/datasets/datasets.go` around lines 116 - 123, Update the dataset-building
loop around fetchDatasetType to avoid serial HTTP requests for every name. Fetch
dataset types with bounded concurrency, ensuring all results are safely
associated with their corresponding Dataset and the client’s request limits are
respected, while preserving the existing behavior when a type lookup fails.
| if err = AddAuthHeaders(req, client.Profile); err != nil { | ||
| return |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Validate the profile before constructing the request URL.
baseAPIURL dereferences client.Profile before AddAuthHeaders can return "profile is nil", so DefaultClient(nil).NewRequest(...) panics.
Proposed fix
func (client *HTTPClient) NewRequest(method string, path string, body io.Reader) (req *http.Request, err error) {
+ if client.Profile == nil {
+ return nil, errors.New("profile is nil")
+ }
req, err = http.NewRequest(method, client.baseAPIURL(path), body)📝 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.
| if err = AddAuthHeaders(req, client.Profile); err != nil { | |
| return | |
| func (client *HTTPClient) NewRequest(method string, path string, body io.Reader) (req *http.Request, err error) { | |
| if client.Profile == nil { | |
| return nil, errors.New("profile is nil") | |
| } | |
| req, err = http.NewRequest(method, client.baseAPIURL(path), body) | |
| if err != nil { | |
| return | |
| } | |
| if err = AddAuthHeaders(req, client.Profile); err != nil { | |
| return |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/http/http.go` around lines 61 - 62, Update NewRequest to validate
client.Profile before constructing the request URL or invoking baseAPIURL,
returning the existing “profile is nil” error through the normal error path.
Ensure DefaultClient(nil).NewRequest(...) does not dereference the nil profile,
while preserving the existing AddAuthHeaders behavior for valid profiles.
| passwordInput.EchoCharacter = '•' | ||
|
|
||
| tokenInput := newInput("paste API key here", 512) | ||
| apiKeyInput := newInput("paste API key here", 512) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Mask API keys while they are entered.
Unlike the password input, apiKeyInput uses visible echo, exposing pasted Cloud and self-hosted credentials on screen.
Proposed fix
apiKeyInput := newInput("paste API key here", 512)
+ apiKeyInput.EchoMode = textinput.EchoPassword
+ apiKeyInput.EchoCharacter = '•'📝 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.
| apiKeyInput := newInput("paste API key here", 512) | |
| apiKeyInput := newInput("paste API key here", 512) | |
| apiKeyInput.EchoMode = textinput.EchoPassword | |
| apiKeyInput.EchoCharacter = '•' |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/model/login/login.go` at line 110, Configure apiKeyInput to use
masked/no-echo input, matching the existing password input behavior, while
preserving its prompt and length limit. Update the newInput invocation or input
configuration associated with apiKeyInput so pasted API keys are not displayed
during entry.
1 participant
Summary
Adds Parseable Cloud API key support to
pb.Users can now create a Cloud profile using either:
or interactive login:
Then select Parseable Cloud and paste API key.
What Changed
Added pb cloud profile add
Enabled Parseable Cloud option in interactive pb login
Added cloud-aware profile fields:
Added API key validation through orchestrator:
Added cloud request auth headers:
Kept self-hosted BasicAuth/token profiles backward compatible
Config file writes with 0600 permissions because API key is stored locally
Summary by CodeRabbit
--output/-o(text/json) plus JSON help; added JSON output to commands likestatus,logout, andtail.