From c2e3e2838551307c9e5ab6d885c739059ee782f4 Mon Sep 17 00:00:00 2001 From: Esity Date: Wed, 1 Jul 2026 23:55:19 -0500 Subject: [PATCH 1/6] fix(cli): connect status reads Entra TokenManager for microsoft (fixes #213) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `connect status` reported `microsoft: not connected` after a successful Teams/delegated login. The status check used the legacy Legion::Auth::TokenManager, which reads secret keys (microsoft_access_token / microsoft_token_expires_at) that the delegated login never writes — it saves via Entra::Helpers::TokenManager (vault/local/memory). The two stores never intersect, so status always returned false. - status now consults Entra::Helpers::TokenManager.token_data(:delegated) + expired? for the microsoft provider, mirroring the Entra delegated `status` command; other providers keep the legacy path. - fix `connect microsoft` argument forwarding: previously ARGV.select { |a| a.start_with?('--') } dropped flag values and never mapped --scope to teams' --scopes. Now forwards tenant_id/client_id/scope explicitly from parsed options. --- lib/legion/cli/connect_command.rb | 43 ++++++++++++++++----- spec/legion/cli/connect_command_spec.rb | 51 ++++++++++++++++++++++++- 2 files changed, 83 insertions(+), 11 deletions(-) diff --git a/lib/legion/cli/connect_command.rb b/lib/legion/cli/connect_command.rb index 99d9f0db..994d9827 100644 --- a/lib/legion/cli/connect_command.rb +++ b/lib/legion/cli/connect_command.rb @@ -8,6 +8,7 @@ class ConnectCommand < Thor namespace :connect PROVIDERS = %w[microsoft github google].freeze + STATE_COLORS = { 'connected' => :green, 'revoked' => :red, 'not connected' => :yellow }.freeze desc 'microsoft', 'Connect a Microsoft account (OAuth2 delegated auth)' method_option :tenant_id, type: :string, desc: 'Azure tenant ID' @@ -17,7 +18,11 @@ class ConnectCommand < Thor method_option :no_browser, type: :boolean, default: false, desc: 'Print URL instead of launching browser' def microsoft say 'Delegating to Teams OAuth2 browser auth...', :blue - Legion::CLI::Auth.start(['teams'] + ARGV.select { |a| a.start_with?('--') }) + forwarded = ['teams'] + forwarded += ['--tenant_id', options[:tenant_id]] if options[:tenant_id] + forwarded += ['--client_id', options[:client_id]] if options[:client_id] + forwarded += ['--scopes', options[:scope]] if options[:scope] + Legion::CLI::Auth.start(forwarded) end desc 'github', 'Connect a GitHub account (OAuth2 device flow)' @@ -31,14 +36,8 @@ def status require 'legion/auth/token_manager' PROVIDERS.each do |provider| - manager = Legion::Auth::TokenManager.new(provider: provider.to_sym) - if manager.token_valid? - say " #{provider}: connected", :green - elsif manager.revoked? - say " #{provider}: revoked", :red - else - say " #{provider}: not connected", :yellow - end + state = provider_state(provider) + say " #{provider}: #{state}", STATE_COLORS.fetch(state, :yellow) end end @@ -51,6 +50,32 @@ def disconnect(provider) say "Disconnected #{provider} account.", :green end + + no_commands do + # Microsoft delegated login writes tokens via the Entra TokenManager + # (vault/local/memory), not the legacy Legion::Auth secret store — so + # status for :microsoft must consult the Entra store to avoid always + # reporting 'not connected' after a successful Teams/delegated login. + def provider_state(provider) + return microsoft_state if provider == 'microsoft' + + manager = Legion::Auth::TokenManager.new(provider: provider.to_sym) + return 'connected' if manager.token_valid? + return 'revoked' if manager.revoked? + + 'not connected' + end + + def microsoft_state + return 'not connected' unless defined?(Legion::Extensions::Identity::Entra::Helpers::TokenManager) + + tm = Legion::Extensions::Identity::Entra::Helpers::TokenManager + data = tm.token_data(:delegated, refresh: false) + data && !tm.expired?(data) ? 'connected' : 'not connected' + rescue StandardError + 'not connected' + end + end end end end diff --git a/spec/legion/cli/connect_command_spec.rb b/spec/legion/cli/connect_command_spec.rb index 098f627d..0406b6a8 100644 --- a/spec/legion/cli/connect_command_spec.rb +++ b/spec/legion/cli/connect_command_spec.rb @@ -6,11 +6,58 @@ RSpec.describe Legion::CLI::ConnectCommand do describe '#status' do - it 'shows status for all providers' do + before do allow(Legion::Auth::TokenManager).to receive(:new).and_return( instance_double(Legion::Auth::TokenManager, token_valid?: false, revoked?: false) ) - expect { described_class.new.invoke(:status, []) }.to output(/not connected/).to_stdout + end + + context 'when the microsoft delegated token is present via the Entra TokenManager' do + let(:entra_tm) { class_double('Legion::Extensions::Identity::Entra::Helpers::TokenManager') } + + before do + stub_const('Legion::Extensions::Identity::Entra::Helpers::TokenManager', entra_tm) + allow(entra_tm).to receive(:token_data).with(:delegated, refresh: false) + .and_return({ access_token: 'abc', expires_at: Time.now + 3600 }) + allow(entra_tm).to receive(:expired?).and_return(false) + end + + it 'reports microsoft as connected' do + expect { described_class.new.invoke(:status, []) }.to output(/microsoft: connected/).to_stdout + end + end + + context 'when no microsoft delegated token exists' do + before do + stub_const('Legion::Extensions::Identity::Entra::Helpers::TokenManager', + class_double('Legion::Extensions::Identity::Entra::Helpers::TokenManager', + token_data: nil, expired?: true)) + end + + it 'reports microsoft as not connected' do + expect { described_class.new.invoke(:status, []) }.to output(/microsoft: not connected/).to_stdout + end + end + + it 'reports non-microsoft providers via the legacy Auth::TokenManager' do + hide_const('Legion::Extensions::Identity::Entra::Helpers::TokenManager') + expect { described_class.new.invoke(:status, []) }.to output(/github: not connected/).to_stdout + end + end + + describe '#microsoft' do + it 'forwards tenant_id, client_id, and scope (as --scopes) to the teams auth flow' do + expect(Legion::CLI::Auth).to receive(:start).with( + ['teams', '--tenant_id', 'tid', '--client_id', 'cid', '--scopes', 'Calendars.Read'] + ) + cmd = described_class.new([], { tenant_id: 'tid', client_id: 'cid', scope: 'Calendars.Read' }) + cmd.microsoft + end + + it 'forwards only teams when no options are given' do + expect(Legion::CLI::Auth).to receive(:start).with(['teams']) + cmd = described_class.new([], {}) + cmd.microsoft end end From f63e998cc9882765eef7023d9b04bf10fcd09792 Mon Sep 17 00:00:00 2001 From: Esity Date: Wed, 1 Jul 2026 23:57:36 -0500 Subject: [PATCH 2/6] chore(release): bump to 1.9.44 for connect status fix (#213) --- CHANGELOG.md | 6 ++++++ lib/legion/version.rb | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 275a0d56..98698b39 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Legion Changelog +## [1.9.44] - 2026-07-01 + +### Fixed +- CLI: `connect status` now reads the Entra `TokenManager` (`token_data(:delegated)` + `expired?`) for the `microsoft` provider instead of the legacy `Legion::Auth::TokenManager` secret store, which the delegated/Teams login never writes to — previously always reported `microsoft: not connected` after a successful login (fixes #213) +- CLI: `connect microsoft` now forwards `--tenant_id`/`--client_id`/`--scope` (as `--scopes`) explicitly to the Teams auth flow instead of dropping flag values via `ARGV.select` + ## [1.9.43] - 2026-06-25 ### Fixed diff --git a/lib/legion/version.rb b/lib/legion/version.rb index 969995f6..b6f84adf 100644 --- a/lib/legion/version.rb +++ b/lib/legion/version.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true module Legion - VERSION = '1.9.43' + VERSION = '1.9.44' end From 609be0efe730c85022a91055629d6341e6320a75 Mon Sep 17 00:00:00 2001 From: Esity Date: Thu, 2 Jul 2026 00:02:41 -0500 Subject: [PATCH 3/6] fix(api): forward tenant_id/client_id in Teams OAuth callback token store (fixes #212) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Teams delegated OAuth callback persisted tokens via Entra::Helpers::TokenManager.save_token(:delegated, …) but omitted tenant_id/client_id. TokenManager#refresh_token requires both from the stored token data; without them, refresh falls back to settings_auth (empty for delegated-only logins) and silently fails. The callback's `entry` hash already carries both — now forwarded through store_teams_token. The original #212 500 (phantom `require` of unshipped TokenCache raising an uncatchable LoadError) was already resolved in 1.9.43 via #229; this completes the remaining piece of #212's suggested fix. Bumped version to 1.9.45 and updated CHANGELOG. --- CHANGELOG.md | 5 +++ lib/legion/api/auth_teams.rb | 11 ++++--- lib/legion/version.rb | 2 +- spec/legion/api/auth_teams_spec.rb | 52 ++++++++++++++++++++++++++++++ 4 files changed, 65 insertions(+), 5 deletions(-) create mode 100644 spec/legion/api/auth_teams_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 98698b39..b404cbc3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Legion Changelog +## [1.9.45] - 2026-07-02 + +### Fixed +- API: Teams delegated OAuth callback now forwards `tenant_id`/`client_id` to `Entra::Helpers::TokenManager.save_token(:delegated, …)` so browser-login tokens can be refreshed (previously omitted, forcing refresh to fall back to settings and silently fail for delegated-only logins). The unshipped `TokenCache` `require` that 500'd the callback was already removed in 1.9.43 (fixes #212) + ## [1.9.44] - 2026-07-01 ### Fixed diff --git a/lib/legion/api/auth_teams.rb b/lib/legion/api/auth_teams.rb index c8c83d55..b6d89bc0 100644 --- a/lib/legion/api/auth_teams.rb +++ b/lib/legion/api/auth_teams.rb @@ -107,8 +107,9 @@ def self.register_callback(app) token_body = Legion::JSON.load(token_response.body) if token_body[:access_token] - # Store token via TokenCache if available - store_teams_token(token_body, entry[:scopes]) + # Persist via the Entra TokenManager (the store the read path consults) + store_teams_token(token_body, entry[:scopes], + tenant_id: entry[:tenant_id], client_id: entry[:client_id]) AuthTeams.mutex.synchronize { entry[:result] = { authenticated: true } } content_type :html '

Authentication successful!

You can close this tab.

' @@ -128,14 +129,16 @@ def self.register_callback(app) end module TeamsTokenHelper - def store_teams_token(token_body, scopes) + def store_teams_token(token_body, scopes, tenant_id: nil, client_id: nil) require 'legion/extensions/identity/entra/helpers/token_manager' Legion::Extensions::Identity::Entra::Helpers::TokenManager.save_token( :delegated, access_token: token_body[:access_token], refresh_token: token_body[:refresh_token], expires_in: token_body[:expires_in] || 3600, - scopes: scopes + scopes: scopes, + tenant_id: tenant_id, + client_id: client_id ) Legion::Logging.info 'Teams delegated token stored' if defined?(Legion::Logging) rescue StandardError => e diff --git a/lib/legion/version.rb b/lib/legion/version.rb index b6f84adf..9f68bbc0 100644 --- a/lib/legion/version.rb +++ b/lib/legion/version.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true module Legion - VERSION = '1.9.44' + VERSION = '1.9.45' end diff --git a/spec/legion/api/auth_teams_spec.rb b/spec/legion/api/auth_teams_spec.rb new file mode 100644 index 00000000..c6dc6e2c --- /dev/null +++ b/spec/legion/api/auth_teams_spec.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +require 'spec_helper' +require 'legion/api/auth_teams' + +RSpec.describe Legion::API::Routes::AuthTeams::TeamsTokenHelper do + subject(:helper) { Object.new.extend(described_class) } + + let(:token_manager) do + class_double('Legion::Extensions::Identity::Entra::Helpers::TokenManager', save_token: true) + end + + let(:token_body) do + { access_token: 'at', refresh_token: 'rt', expires_in: 3600 } + end + + before do + stub_const('Legion::Extensions::Identity::Entra::Helpers::TokenManager', token_manager) + allow(helper).to receive(:require).and_return(true) + end + + describe '#store_teams_token' do + it 'persists the delegated token via the Entra TokenManager' do + helper.store_teams_token(token_body, 'OnlineMeetings.Read', + tenant_id: 'tid', client_id: 'cid') + + expect(token_manager).to have_received(:save_token).with( + :delegated, + access_token: 'at', + refresh_token: 'rt', + expires_in: 3600, + scopes: 'OnlineMeetings.Read', + tenant_id: 'tid', + client_id: 'cid' + ) + end + + it 'forwards tenant_id and client_id so the stored token can be refreshed' do + helper.store_teams_token(token_body, 'scope', tenant_id: 'tid', client_id: 'cid') + + expect(token_manager).to have_received(:save_token) + .with(:delegated, hash_including(tenant_id: 'tid', client_id: 'cid')) + end + + it 'does not raise when the token store fails (logs a warning instead)' do + allow(token_manager).to receive(:save_token).and_raise(StandardError, 'vault down') + + expect { helper.store_teams_token(token_body, 'scope', tenant_id: 't', client_id: 'c') } + .not_to raise_error + end + end +end From 4b8895873dde084d84a88522c8046740931685b5 Mon Sep 17 00:00:00 2001 From: Esity Date: Thu, 2 Jul 2026 00:10:27 -0500 Subject: [PATCH 4/6] fix(api): call extensions/tools helpers on correct receiver (fixes #227) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GET /api/extensions/tools returned HTTP 500 with `undefined method 'filter_tool_entries'`. The route block executes in the Sinatra instance context, but filter_tool_entries/serialize_tool_entry are class methods on Routes::Extensions (inside `class << self`), so bare calls resolved against the wrong receiver and raised NoMethodError. Call them on the explicit Routes::Extensions receiver, matching the pattern already used by the other catalog routes (e.g. Routes::Extensions.extension_entries). Added request specs for /api/extensions/tools (200 + extension/deferred filters) — the route had no coverage, which is how the regression slipped in. Bumped version to 1.9.46 and updated CHANGELOG. --- CHANGELOG.md | 5 +++++ lib/legion/api/extensions.rb | 4 ++-- lib/legion/version.rb | 2 +- spec/legion/api/extensions_spec.rb | 35 ++++++++++++++++++++++++++++++ 4 files changed, 43 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b404cbc3..1c9467fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Legion Changelog +## [1.9.46] - 2026-07-02 + +### Fixed +- API: `GET /api/extensions/tools` no longer 500s with `undefined method 'filter_tool_entries'`. The route block runs in the Sinatra instance context but `filter_tool_entries`/`serialize_tool_entry` are class methods on `Routes::Extensions`; they are now called on the explicit receiver, matching the pattern used by the other catalog routes. Added request specs for `/api/extensions/tools` (fixes #227) + ## [1.9.45] - 2026-07-02 ### Fixed diff --git a/lib/legion/api/extensions.rb b/lib/legion/api/extensions.rb index 0d59b5e9..f29b251f 100644 --- a/lib/legion/api/extensions.rb +++ b/lib/legion/api/extensions.rb @@ -32,8 +32,8 @@ def self.register_loaded_summary_route(app) def self.register_tools_route(app) app.get '/api/extensions/tools' do - entries = filter_tool_entries(Array(Legion::Settings::Extensions.tools), params) - tools = entries.map { |e| serialize_tool_entry(e) } + entries = Routes::Extensions.filter_tool_entries(Array(Legion::Settings::Extensions.tools), params) + tools = entries.map { |e| Routes::Extensions.serialize_tool_entry(e) } json_response({ total: tools.size, tools: tools }) end end diff --git a/lib/legion/version.rb b/lib/legion/version.rb index 9f68bbc0..a9225af4 100644 --- a/lib/legion/version.rb +++ b/lib/legion/version.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true module Legion - VERSION = '1.9.45' + VERSION = '1.9.46' end diff --git a/spec/legion/api/extensions_spec.rb b/spec/legion/api/extensions_spec.rb index 5a59f879..dcecb36e 100644 --- a/spec/legion/api/extensions_spec.rb +++ b/spec/legion/api/extensions_spec.rb @@ -137,6 +137,41 @@ def app end end + describe 'GET /api/extensions/tools' do + let(:tool_entries) do + [ + { name: 'get_key', description: 'Get a key', extension: 'redis', runner: 'keys', + deferred: false, trigger_words: %w[redis key], source: 'lex', sticky: false }, + { name: 'run_query', description: 'Run a query', extension: 'data', runner: 'sql', + deferred: true, trigger_words: [], source: 'lex', sticky: false } + ] + end + + before do + allow(Legion::Settings::Extensions).to receive(:tools).and_return(tool_entries) + end + + it 'returns 200 with serialized tools (regression: helpers are class methods, not instance)' do + get '/api/extensions/tools' + expect(last_response.status).to eq(200) + body = Legion::JSON.load(last_response.body) + expect(body[:data][:total]).to eq(2) + expect(body[:data][:tools].map { |t| t[:name] }).to contain_exactly('get_key', 'run_query') + end + + it 'filters by ?extension= param' do + get '/api/extensions/tools?extension=redis' + body = Legion::JSON.load(last_response.body) + expect(body[:data][:tools].map { |t| t[:name] }).to contain_exactly('get_key') + end + + it 'filters by ?deferred= param' do + get '/api/extensions/tools?deferred=true' + body = Legion::JSON.load(last_response.body) + expect(body[:data][:tools].map { |t| t[:name] }).to contain_exactly('run_query') + end + end + describe 'GET /api/extension_catalog/available' do it 'returns the full ecosystem list' do get '/api/extension_catalog/available' From b1b05bda30b96798b754eda1b71f3a7a120194de Mon Sep 17 00:00:00 2001 From: Esity Date: Thu, 2 Jul 2026 00:20:57 -0500 Subject: [PATCH 5/6] fix(api): /api/health reflects subsystem health, 503 when degraded (fixes #194) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit /api/health returned 200 ok unconditionally, so load balancers and monitors kept routing to instances whose transport session was broken (session_open: false after a recovery failure). Added Legion::API::Health which assesses transport/cache/data and returns `degraded` + HTTP 503 when an enabled, previously-healthy subsystem has broken. The "enabled AND previously-healthy" gate maps to Legion::Readiness.status[c] == true: Service marks a component ready only when its config flag is on and setup succeeded, so disabled (:skipped) and still-booting (nil/false) subsystems never fail the check — only a subsystem that came up and later degraded does. Live liveness reuses the same checks as /api/stats (session.open?/lite mode, Cache.connected?, data connected). Response now includes a per-component `components` breakdown (enabled/healthy/detail). OpenAPI updated to document the 503 + components. Bumped version to 1.9.47 and updated CHANGELOG. --- CHANGELOG.md | 5 ++ lib/legion/api.rb | 7 ++- lib/legion/api/health.rb | 73 +++++++++++++++++++++++++++ lib/legion/api/openapi.rb | 10 ++-- lib/legion/version.rb | 2 +- spec/api/health_spec.rb | 18 ++++++- spec/legion/api/health_spec.rb | 91 ++++++++++++++++++++++++++++++++++ 7 files changed, 200 insertions(+), 6 deletions(-) create mode 100644 lib/legion/api/health.rb create mode 100644 spec/legion/api/health_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 1c9467fb..d4172260 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Legion Changelog +## [1.9.47] - 2026-07-02 + +### Fixed +- API: `GET /api/health` now reflects subsystem health instead of always returning `200 ok`. It reports `degraded` with HTTP `503` when an **enabled and previously-healthy** subsystem (transport, cache, data) has broken — e.g. transport `session_open: false` after a connection recovery failure — and includes a per-component `components` breakdown (`enabled`/`healthy`/`detail`). A subsystem only degrades health if it was marked ready at boot (via `Legion::Readiness`); disabled or still-booting subsystems never fail the check. This makes the endpoint usable for load balancers and monitoring (fixes #194) + ## [1.9.46] - 2026-07-02 ### Fixed diff --git a/lib/legion/api.rb b/lib/legion/api.rb index 62c54fae..3ba368bc 100644 --- a/lib/legion/api.rb +++ b/lib/legion/api.rb @@ -5,6 +5,7 @@ require_relative 'events' require_relative 'readiness' require_relative 'api/default_settings' +require_relative 'api/health' require_relative 'api/middleware/auth' require_relative 'api/middleware/body_limit' @@ -107,7 +108,11 @@ class API < Sinatra::Base # Health and readiness get '/api/health' do uptime_seconds = (::Process.clock_gettime(::Process::CLOCK_MONOTONIC) - START_TIME).to_i - json_response({ status: 'ok', version: Legion::VERSION, uptime_seconds: uptime_seconds, uptime: uptime_seconds }) + assessment = Legion::API::Health.assess + json_response({ status: assessment[:status], version: Legion::VERSION, + uptime_seconds: uptime_seconds, uptime: uptime_seconds, + components: assessment[:components] }, + status_code: assessment[:status] == 'ok' ? 200 : 503) end get '/api/ready' do diff --git a/lib/legion/api/health.rb b/lib/legion/api/health.rb new file mode 100644 index 00000000..51eae6b1 --- /dev/null +++ b/lib/legion/api/health.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: true + +module Legion + class API < Sinatra::Base + # Subsystem health assessment for GET /api/health. + # + # A component degrades health ONLY if it is both: + # 1. enabled — Legion::Readiness.status[c] == true (Service marks a + # component ready only when its config flag is on AND setup succeeded). + # :skipped (disabled) and false/nil (never booted / still booting) are + # NOT enabled-and-previously-healthy, so they never fail health. + # 2. currently unhealthy — its live liveness check now returns false. + # + # This prevents both false negatives (200 while transport can't process + # messages) and false positives (503 for a subsystem that was never + # configured or hasn't finished booting). + module Health + COMPONENTS = %i[transport cache data].freeze + + class << self + # Returns { status:, components: { name => { enabled:, healthy:, detail? } } } + def assess + components = COMPONENTS.to_h { |name| [name, component_health(name)] } + degraded = components.any? { |_name, c| c[:enabled] && c[:healthy] == false } + { status: degraded ? 'degraded' : 'ok', components: components } + end + + def component_health(name) + enabled = Legion::Readiness.status[name] == true + return { enabled: false, healthy: nil } unless enabled + + healthy, detail = send("#{name}_liveness") + info = { enabled: true, healthy: healthy } + info[:detail] = detail if detail && !healthy + info + end + + # --- liveness checks: [healthy_boolean, detail_string_or_nil] --- + + def transport_liveness + conn = Legion::Transport::Connection + return [true, nil] if conn.respond_to?(:lite_mode?) && conn.lite_mode? + + session = conn.session + open = session.respond_to?(:open?) && session.open? + [open, open ? nil : 'session_open: false'] + rescue StandardError => e + [false, e.message] + end + + def cache_liveness + return [true, nil] unless defined?(Legion::Cache) + + connected = Legion::Cache.connected? + [connected, connected ? nil : 'cache not connected'] + rescue StandardError => e + [false, e.message] + end + + def data_liveness + connected = begin + Legion::Settings[:data][:connected] == true + rescue StandardError + false + end + [connected, connected ? nil : 'data not connected'] + rescue StandardError => e + [false, e.message] + end + end + end + end +end diff --git a/lib/legion/api/openapi.rb b/lib/legion/api/openapi.rb index a8b5ae1f..f04d4d90 100644 --- a/lib/legion/api/openapi.rb +++ b/lib/legion/api/openapi.rb @@ -432,7 +432,9 @@ def self.health_paths get: { tags: ['Health'], summary: 'Health check', - description: 'Returns ok status and version. Skips auth middleware.', + description: 'Returns ok/degraded status, version, and per-subsystem component health. ' \ + 'Returns 503 when an enabled, previously-healthy subsystem (transport, cache, data) ' \ + 'has degraded. Skips auth middleware.', operationId: 'getHealth', security: [], responses: { @@ -440,11 +442,13 @@ def self.health_paths properties: { data: { type: 'object', - properties: { status: { type: 'string', example: 'ok' }, version: { type: 'string' } } + properties: { status: { type: 'string', example: 'ok' }, version: { type: 'string' }, + components: { type: 'object' } } }, meta: { '$ref' => '#/components/schemas/Meta' } } - )) + )), + '503' => { description: 'Degraded — an enabled subsystem has broken' } } } }, diff --git a/lib/legion/version.rb b/lib/legion/version.rb index a9225af4..283eb397 100644 --- a/lib/legion/version.rb +++ b/lib/legion/version.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true module Legion - VERSION = '1.9.46' + VERSION = '1.9.47' end diff --git a/spec/api/health_spec.rb b/spec/api/health_spec.rb index 3561e61d..cade21a4 100644 --- a/spec/api/health_spec.rb +++ b/spec/api/health_spec.rb @@ -12,7 +12,10 @@ def app before(:all) { ApiSpecSetup.configure_settings } describe 'GET /api/health' do - it 'returns ok status' do + after { Legion::Readiness.reset } + + it 'returns ok status when no enabled subsystem is degraded' do + Legion::Readiness.reset get '/api/health' expect(last_response.status).to eq(200) body = Legion::JSON.load(last_response.body) @@ -20,6 +23,19 @@ def app expect(body[:data][:version]).to eq(Legion::VERSION) expect(body[:data][:uptime_seconds]).to be_an(Integer) expect(body[:data][:uptime]).to eq(body[:data][:uptime_seconds]) + expect(body[:data][:components]).to have_key(:transport) + end + + it 'returns 503 degraded when an enabled subsystem has broken' do + Legion::Readiness.reset + Legion::Readiness.mark_ready(:transport) + allow(Legion::API::Health).to receive(:transport_liveness).and_return([false, 'session_open: false']) + + get '/api/health' + expect(last_response.status).to eq(503) + body = Legion::JSON.load(last_response.body) + expect(body[:data][:status]).to eq('degraded') + expect(body[:data][:components][:transport]).to include(enabled: true, healthy: false) end end diff --git a/spec/legion/api/health_spec.rb b/spec/legion/api/health_spec.rb new file mode 100644 index 00000000..d649de0f --- /dev/null +++ b/spec/legion/api/health_spec.rb @@ -0,0 +1,91 @@ +# frozen_string_literal: true + +require 'spec_helper' +require 'legion/api/health' + +RSpec.describe Legion::API::Health do + before { Legion::Readiness.reset } + after { Legion::Readiness.reset } + + describe '.assess' do + context 'when an enabled, previously-healthy component has degraded' do + before do + Legion::Readiness.mark_ready(:transport) + Legion::Readiness.mark_skipped(:cache) + Legion::Readiness.mark_skipped(:data) + allow(described_class).to receive(:transport_liveness).and_return([false, 'session_open: false']) + end + + it 'reports degraded' do + result = described_class.assess + expect(result[:status]).to eq('degraded') + expect(result[:components][:transport]).to eq(enabled: true, healthy: false, detail: 'session_open: false') + end + end + + context 'when all enabled components are healthy' do + before do + Legion::Readiness.mark_ready(:transport) + Legion::Readiness.mark_ready(:cache) + Legion::Readiness.mark_skipped(:data) + allow(described_class).to receive(:transport_liveness).and_return([true, nil]) + allow(described_class).to receive(:cache_liveness).and_return([true, nil]) + end + + it 'reports ok' do + result = described_class.assess + expect(result[:status]).to eq('ok') + expect(result[:components][:cache]).to eq(enabled: true, healthy: true) + end + end + + context 'when a component is skipped (disabled)' do + before do + Legion::Readiness.mark_ready(:transport) + Legion::Readiness.mark_skipped(:cache) + Legion::Readiness.mark_skipped(:data) + allow(described_class).to receive(:transport_liveness).and_return([true, nil]) + end + + it 'does not degrade health for the disabled component' do + result = described_class.assess + expect(result[:status]).to eq('ok') + expect(result[:components][:data]).to eq(enabled: false, healthy: nil) + end + + it 'does not run the liveness check for a disabled component' do + expect(described_class).not_to receive(:cache_liveness) + described_class.assess + end + end + + context 'when a component has not finished booting (never marked ready)' do + before do + Legion::Readiness.mark_ready(:transport) + allow(described_class).to receive(:transport_liveness).and_return([true, nil]) + # cache/data left unset (nil) — still booting, not enabled-and-healthy + end + + it 'does not degrade health for a still-booting component' do + result = described_class.assess + expect(result[:status]).to eq('ok') + expect(result[:components][:cache]).to eq(enabled: false, healthy: nil) + end + end + end + + describe '.transport_liveness' do + it 'returns healthy in lite mode without checking the session' do + conn = class_double('Legion::Transport::Connection', lite_mode?: true) + stub_const('Legion::Transport::Connection', conn) + expect(described_class.transport_liveness).to eq([true, nil]) + end + + it 'returns unhealthy when the session is not open' do + session = instance_double('session', open?: false) + conn = class_double('Legion::Transport::Connection', lite_mode?: false, session: session) + stub_const('Legion::Transport::Connection', conn) + expect(described_class.transport_liveness).to eq([false, 'session_open: false']) + end + end +end From adf6aaba9ea004f5cd9bb46c9c6284e14afcc142 Mon Sep 17 00:00:00 2001 From: Esity Date: Thu, 2 Jul 2026 00:28:55 -0500 Subject: [PATCH 6/6] fix(api): segment-bounded skip_path? to close auth bypass (fixes #209) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The auth middleware's skip_path? used bare prefix matching (start_with?), so any path sharing a prefix with an unauthenticated route bypassed auth — e.g. /api/healthily_fake, /api/health/admin/export, /api/auth/tokens_dump all matched /api/health or /api/auth/token (CWE-284, Improper Access Control). Replaced with segment-bounded matching via precompiled SKIP_PATTERNS: each skip path matches only exactly or as a `/`-delimited sub-path. Legit sub-paths like /api/health/live still skip; prefix-collision paths now require auth (401). Bumped version to 1.9.48 and updated CHANGELOG. --- CHANGELOG.md | 5 +++++ lib/legion/api/middleware/auth.rb | 5 ++++- lib/legion/version.rb | 2 +- spec/legion/api/middleware/auth_spec.rb | 25 +++++++++++++++++++++++++ 4 files changed, 35 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d4172260..651a33f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Legion Changelog +## [1.9.48] - 2026-07-02 + +### Security +- API: fixed an authentication bypass in the auth middleware's `skip_path?` (CWE-284). It used bare prefix matching (`start_with?`), so any path sharing a prefix with an unauthenticated route bypassed auth — e.g. `/api/healthily_fake`, `/api/health/admin/export`, `/api/auth/tokens_dump` all matched `/api/health` / `/api/auth/token`. Now uses segment-bounded matching (exact path or `/`-delimited sub-path) via precompiled `SKIP_PATTERNS`, so only the intended paths and their legitimate sub-paths skip auth (fixes #209) + ## [1.9.47] - 2026-07-02 ### Fixed diff --git a/lib/legion/api/middleware/auth.rb b/lib/legion/api/middleware/auth.rb index 6f19b64c..4dd44548 100644 --- a/lib/legion/api/middleware/auth.rb +++ b/lib/legion/api/middleware/auth.rb @@ -6,6 +6,9 @@ module Middleware class Auth SKIP_PATHS = %w[/api/health /api/ready /api/openapi.json /metrics /api/auth/token /api/auth/worker-token /api/auth/authorize /api/auth/callback /api/auth/negotiate].freeze + # Match a skip path exactly or as a bounded sub-path (segment boundary), + # NOT as a bare prefix — `/api/healthily_fake` must not match `/api/health`. + SKIP_PATTERNS = SKIP_PATHS.map { |p| %r{\A#{Regexp.escape(p)}(?:/|\z)} }.freeze AUTH_HEADER = 'HTTP_AUTHORIZATION' BEARER_PATTERN = /\ABearer\s+(.+)\z/i NEGOTIATE_PATTERN = /\ANegotiate\s+(.+)\z/i @@ -81,7 +84,7 @@ def try_negotiate(env) end def skip_path?(path) - SKIP_PATHS.any? { |p| path.start_with?(p) } + SKIP_PATTERNS.any? { |re| re.match?(path) } end def extract_negotiate_token(env) diff --git a/lib/legion/version.rb b/lib/legion/version.rb index 283eb397..9dc8d70e 100644 --- a/lib/legion/version.rb +++ b/lib/legion/version.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true module Legion - VERSION = '1.9.47' + VERSION = '1.9.48' end diff --git a/spec/legion/api/middleware/auth_spec.rb b/spec/legion/api/middleware/auth_spec.rb index 08b8545f..40ae73a5 100644 --- a/spec/legion/api/middleware/auth_spec.rb +++ b/spec/legion/api/middleware/auth_spec.rb @@ -53,6 +53,31 @@ status, = app.call(Rack::MockRequest.env_for('/api/auth/token')) expect(status).to eq(200) end + + it 'skips a legitimate bounded sub-path (e.g. /api/health/live)' do + status, = app.call(Rack::MockRequest.env_for('/api/health/live')) + expect(status).to eq(200) + end + end + + describe 'skip path prefix-bypass hardening (CWE-284)' do + # Prefix matching would have let these bypass auth; segment-bounded + # matching must require auth (401) for paths that merely share a prefix. + bypass_paths = %w[ + /api/healthily_fake + /api/health_admin_backdoor + /api/ready_to_explode + /api/auth/tokens_dump + /api/auth/authorized_mimic + /metrics_debug + ] + + bypass_paths.each do |path| + it "requires auth for #{path} (does not treat it as a skip path)" do + status, = app.call(Rack::MockRequest.env_for(path)) + expect(status).to eq(401) + end + end end describe 'missing auth' do