Skip to content

Conversation

@YaroSpace
Copy link
Collaborator

  • docs: update keymaps docs

YaroSpace and others added 7 commits January 24, 2026 10:03
* fix: use standard Base64 encoding for OAuth2 Basic authentication

- Add base64_encode_standard() function for OAuth2 Basic auth headers
- Keep base64_encode() as URL-safe for PKCE and JWT use cases
- Update oauth.lua to use standard Base64 for Authorization: Basic headers

This fixes the OAuth2 client credentials authentication issue where kulala
was incorrectly using URL-safe Base64 encoding (+ -> -, / -> _) instead of
standard Base64 encoding as required by RFC 6749 Section 2.3.1.

Fixes authentication failures with OAuth2 providers that strictly validate
Base64 encoding in Authorization: Basic headers.

* test: add comprehensive tests for Base64 encoding fix with safe credentials

- Add unit tests for base64_encode_standard() and base64_encode() functions
- Add integration tests for OAuth2 Basic auth with safe test credentials
- Add regression test ensuring PKCE still uses URL-safe encoding

All test credentials are randomly generated safe values that produce the
required Base64 patterns (+ and / characters) without exposing real secrets.

Tests verify:
- Standard Base64 encoding for OAuth2 Basic auth (fixes "invalid_client" errors)
- URL-safe Base64 encoding still works for PKCE/JWT (no regression)
- Proper character substitutions: + vs -, / vs _, padding handling

This prevents regression of the OAuth2 client credentials authentication
bug where kulala incorrectly used URL-safe Base64 instead of standard
Base64 for Authorization: Basic headers per RFC 7617.

Co-Authored-By: Claude (claude-sonnet-4) <noreply@anthropic.com>

* fix: update OAuth Basic auth test to expect standard Base64 with padding

The existing test expected URL-safe Base64 encoding (Y2xpZW50X2lkOmNsaWVudF9zZWNyZXQ)
but the fix correctly changed OAuth2 Basic auth to use standard Base64 encoding
(Y2xpZW50X2lkOmNsaWVudF9zZWNyZXQ=) per RFC 7617.

For the test credentials "client_id:client_secret", the only difference is
padding (= character) since this string doesn't produce + or / characters.

This fixes the CI test failure caused by the Base64 encoding fix.

Co-Authored-By: Claude (claude-sonnet-4) <noreply@anthropic.com>

* fix: update OAuth tests to properly override Client Secret in private env

The test failures were caused by Client Secret values being overridden by the
private environment configuration. The update_env() function requires a second
parameter `true` to update the private environment where "Client Secret" is defined.

Changes:
- Update "Client Secret" in private environment for + character test
- Update "Client Secret" in private environment for / character test
- Keep "Client ID" and other settings in public environment

This ensures the test credentials (test:> and user123:?pass) are used instead
of the default client_secret, allowing proper testing of Base64 character handling.

Co-Authored-By: Claude (claude-sonnet-4) <noreply@anthropic.com>

* fix: remove URL encoding from OAuth2 Basic auth credentials

---------

Co-authored-by: 🚀 Niklas Arens <niklas.arens@mercedes-benz.com>
Co-authored-by: Claude (claude-sonnet-4) <noreply@anthropic.com>
Co-authored-by: Marco Kellershoff <1384938+gorillamoe@users.noreply.github.com>
* origin/main:
  docs: Auto-generate vimdocs + vimtags (#830)
  Weekly Updates (#819)
@YaroSpace YaroSpace requested a review from gorillamoe as a code owner February 8, 2026 22:11
Copilot AI review requested due to automatic review settings February 8, 2026 22:11
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the default keymaps documentation to clarify interruption behavior and improve wording for WebSocket keybindings.

Changes:

  • Documented a new keymap for interrupting in-flight requests (<C-c>).
  • Renamed the WebSocket <C-c> keymap label to “Close WS connection”.
  • Removed an inline desc note from the WebSocket close keymap entry.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

["Send request"] = { "s", function() require("kulala").run() end, mode = { "n", "v" }, },
["Send request <cr>"] = { "<CR>", function() require("kulala").run() end, mode = { "n", "v" }, ft = { "http", "rest" }, },
["Send all requests"] = { "a", function() require("kulala").run_all() end, mode = { "n", "v" }, },
["Interrupt requests"] = { "<C-c>", function() require("kulala.ui").interrupt_requests() end },
Copy link

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documentation now assigns <C-c> to two different actions (“Interrupt requests” and “Close WS connection”) without clarifying that they apply in different contexts (e.g., different ft/modes or websocket buffer state). Add an explicit note or add the appropriate ft = { ... }/mode = { ... } fields in the examples so readers understand when each mapping is active and avoid assuming a global conflict.

Suggested change
["Interrupt requests"] = { "<C-c>", function() require("kulala.ui").interrupt_requests() end },
["Interrupt requests"] = { "<C-c>", function() require("kulala.ui").interrupt_requests() end, ft = { "http", "rest" }, },

Copilot uses AI. Check for mistakes.

["Send WS message"] = { "<S-CR>", function() require("kulala.cmd.websocket").send() end, mode = { "n", "v" }, },
["Interrupt requests"] = { "<C-c>", function() require("kulala.cmd.websocket").close() end, desc = "also: CLose WS connection" },
["Close WS connection"] = { "<C-c>", function() require("kulala.cmd.websocket").close() end },
Copy link

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documentation now assigns <C-c> to two different actions (“Interrupt requests” and “Close WS connection”) without clarifying that they apply in different contexts (e.g., different ft/modes or websocket buffer state). Add an explicit note or add the appropriate ft = { ... }/mode = { ... } fields in the examples so readers understand when each mapping is active and avoid assuming a global conflict.

Copilot uses AI. Check for mistakes.

["Send WS message"] = { "<S-CR>", function() require("kulala.cmd.websocket").send() end, mode = { "n", "v" }, },
["Interrupt requests"] = { "<C-c>", function() require("kulala.cmd.websocket").close() end, desc = "also: CLose WS connection" },
["Close WS connection"] = { "<C-c>", function() require("kulala.cmd.websocket").close() end },
Copy link

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The prior entry had a helpful clarification (“also: Close WS connection”) and the updated version removes that context entirely. Consider reintroducing a clear description (and fix the previous capitalization/typo) so users understand that <C-c> is meant specifically to close the current WebSocket connection.

Suggested change
["Close WS connection"] = { "<C-c>", function() require("kulala.cmd.websocket").close() end },
["Close WS connection"] = { "<C-c>", function() require("kulala.cmd.websocket").close() end, desc = "Close current WebSocket connection" },

Copilot uses AI. Check for mistakes.
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.

2 participants