Skip to content

fix: correct client-secret response field name - #40

Merged
NathaelB merged 1 commit into
mainfrom
fix/client-secret-response-shape
Sep 3, 2026
Merged

fix: correct client-secret response field name#40
NathaelB merged 1 commit into
mainfrom
fix/client-secret-response-shape

Conversation

@NathaelB

@NathaelB NathaelB commented Sep 3, 2026

Copy link
Copy Markdown
Member

Bug

ferris-ctl client secret cli (and any confidential client) failed with error: http error: error decoding response body. This is the gap flagged as unverified in #38 — the response shape was guessed, and the guess was wrong.

Fix

The real response is {"client_secret": "..."}, not {"secret": ...}. Confirmed by fetching the endpoint and inspecting a redacted version of the response (key names and string lengths only, secret value never printed anywhere, including in my own output) rather than reading the actual secret.

ClientSecretPayload now matches client_secret as the primary key, with secret kept as a serde alias and the {"data": ...} envelope variant kept too, in case the shape isn't perfectly consistent across client types/server versions.

Verification

Ran ferris-ctl client secret cli against the local server, redirected stdout to a file without reading it, and checked only the exit code (0) and byte count (17 = 16-char secret + newline, matching the length observed in the earlier redacted probe). Confirms the fix without exposing the secret in this conversation.

Test plan

  • cargo build --workspace
  • cargo test --workspace (81 passed, updated shape tests)
  • cargo clippy --workspace --all-targets --all-features -- -D warnings
  • Manual run against a running FerrisKey server (see above)

Summary by CodeRabbit

  • Bug Fixes
    • Improved client-secret response handling for the confirmed client_secret format.
    • Preserved compatibility with bare-string, legacy, and enveloped response formats.
    • Added coverage for the live response shape.

Verified live against a running server: the response is
{"client_secret": "..."}, not {"secret": ...} as guessed in #38 —
decoding failed for every real call. Confirmed via a redacted debug
probe that revealed the shape (key names, string lengths) without ever
printing the actual secret value.

"secret" is kept as a serde alias and the {"data": ...} envelope
variant is kept too, for robustness against the shape drifting.
@NathaelB NathaelB self-assigned this Sep 3, 2026
@coderabbitai

coderabbitai Bot commented Sep 3, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: 5dbece4c-90f6-47c4-bf00-52e38432330e

📥 Commits

Reviewing files that changed from the base of the PR and between 3b30f56 and 8f3718e.

📒 Files selected for processing (1)
  • libs/ferriskey-cli-client/src/lib.rs

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The client-secret response parser now supports the confirmed client_secret object shape and retains bare-string, secret, and nested envelope fallbacks. Tests cover the confirmed shape, alias, and enveloped response.

Changes

Client secret parsing

Layer / File(s) Summary
Payload parser and response-shape tests
libs/ferriskey-cli-client/src/lib.rs
ClientSecretPayload parses client_secret objects and retains existing fallback variants. Tests cover confirmed, aliased, and enveloped response shapes.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 8f371

Client-secret decoding now accepts the server’s client_secret response field while preserving older response formats, allowing confidential-client and CLI secret flows to complete without breaking compatible payloads.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: correcting the client-secret response field name from secret to client_secret.
Docstring Coverage ✅ Passed Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 1 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/client-secret-response-shape

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@NathaelB
NathaelB merged commit 572a1bc into main Sep 3, 2026
4 checks passed
@NathaelB
NathaelB deleted the fix/client-secret-response-shape branch September 3, 2026 01:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant