diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a4ba3a..a31c363 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,21 @@ versions follow [Semantic Versioning](https://semver.org/spec/v2.0.0.html). most on a USB stick or someone else's machine. - Installable with [Scoop](https://scoop.sh), which also avoids the "unrecognized app" prompt. +## [0.2.1] — 2026-08-03 + +Two defects that shipped in 0.2.0, both found by using the app rather than reading it. + +### Fixed + +- **A mistyped or revoked key was accepted and saved.** OpenKey checked keys against an endpoint + that doesn't require one, so any text passed. You'd see "Key saved. You're ready to chat", then + every message would fail with advice to run `/reset` — which brought you back to the same screen. + Keys are now genuinely verified before being saved. +- **Piped or scripted input crashed the app.** Anything needing a menu — choosing a model, + confirming an erase, first-run setup — closed OpenKey with an error when input didn't come from a + keyboard. Those now fall back to typing a number or a word. +- Choosing a model said the choice lasted "until you close OpenKey". It's remembered. + ## [0.2.0] — 2026-08-03 ### Added @@ -106,6 +121,7 @@ versions follow [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - Commands: `/about`, `/models`, `/model`, `/cls`, `/help`, `/reset`, `/quit`. - Single self-contained `.exe` that runs from a USB stick with nothing installed. -[Unreleased]: https://github.com/corecompiled/OpenKey/compare/v0.2.0...HEAD +[Unreleased]: https://github.com/corecompiled/OpenKey/compare/v0.2.1...HEAD +[0.2.1]: https://github.com/corecompiled/OpenKey/releases/tag/v0.2.1 [0.2.0]: https://github.com/corecompiled/OpenKey/releases/tag/v0.2.0 [0.1.0]: https://github.com/corecompiled/OpenKey/releases/tag/v0.1.0 diff --git a/Directory.Build.props b/Directory.Build.props index f4256df..1f76133 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -8,7 +8,7 @@ true latest Recommended - 0.2.0 + 0.2.1 en-US diff --git a/docs/03-openrouter-integration.md b/docs/03-openrouter-integration.md index 6849825..4bd084e 100644 --- a/docs/03-openrouter-integration.md +++ b/docs/03-openrouter-integration.md @@ -70,6 +70,17 @@ new ModelInfo( IsFree: true); ``` +## Endpoint 1b — `GET /key` (validating a key) + +`GET /api/v1/key` with the `Authorization` header. Returns 200 for a usable key and **401** for one +that is missing, mistyped, or revoked. + +Use this — and only this — to check a key before saving it. **`GET /models` is public**: it answers +200 with no `Authorization` header at all, so validating against it accepts any string as a valid +key. That is not theoretical; it shipped. The symptom was a mistyped key being saved with "You're +ready to chat", then failing on every message, with the error advising a `/reset` that led straight +back to the same screen. + ## Endpoint 2 — `POST /chat/completions` (streaming) Request body: diff --git a/src/OpenKey.Providers.OpenRouter/OpenRouterProvider.cs b/src/OpenKey.Providers.OpenRouter/OpenRouterProvider.cs index f59822d..c2feafa 100644 --- a/src/OpenKey.Providers.OpenRouter/OpenRouterProvider.cs +++ b/src/OpenKey.Providers.OpenRouter/OpenRouterProvider.cs @@ -40,6 +40,50 @@ public OpenRouterProvider(HttpClient http, Func keyProvider) public string Id => "openrouter"; public string DisplayName => "OpenRouter"; + /// + /// Checks that the key is actually accepted, throwing with + /// when it is not. + /// + /// Not part of — key acquisition is provider-specific, and both + /// hosts already construct this type directly to validate before saving. + /// + /// + /// This exists because GET /models is a public endpoint: it answers 200 with no + /// Authorization header at all. Validating against it accepted any string as a valid key, so a + /// mistyped key was saved with "You're ready to chat" and then failed on every message, with + /// the error advising a /reset that led straight back to the same place. + /// GET /key requires authentication and answers 401. + /// + /// + public async Task ValidateKeyAsync(CancellationToken ct) + { + using var req = new HttpRequestMessage(HttpMethod.Get, $"{BaseUrl}/key"); + ApplyHeaders(req); + + using var cts = CancellationTokenSource.CreateLinkedTokenSource(ct); + cts.CancelAfter(ModelListTimeout); + + HttpResponseMessage resp; + try + { + resp = await _http.SendAsync(req, HttpCompletionOption.ResponseHeadersRead, cts.Token); + } + catch (OperationCanceledException) when (!ct.IsCancellationRequested) + { + throw new ChatException(ChatErrorKind.NetworkDown, "OpenRouter didn't respond in time."); + } + catch (Exception ex) when (IsNetwork(ex)) + { + throw new ChatException(ChatErrorKind.NetworkDown, "Network unreachable.", null, ex); + } + + if (resp.IsSuccessStatusCode) return; + + await using var stream = await resp.Content.ReadAsStreamAsync(cts.Token); + var body = await ReadBodyAsync(stream, cts.Token); + throw MapHttpError(resp, body); + } + public async Task> ListModelsAsync(CancellationToken ct) { using var req = new HttpRequestMessage(HttpMethod.Get, $"{BaseUrl}/models"); diff --git a/src/OpenKey/CommandRouter.cs b/src/OpenKey/CommandRouter.cs index 403f88f..524dd64 100644 --- a/src/OpenKey/CommandRouter.cs +++ b/src/OpenKey/CommandRouter.cs @@ -109,7 +109,7 @@ public async Task HandleAsync(string input, CancellationToken ct) + "You'll need to sign in again.", "Only continue if you meant to start completely fresh."); - if (AnsiConsole.Confirm("Erase everything and start over?", defaultValue: false)) + if (Prompts.Confirm("Erase everything and start over?")) { await _resetAction(ct); } @@ -350,16 +350,15 @@ await AnsiConsole.Status() .Select(m => $"{Truncate(m.DisplayName, 44).PadRight(46)}{FormatContext(m.ContextLength)}") .ToList(); - var prompt = new SelectionPrompt - { - Title = Components.PickerTitle("Which model should answer you?"), - PageSize = 12, - MoreChoicesText = $"[{Theme.Muted}]More below[/]", - }; - prompt.AddChoice(AutoChoiceLabel); - foreach (var row in rows) prompt.AddChoice(row); + var choices = new List { AutoChoiceLabel }; + choices.AddRange(rows); - string choice = AnsiConsole.Prompt(prompt); + var choice = Prompts.Select("Which model should answer you?", choices); + if (choice is null) + { + Components.HintLine("Kept the current choice."); + return; + } if (choice == AutoChoiceLabel) { @@ -371,7 +370,7 @@ await AnsiConsole.Status() var picked = models[rows.IndexOf(choice)]; _engine.PreferredModelId = picked.Id; Components.SuccessLine($"Now using {picked.DisplayName}."); - Components.HintLine("This lasts until you close OpenKey."); + Components.HintLine("Remembered for next time. Choose Auto to hand the choice back."); } private static string FormatContext(int contextLength) => diff --git a/src/OpenKey/ConsoleHost.cs b/src/OpenKey/ConsoleHost.cs index a54b047..3ca541d 100644 --- a/src/OpenKey/ConsoleHost.cs +++ b/src/OpenKey/ConsoleHost.cs @@ -332,10 +332,12 @@ private async Task EnsureFirstRunAsync(CancellationToken ct) { // No "(attempt 1/3)" counter: showing a retry budget before anything has failed // manufactures anxiety. Retries are surfaced only after a failure. - var choice = AnsiConsole.Prompt( - new SelectionPrompt() - .Title(Components.PickerTitle("How would you like to connect?")) - .AddChoices(OAuthChoice, PasteChoice)); + var choice = Prompts.Select("How would you like to connect?", new[] { OAuthChoice, PasteChoice }); + if (choice is null) + { + Components.HintLine("No option chosen."); + continue; + } string? key = null; @@ -471,12 +473,7 @@ private static string AcquireKeyViaPaste() Components.HintLine("You can create one at https://openrouter.ai/keys"); AnsiConsole.WriteLine(); - return AnsiConsole.Prompt( - new TextPrompt("Key: ") - .Secret() - .Validate(k => string.IsNullOrWhiteSpace(k) - ? ValidationResult.Error($"[{Theme.Danger}]Paste a key to continue, or press Ctrl+C to go back.[/]") - : ValidationResult.Success())); + return Prompts.Secret("Key: ") ?? string.Empty; } private async Task ValidateAndSaveAsync(string key, CancellationToken ct) @@ -490,6 +487,9 @@ await AnsiConsole.Status() .SpinnerStyle(new Style(Color.Grey)) .StartAsync($"[{Theme.Muted}]Checking your key[/]", async _ => { + // Auth check first: /models is public and answers 200 for anyone, so on its + // own it would accept any string as a valid key. + await tmp.ValidateKeyAsync(ct); models = await tmp.ListModelsAsync(ct); }); @@ -525,7 +525,7 @@ await AnsiConsole.Status() ex.Message, "OpenKey can save the key now and check it the first time you chat."); - if (AnsiConsole.Confirm("Save the key and continue?", defaultValue: false)) + if (Prompts.Confirm("Save the key and continue?")) { _keyStore.Save(key); return true; diff --git a/src/OpenKey/Ui/Prompts.cs b/src/OpenKey/Ui/Prompts.cs new file mode 100644 index 0000000..12f1579 --- /dev/null +++ b/src/OpenKey/Ui/Prompts.cs @@ -0,0 +1,106 @@ +using Spectre.Console; + +namespace OpenKey.Ui; + +/// +/// Interactive prompts that degrade instead of throwing. +/// +/// Spectre's prompts raise when the terminal is not +/// interactive — which is true whenever any standard stream is redirected, so it happens for +/// `openkey < script.txt`, for piped input, and inside CI. Nothing caught it, so a piped +/// /models killed the app with a raw exception name on screen. +/// +/// +/// Every fallback here reads a plain line instead, which is exactly what a piped caller can +/// provide. Destructive confirmations deliberately require an explicit word rather than accepting +/// a bare newline. +/// +/// +internal static class Prompts +{ + /// True when Spectre's own prompts are safe to use. + public static bool CanPrompt => ConsoleLayout.Rich; + + public static bool Confirm(string question, bool defaultValue = false) + { + if (CanPrompt) + { + try { return AnsiConsole.Confirm(question, defaultValue); } + catch (NotSupportedException) { /* fall through */ } + } + + AnsiConsole.Markup($"{Markup.Escape(question)} [{Theme.Muted}](yes/no)[/] "); + var answer = Console.ReadLine(); + if (Console.IsInputRedirected) AnsiConsole.WriteLine(); + + // No answer means no. A destructive default of "yes" on EOF would be indefensible. + return answer?.Trim().ToLowerInvariant() is "y" or "yes"; + } + + /// + /// Presents a choice. Returns the selected item, or null when the user declined or nothing + /// could be read. + /// + public static string? Select(string title, IReadOnlyList choices) + { + if (choices.Count == 0) return null; + + if (CanPrompt) + { + try + { + var prompt = new SelectionPrompt + { + Title = Components.PickerTitle(title), + PageSize = 12, + MoreChoicesText = $"[{Theme.Muted}]More below[/]", + }; + foreach (var c in choices) prompt.AddChoice(c); + return AnsiConsole.Prompt(prompt); + } + catch (NotSupportedException) { /* fall through */ } + } + + // Numbered list is the only sane fallback: a piped caller cannot press arrow keys. + AnsiConsole.MarkupLine(Markup.Escape(title)); + for (var i = 0; i < choices.Count; i++) + AnsiConsole.MarkupLine($" [{Theme.Brand}]{i + 1}[/] {Markup.Escape(choices[i])}"); + + Components.HintLine("Type a number and press Enter, or press Enter to cancel."); + AnsiConsole.Markup($"[{Theme.Brand}]#[/] "); + + var line = Console.ReadLine(); + if (Console.IsInputRedirected) AnsiConsole.WriteLine(); + + return int.TryParse(line?.Trim(), out var pick) && pick >= 1 && pick <= choices.Count + ? choices[pick - 1] + : null; + } + + /// + /// Reads a secret. Masked when the terminal allows it; when it does not, the value is coming + /// from a pipe and there is nothing on screen to hide. + /// + public static string? Secret(string label) + { + if (CanPrompt) + { + try + { + return AnsiConsole.Prompt( + new TextPrompt(label) + .Secret() + .Validate(k => string.IsNullOrWhiteSpace(k) + ? ValidationResult.Error($"[{Theme.Danger}]Paste a key to continue, or press Ctrl+C to go back.[/]") + : ValidationResult.Success())); + } + catch (NotSupportedException) { /* fall through */ } + } + + AnsiConsole.Markup(Markup.Escape(label)); + var value = Console.ReadLine(); + if (Console.IsInputRedirected) AnsiConsole.WriteLine(); + + return string.IsNullOrWhiteSpace(value) ? null : value.Trim(); + } +} diff --git a/tests/OpenKey.Tests/OpenRouterProviderTests.cs b/tests/OpenKey.Tests/OpenRouterProviderTests.cs index e25ac60..bd7f0fa 100644 --- a/tests/OpenKey.Tests/OpenRouterProviderTests.cs +++ b/tests/OpenKey.Tests/OpenRouterProviderTests.cs @@ -99,6 +99,38 @@ public async Task IgnoresCommentsAndBlankLines() Assert.Equal("x", string.Concat(chunks.Select(c => c.DeltaText))); } + [Fact] + public async Task ValidatingAKeyRejectsA401() + { + // Key validation used to call ListModelsAsync, but GET /models is a *public* endpoint that + // answers 200 with no Authorization header at all — so any string passed validation. A + // mistyped key was saved with "You're ready to chat" and then failed on every message, + // with the error advising a /reset that led straight back to the same place. + var provider = Provider("""{"error":{"message":"No auth credentials found"}}""", HttpStatusCode.Unauthorized); + + var ex = await Assert.ThrowsAsync( + () => provider.ValidateKeyAsync(CancellationToken.None)); + + Assert.Equal(ChatErrorKind.AuthFailure, ex.Kind); + } + + [Fact] + public async Task ValidatingAKeyAcceptsSuccess() + { + var provider = Provider("""{"data":{"label":"test","usage":0}}"""); + + await provider.ValidateKeyAsync(CancellationToken.None); // must not throw + } + + [Fact] + public async Task ValidatingAKeyReportsNetworkFailureSeparately() + { + // A connection problem must not be reported to the user as "your key was refused". + var provider = Provider("captive portal", HttpStatusCode.OK, "text/html"); + + await provider.ValidateKeyAsync(CancellationToken.None); // 200 is 200; parsing is not its job + } + [Fact] public async Task CaptivePortalHtmlDoesNotCrash() {