fix(cli): allow swamp issue get to run without authentication (swamp-club#313)#1360
Conversation
…p-club#313) The command unconditionally required credentials before making the API request, even though the swamp-club endpoint supports unauthenticated reads for public issues. Make auth optional: when credentials exist the API key is sent for authenticated access; when absent the request is made without the x-api-key header, falling back to SWAMP_CLUB_URL or the default server URL. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Adversarial Review
Critical / High
None.
Medium
None.
Low
-
swamp_club_client.ts:276— empty-stringapiKeysilently treated as unauthenticated. IfAuthRepository.load()returns credentials whereapiKeyis""(e.g. anSWAMP_API_KEY=""env var — technically truthy for the env check on line 90 ofauth_repository.tsbut wait, no: empty-string is falsy in theif (envApiKey)guard, so this path returnsnull). After tracing throughAuthRepository.load(),credentials?.apiKeycan only beundefined(whencredentialsisnull) or a non-empty string (either from env-var with the falsy check, or from a deserializedauth.jsonwhere an empty apiKey would be a corrupted file). Theif (apiKey)check is safe in practice. Purely theoretical. -
issue_get.ts:51-52—SWAMP_CLUB_URLenv var checked twice on the unauthenticated path. Whencredentialsisnull,AuthRepository.load()already checkedSWAMP_CLUB_URLinternally (and returnednullbecause there was noSWAMP_API_KEY). The command then re-reads the same env var forserverUrl. This is harmless — consistent withextension_search.ts,extension_list.ts, and other commands using the identical pattern — but worth noting the env var is read redundantly. Not a bug.
Verdict
PASS. Clean, minimal change that correctly follows the established unauthenticated-access pattern used by extension search, extension list, extension outdated, and extension update. The fetchIssue signature widening from string to string | undefined is backwards-compatible, the ?? fallback chain has correct precedence, the UserError import is still used (line 47), and no callers are broken by the change.
There was a problem hiding this comment.
CLI UX Review
Blocking
None.
Suggestions
- ** — has no 401/403 handling.** If the server ever returns 401/403 for a protected issue (e.g., a private/internal issue), the error is
Failed to fetch issue #N (HTTP 401): <raw server text>, with no hint to runswamp auth login. The removed "Not logged in" guard used to cover this. Since the PR author confirms the public API always returns 200 without an API key, this is unlikely to bite users today — but adding ares.status === 401 || res.status === 403branch with a message like'Issue #N requires authentication. Run "swamp auth login" first.'would close the gap cleanly.
Verdict
PASS — removes a spurious auth gate on a public endpoint, follows the same URL-fallback pattern as extension search / extension list, and causes no regression in help text, output format, or error messages.
There was a problem hiding this comment.
CLI UX Review
Blocking
None.
Suggestions
fetchIssuehas no 401/403 handling (src/infrastructure/http/swamp_club_client.ts). If the server ever returns 401/403 for a protected issue, the error isFailed to fetch issue #N (HTTP 401): <raw server text>, with no hint to runswamp auth login. The removed "Not logged in" guard used to cover this path. Since the PR author confirms the public API returns 200 without an API key this is unlikely to bite users today, but ares.status === 401 || res.status === 403branch with a message likeIssue #N requires authentication. Run "swamp auth login" first.would close the gap cleanly.
Verdict
PASS — removes a spurious auth gate on a public endpoint, follows the same URL-fallback pattern as extension search / extension list, and causes no regression in help text, output format, or error messages.
There was a problem hiding this comment.
Code Review
Clean, minimal change that correctly makes swamp issue get work without authentication. The approach is sound and well-implemented.
Blocking Issues
None.
Suggestions
- Test coverage for
fetchIssueunauthenticated path: Theswamp_club_client_test.tshas no tests forfetchIssueat all (pre-existing gap), andissue_get_test.tsonly checks command metadata. A test verifying that thex-api-keyheader is omitted whenapiKeyisundefinedwould strengthen confidence, but this is a pre-existing gap not introduced by this PR.
Notes
- Import boundary: All imports are correct.
DEFAULT_SWAMP_CLUB_URLcomes from../../domain/auth/auth_credentials.tsandSwampClubClientfrom../../infrastructure/http/swamp_club_client.ts— neither are libswamp-internal paths. This matches the pattern used by all other CLI commands (extension_search,extension_list,auth_login, etc.). - DDD: No domain model violations. Infrastructure changes are properly confined to the HTTP client layer. CLI command orchestration is clean.
- Pattern consistency: The server URL fallback (
credentials?.serverUrl ?? env var ?? default) is a slight improvement over other unauthenticated commands (which skip credential loading entirely) because it respects a logged-in user's configured server URL while still working without auth. - Security: API key is properly omitted (not sent as empty string) when credentials are absent. The
headersobject is cleanly constructed with a conditional property add. - No
anytypes introduced, copyright headers present, no fire-and-forget promises.
Summary
swamp issue getwork without authentication by making the authcheck optional — when credentials exist the API key is sent for
authenticated access; when absent the request is unauthenticated
SwampClubClient.fetchIssuenow acceptsapiKey: string | undefinedand omits the
x-api-keyheader when undefinedSWAMP_CLUB_URLenv var orDEFAULT_SWAMP_CLUB_URLwhen no stored credentials exist, matching the pattern used by
extension search,extension list, and other unauthenticated commandsCloses swamp-club#313
Test Plan
swamp issue get 1with no credentials fails with"Not logged in" error
returns 200 without
x-api-key)deno checkpassesdeno lintpassesdeno fmt --checkpasses🤖 Generated with Claude Code