diff --git a/CHANGELOG.md b/CHANGELOG.md index a0ea767..a264972 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,10 @@ This file tracks the major changes in each release. - Added a security policy and threat model covering safe deployment defaults, cross-tenant data exposure, schema-context filtering, observability data exposure, and vulnerability reporting. ### Changed +- Security compatibility: when a `policy_adapter` is configured, executable `Query` objects must come from `CodeToQuery.ask` (or the compiler path) with the compiler's opaque policy contract. Directly constructed queries have no valid contract and now fail closed. +- `Query#sql`, `#params`, `#intent`, and `#metrics` now return detached deep copies. Mutating a returned value no longer changes query state; repeated reads may allocate new objects. +- Compiled policy queries are bound to the exact policy adapter and configuration identity used at compilation. Replacing the adapter or relevant configuration invalidates existing queries; compile them again before execution. +- Compatibility note: policy adapters that explicitly return `allowed_tables: []` deny access to all tables, as intended; omit `allowed_tables` only when no policy table allowlist is being supplied. - Added explicit explain-gate profiles and deployment tradeoff guidance for `explain_fail_open` in the README, including strict, availability-first, and local/test configuration patterns. - Breaking default change: EXPLAIN gate errors now fail closed by default; set `explain_fail_open = true` only for availability-first readonly deployments that can tolerate skipped EXPLAIN checks. - Context pack generation now omits sensitive schema metadata by default for password, secret, token, credential, digest, salt, OTP, and API key style columns; customize `sensitive_column_patterns` when applications use additional naming conventions. diff --git a/docs/pundit-policy-adapter.md b/docs/pundit-policy-adapter.md index 2a34db3..8e078a9 100644 --- a/docs/pundit-policy-adapter.md +++ b/docs/pundit-policy-adapter.md @@ -27,6 +27,12 @@ Keep these boundaries explicit: - **Pundit remains authoritative.** Use Pundit to decide whether the user can access the reporting surface. Do not use natural-language prompts as authorization input. - **Keep logs narrow.** Prefer logging table names, policy key names, and decisions. Avoid logging raw prompts, row data, full bind values, credentials, or full generated schema context in normal application logs. +### Query lifecycle and compatibility + +- With a `policy_adapter` configured, obtain executable queries through `CodeToQuery.ask` or the compiler path. Policy compilation issues an opaque contract that callers cannot construct; a directly instantiated `CodeToQuery::Query` lacks that evidence and fails closed at safety checks and execution. +- A compiled query's policy contract is bound to the exact policy adapter object and configuration instance used to compile it, including the captured database adapter and policy fail-open setting. Replacing the policy adapter or changing that relevant configuration invalidates the query. Recompile instead of retaining policy-bearing queries across configuration changes or application reloads. +- The public `Query#sql`, `#params`, `#intent`, and `#metrics` readers return detached deep copies. Mutating those values does not alter the immutable state used for safety checks or execution, and repeated reads may allocate new objects. + ## Lambda adapter for tenant, account, and user predicates This example keeps table and column exposure small and derives row predicates from trusted application state. It raises on missing users, unknown tables, and denied policies so CodeToQuery fails closed. Define the custom Pundit predicates such as `code_to_query?`, `view_all_tickets?`, and `view_account_users?` on the relevant policies before using this shape. diff --git a/lib/code_to_query.rb b/lib/code_to_query.rb index c3b6953..82653cf 100644 --- a/lib/code_to_query.rb +++ b/lib/code_to_query.rb @@ -12,6 +12,8 @@ require_relative 'code_to_query/configuration' require_relative 'code_to_query/errors' require_relative 'code_to_query/instrumentation' +require_relative 'code_to_query/identifier_semantics' +require_relative 'code_to_query/policy_adapter_invoker' require_relative 'code_to_query/providers/base' require_relative 'code_to_query/providers/openai' require_relative 'code_to_query/providers/local' @@ -110,7 +112,9 @@ def self.ask(prompt:, schema: nil, allow_tables: nil, current_user: nil) policy_applied: false } compiled = Instrumentation.instrument(:compile, compile_payload) do - compile_result = Compiler.new(config).compile(validated_intent, current_user: current_user) + compile_result = Compiler.new(config).compile( + validated_intent, current_user: current_user, allow_tables: allow_tables + ) compile_payload[:policy_applied] = policy_applied_from_bind_spec?(compile_result[:bind_spec]) compile_result end @@ -124,9 +128,13 @@ def self.ask(prompt:, schema: nil, allow_tables: nil, current_user: nil) row_limit: validated_intent['limit'], policy_applied: policy_applied } + query = Query.new(sql: compiled[:sql], params: compiled[:params], bind_spec: compiled[:bind_spec], + intent: compiled[:intent] || validated_intent, allow_tables: allow_tables, config: config, + policy_contract: compiled[:policy_contract]) + begin Instrumentation.instrument(:lint, **lint_payload) do - Guardrails::SqlLinter.new(config, allow_tables: allow_tables).check!(compiled[:sql]) + query.send(:lint_sql!) end rescue SecurityError => e Instrumentation.instrument( @@ -141,8 +149,7 @@ def self.ask(prompt:, schema: nil, allow_tables: nil, current_user: nil) raise end - Query.new(sql: compiled[:sql], params: compiled[:params], bind_spec: compiled[:bind_spec], - intent: validated_intent, allow_tables: allow_tables, config: config) + query end def self.query_shape(intent) diff --git a/lib/code_to_query/compiler.rb b/lib/code_to_query/compiler.rb index 0c04985..3afcdc7 100644 --- a/lib/code_to_query/compiler.rb +++ b/lib/code_to_query/compiler.rb @@ -11,16 +11,92 @@ module CodeToQuery # rubocop:disable Metrics/ClassLength class Compiler + # Policy evidence is an opaque capability. Its constructor and immutable + # snapshot are private, so caller-supplied metadata can never stand in for + # a successful policy compilation. The issuing adapter is retained by + # reference so its identity cannot be recycled while the capability lives. + class PolicyContract + def initialize(snapshot, policy_adapter) + @snapshot = snapshot + @policy_adapter = policy_adapter + freeze + end + + attr_reader :snapshot, :policy_adapter + private :snapshot, :policy_adapter + private_class_method :new + end + private_constant :PolicyContract + def initialize(config) @config = config end - def compile(intent, current_user: nil) - intent_with_policy = apply_policy_predicates(intent, current_user) - if use_arel? - compile_with_arel(intent_with_policy, current_user) - else - compile_with_string_building(intent_with_policy, current_user) + def compile(intent, current_user: nil, allow_tables: nil) + working_intent = deep_dup_value(intent) + strip_untrusted_policy_expectations!(working_intent) + intent_with_policy = apply_policy_predicates(working_intent, current_user) + result = if use_arel? + compile_with_arel(intent_with_policy, current_user) + else + compile_with_string_building(intent_with_policy, current_user) + end + verify_compiled_policy_binds!(result) + result[:policy_contract] = build_policy_contract(result, allow_tables) if @config.policy_adapter.respond_to?(:call) + result + end + + class << self + private + + def valid_policy_contract?(contract, sql:, params:, bind_spec:, intent:, allow_tables:, config:) + return false unless contract.instance_of?(PolicyContract) + return false unless contract.send(:policy_adapter).equal?(config.policy_adapter) + + contract.send(:snapshot) == policy_contract_snapshot( + sql: sql, params: params, bind_spec: bind_spec, intent: intent, + allow_tables: allow_tables, config: config + ) + end + + def issue_policy_contract(result, allow_tables, config) + policy_adapter = config.policy_adapter + snapshot = policy_contract_snapshot( + sql: result[:sql], params: result[:params], bind_spec: result[:bind_spec], + intent: result[:intent], allow_tables: allow_tables, config: config + ) + PolicyContract.send(:new, snapshot, policy_adapter) + end + + def policy_contract_snapshot(sql:, params:, bind_spec:, intent:, allow_tables:, config:) + deep_freeze_contract_value( + sql: sql, + params: params, + bind_spec: bind_spec, + intent: intent, + allow_tables: allow_tables, + config_identity: config.__id__, + adapter: config.adapter, + policy_adapter_fail_open: config.respond_to?(:policy_adapter_fail_open) && config.policy_adapter_fail_open + ) + end + + def deep_freeze_contract_value(value) + copy = case value + when Hash + value.each_with_object({}) do |(key, item), result| + result[deep_freeze_contract_value(key)] = deep_freeze_contract_value(item) + end + when Array + value.map { |item| deep_freeze_contract_value(item) } + when Symbol, Numeric, true, false, nil + value + else + value.dup + end + copy.freeze + rescue TypeError + value end end @@ -31,11 +107,15 @@ def apply_policy_predicates(intent, current_user) table = intent['table'] policy_info = safely_fetch_policy(table: table, current_user: current_user, intent: intent) + merge_policy_allowed_tables!(intent, policy_info) policy_hash = extract_enforced_predicates(policy_info) return intent if policy_hash.empty? + policy_keys = [] filters = Array(intent['filters']) + policy_hash.map do |column, value| if value.is_a?(Range) && value.begin && value.end + policy_keys << "policy_#{column}_start" + policy_keys << "policy_#{column}_end" { 'column' => column.to_s, 'op' => 'between', @@ -43,6 +123,7 @@ def apply_policy_predicates(intent, current_user) 'param_end' => "policy_#{column}_end" } else + policy_keys << "policy_#{column}" { 'column' => column.to_s, 'op' => '=', @@ -63,7 +144,8 @@ def apply_policy_predicates(intent, current_user) intent.merge( 'filters' => filters, - 'params' => params + 'params' => params, + '__policy_expected_keys' => merge_policy_expected_keys(intent, policy_keys) ) rescue PolicyAdapterError raise @@ -75,28 +157,7 @@ def apply_policy_predicates(intent, current_user) end def safely_fetch_policy(table:, current_user:, intent: nil) - if intent - @config.policy_adapter.call(current_user, table: table, intent: intent) - else - @config.policy_adapter.call(current_user, table: table) - end - rescue ArgumentError - # Backward compatibility: adapters may accept user plus table or only user. - begin - @config.policy_adapter.call(current_user, table: table) - rescue ArgumentError - begin - @config.policy_adapter.call(current_user) - rescue StandardError => e - return handle_policy_failure("Policy adapter failed: #{e.message}") if policy_adapter_fail_open? - - raise policy_failure("Policy adapter failed: #{e.message}") - end - rescue StandardError => e - return handle_policy_failure("Policy adapter failed: #{e.message}") if policy_adapter_fail_open? - - raise policy_failure("Policy adapter failed: #{e.message}") - end + PolicyAdapterInvoker.call(@config.policy_adapter, current_user, table: table, intent: intent) rescue StandardError => e return handle_policy_failure("Policy adapter failed: #{e.message}") if policy_adapter_fail_open? @@ -200,7 +261,7 @@ def compile_with_arel(intent, current_user = nil) sql = visitor.accept(query.ast, Arel::Collectors::SQLString.new).value - { sql: sql, params: params_hash, bind_spec: bind_spec } + { sql: sql, params: params_hash, bind_spec: bind_spec, intent: intent } rescue StandardError => e @config.logger.warn("[code_to_query] Arel compilation failed: #{e.message}") compile_with_string_building(intent, current_user) @@ -217,7 +278,7 @@ def compile_with_string_building(intent, current_user = nil) if (filters = intent['filters']).present? where_fragments = filters.map do |filter| fragment, placeholder_index = build_string_filter_fragment( - filter, table, bind_spec, params_hash, placeholder_index, current_user + filter, table, bind_spec, params_hash, placeholder_index, current_user, intent ) fragment end @@ -238,7 +299,7 @@ def compile_with_string_building(intent, current_user = nil) sql_parts << build_string_limit_clause(limit) end - { sql: sql_parts.join(' '), params: params_hash, bind_spec: bind_spec } + { sql: sql_parts.join(' '), params: params_hash, bind_spec: bind_spec, intent: intent } end def build_arel_select_query(intent, table) @@ -393,13 +454,14 @@ def build_string_limit_clause(limit) "LIMIT #{Integer(limit)}" end - def build_string_filter_fragment(filter, table, bind_spec, params_hash, placeholder_index, current_user) + def build_string_filter_fragment(filter, table, bind_spec, params_hash, placeholder_index, current_user, intent) col = quote_ident(filter['column']) + col = "#{quote_ident(table)}.#{col}" if base_policy_filter?(filter, intent) case filter['op'] when '=', '>', '<', '>=', '<=', '!=', '<>' build_string_comparison_fragment(col, filter, bind_spec, placeholder_index) when 'exists', 'not_exists' - build_string_subquery_fragment(filter, table, bind_spec, params_hash, placeholder_index, current_user) + build_string_subquery_fragment(filter, table, bind_spec, params_hash, placeholder_index, current_user, intent) when 'between' build_string_between_fragment(col, filter, bind_spec, params_hash, placeholder_index) when 'in' @@ -419,12 +481,17 @@ def build_string_comparison_fragment(quoted_column, filter, bind_spec, placehold end def build_string_between_fragment(quoted_column, filter, bind_spec, params_hash, placeholder_index) - append_between_bind_specs(bind_spec, filter, params_hash) - - placeholder1 = placeholder_for_adapter(placeholder_index) - placeholder2 = placeholder_for_adapter(placeholder_index + 1) + between_clause = build_between_bind_clause( + filter, + bind_spec, + params_hash, + placeholder_index: placeholder_index + ) - ["#{quoted_column} BETWEEN #{placeholder1} AND #{placeholder2}", placeholder_index + 2] + [ + "#{quoted_column} BETWEEN #{between_clause[:start_placeholder]} AND #{between_clause[:end_placeholder]}", + between_clause[:next_placeholder_index] + ] end def build_string_in_fragment(quoted_column, filter, bind_spec, params_hash, placeholder_index) @@ -444,7 +511,7 @@ def build_string_pattern_fragment(quoted_column, filter, bind_spec, placeholder_ ["#{quoted_column} #{filter['op'].upcase} #{placeholder}", placeholder_index + 1] end - def build_string_subquery_fragment(filter, table, bind_spec, params_hash, placeholder_index, current_user) + def build_string_subquery_fragment(filter, table, bind_spec, params_hash, placeholder_index, current_user, intent) related_table = filter['related_table'] fk_column = filter['fk_column'] base_column = filter['base_column'] || 'id' @@ -459,7 +526,7 @@ def build_string_subquery_fragment(filter, table, bind_spec, params_hash, placeh sub_where = ["#{rt}.#{fk_col} = #{quote_ident(table)}.#{base_col}"] sub_where, placeholder_index = apply_policy_in_subquery( - sub_where, bind_spec, params_hash, related_table, placeholder_index, current_user + sub_where, bind_spec, params_hash, placeholder_index, current_user, intent, filter ) related_filters.each do |related_filter| @@ -489,40 +556,48 @@ def build_string_subquery_filter_fragment(quoted_related_table, filter, bind_spe end end - def apply_policy_in_subquery(sub_where, bind_spec, params_hash, related_table, placeholder_index, current_user) + def apply_policy_in_subquery(sub_where, bind_spec, params_hash, placeholder_index, current_user, intent, filter) return [sub_where, placeholder_index] unless @config.policy_adapter.respond_to?(:call) - info = safely_fetch_policy(table: related_table, current_user: current_user) + related_table = filter['related_table'] + info = safely_fetch_policy(table: related_table, current_user: current_user, intent: intent) + merge_policy_allowed_tables!(intent, info, required_table: related_table) predicates = extract_enforced_predicates(info) + enforce_related_allowed_columns!(info, related_table, filter) return [sub_where, placeholder_index] unless predicates.is_a?(Hash) && predicates.any? + # Preserve declaration ownership structurally; policy key fragments are + # intentionally display-safe but are not an injective table identity. + policy_filter_index = Array(intent['filters']).index { |candidate| candidate.equal?(filter) } predicates.each do |column, value| rcol = "#{quote_ident(related_table)}.#{quote_ident(column)}" policy_key_prefix = subquery_policy_key_prefix(related_table, column, placeholder_index) if value.is_a?(Range) && value.begin && value.end start_key = "#{policy_key_prefix}_start" end_key = "#{policy_key_prefix}_end" + merge_policy_expected_keys!(intent, start_key, end_key) params_hash[start_key] = value.begin params_hash[end_key] = value.end p1 = placeholder_for_adapter(placeholder_index) - append_bind_spec(bind_spec, key: start_key, column: column) + append_bind_spec(bind_spec, key: start_key, column: column, policy_filter_index: policy_filter_index) placeholder_index += 1 p2 = placeholder_for_adapter(placeholder_index) - append_bind_spec(bind_spec, key: end_key, column: column) + append_bind_spec(bind_spec, key: end_key, column: column, policy_filter_index: policy_filter_index) placeholder_index += 1 sub_where << "#{rcol} BETWEEN #{p1} AND #{p2}" else key = policy_key_prefix + merge_policy_expected_keys!(intent, key) params_hash[key] = value p = placeholder_for_adapter(placeholder_index) - append_bind_spec(bind_spec, key: key, column: column) + append_bind_spec(bind_spec, key: key, column: column, policy_filter_index: policy_filter_index) placeholder_index += 1 sub_where << "#{rcol} = #{p}" end end [sub_where, placeholder_index] - rescue PolicyAdapterError + rescue PolicyAdapterError, ArgumentError raise rescue StandardError => e raise policy_failure("Policy application failed in subquery: #{e.message}") unless policy_adapter_fail_open? @@ -530,6 +605,48 @@ def apply_policy_in_subquery(sub_where, bind_spec, params_hash, related_table, p [sub_where, placeholder_index] end + def enforce_related_allowed_columns!(policy_info, related_table, filter) + allowed_columns = policy_info[:allowed_columns] || policy_info['allowed_columns'] + return unless allowed_columns.is_a?(Hash) && allowed_columns.any? + + entry = allowed_columns.find do |table, _columns| + policy_column_table_allowed?(related_table, table) + end + if entry.nil? && %i[postgres postgresql].include?(@config.adapter.to_sym) && + allowed_columns.any? { |table, _columns| IdentifierSemantics.ascii_case_insensitive?(related_table, table) } + raise ArgumentError, + "Invalid intent: policy table key not permitted on '#{related_table}' due to identifier casing" + end + return unless entry + + columns = Array(entry.last).map(&:to_s) + return if columns.empty? + + fk_column = filter['fk_column'] + unless policy_column_allowed?(fk_column, columns) + raise ArgumentError, "Invalid intent: column '#{fk_column}' not permitted on '#{related_table}'" + end + + Array(filter['related_filters']).each do |related_filter| + column = related_filter['column'] + next if column.nil? || policy_column_allowed?(column, columns) + + raise ArgumentError, "Invalid intent: filter column '#{column}' not permitted on '#{related_table}'" + end + end + + def policy_column_allowed?(column, allowed_columns) + return allowed_columns.include?(column.to_s) if %i[postgres postgresql].include?(@config.adapter.to_sym) + + allowed_columns.any? { |allowed| IdentifierSemantics.ascii_case_insensitive?(allowed, column) } + end + + def policy_column_table_allowed?(table, allowed) + return IdentifierSemantics.ascii_case_insensitive?(table, allowed) if %i[mysql sqlite].include?(@config.adapter.to_sym) + + table.to_s == allowed.to_s + end + def subquery_policy_key_prefix(table, column, placeholder_index) safe_table = policy_key_fragment(table) safe_column = policy_key_fragment(column) @@ -541,6 +658,94 @@ def policy_key_fragment(value) value.to_s.gsub(/[^a-zA-Z0-9_]/, '_') end + def merge_policy_expected_keys(intent, keys) + (Array(intent['__policy_expected_keys']) + Array(keys)).map(&:to_s).uniq + end + + def merge_policy_expected_keys!(intent, *keys) + intent['__policy_expected_keys'] = merge_policy_expected_keys(intent, keys.flatten) + end + + def merge_policy_allowed_tables!(intent, policy_info, required_table: nil) + return unless policy_info.is_a?(Hash) + + present = policy_info.key?(:allowed_tables) || policy_info.key?('allowed_tables') + return unless present + + value = policy_info.key?(:allowed_tables) ? policy_info[:allowed_tables] : policy_info['allowed_tables'] + allowed_tables = Array(value).map(&:to_s).reject(&:empty?) + if required_table && allowed_tables.none? { |allowed| policy_table_allowed?(required_table, allowed) } + raise PolicyAdapterError, "Policy does not allow related table: #{required_table}" + end + + if required_table + intent['__policy_related_tables'] = + (Array(intent['__policy_related_tables']) + [required_table.to_s]).uniq + return + end + + if intent.key?('__policy_allowed_tables') + existing = Array(intent['__policy_allowed_tables']) + intent['__policy_allowed_tables'] = if existing.empty? || allowed_tables.empty? + [] + else + (existing + allowed_tables).uniq + end + else + intent['__policy_allowed_tables'] = allowed_tables + end + end + + def policy_table_allowed?(table, allowed) + if @config.adapter.to_sym == :sqlite + return IdentifierSemantics.ascii_case_insensitive?(table, allowed) + end + + table.to_s == allowed.to_s + end + + def strip_untrusted_policy_expectations!(intent) + intent.delete('__policy_expected_keys') + intent.delete(:__policy_expected_keys) + intent.delete('__policy_allowed_tables') + intent.delete(:__policy_allowed_tables) + intent.delete('__policy_related_tables') + intent.delete(:__policy_related_tables) + end + + # Independent compiler backstop: metadata alone never proves that a policy + # predicate was emitted. Every expected key must have both a bind and value. + def verify_compiled_policy_binds!(result) + expected = Array(result.dig(:intent, '__policy_expected_keys')).map(&:to_s) + return if expected.empty? + + binds = Array(result[:bind_spec]).filter_map { |bind| bind[:key]&.to_s } + params = (result[:params] || {}).keys.map(&:to_s) + missing = expected.reject { |key| binds.include?(key) && params.include?(key) } + return if missing.empty? + + raise PolicyAdapterError, "Compiled policy binds are missing: #{missing.join(', ')}" + end + + def build_policy_contract(result, allow_tables) + self.class.send(:issue_policy_contract, result, allow_tables, @config) + end + + def deep_dup_value(value) + case value + when Hash + value.each_with_object({}) do |(key, nested_value), copy| + copy[key] = deep_dup_value(nested_value) + end + when Array + value.map { |nested_value| deep_dup_value(nested_value) } + else + value.dup + end + rescue TypeError + value + end + def build_arel_condition(table, filter, bind_spec, params_hash = nil) column = table[filter['column']] operator = filter['op'] @@ -555,11 +760,11 @@ def build_arel_condition(table, filter, bind_spec, params_hash = nil) # Force fallback to string builder for complex correlated subqueries raise StandardError, 'not_exists Arel compilation is not implemented; falling back to string builder' when 'between' - start_key, end_key = append_between_bind_specs(bind_spec, filter, params_hash) + between_clause = build_between_bind_clause(filter, bind_spec, params_hash) - start_param = Arel::Nodes::BindParam.new(start_key) - end_param = Arel::Nodes::BindParam.new(end_key) - column.between(start_param..end_param) + start_param = Arel::Nodes::BindParam.new(between_clause[:start_key]) + end_param = Arel::Nodes::BindParam.new(between_clause[:end_key]) + Arel::Nodes::Between.new(column, Arel::Nodes::And.new([start_param, end_param])) when 'in' key = filter_bind_key(filter) append_bind_spec(bind_spec, key: key, column: filter['column'], cast: :array) @@ -604,6 +809,13 @@ def filter_bind_key(filter) filter['param'] || filter['column'] end + def base_policy_filter?(filter, intent) + expected_keys = Array(intent['__policy_expected_keys']).map(&:to_s) + filter_keys = [filter['param'], filter['param_start'], filter['param_end']].compact.map(&:to_s) + + filter_keys.any? { |key| expected_keys.include?(key) && !key.start_with?('policy_subquery_') } + end + def between_bind_keys(filter) [ filter['param_start'] || default_between_bind_key(filter, 'start'), @@ -621,22 +833,32 @@ def append_between_bind_specs(bind_spec, filter, params_hash = nil) [start_key, end_key] end - def alias_legacy_between_params!(params_hash, filter, start_key, end_key) - return if filter['param_start'] || filter['param_end'] + def build_between_bind_clause(filter, bind_spec, params_hash = nil, placeholder_index: nil) + start_key, end_key = append_between_bind_specs(bind_spec, filter, params_hash) + + { + start_key: start_key, + end_key: end_key, + start_placeholder: placeholder_index && placeholder_for_adapter(placeholder_index), + end_placeholder: placeholder_index && placeholder_for_adapter(placeholder_index + 1), + next_placeholder_index: placeholder_index && (placeholder_index + 2) + } + end - if !params_hash_has_key?(params_hash, start_key) && params_hash_has_key?(params_hash, 'start') - params_hash[start_key] = params_hash['start'] + def alias_legacy_between_params!(params_hash, filter, start_key, end_key) + if !filter['param_start'] && !params_hash_has_key?(params_hash, start_key) && params_hash_has_key?(params_hash, 'start') + params_hash[start_key] = params_hash_value(params_hash, 'start') end - return if params_hash_has_key?(params_hash, end_key) + return if filter['param_end'] || params_hash_has_key?(params_hash, end_key) return unless params_hash_has_key?(params_hash, 'end') - params_hash[end_key] = params_hash['end'] + params_hash[end_key] = params_hash_value(params_hash, 'end') end def default_between_bind_key(filter, bound_side) column = filter['column'].to_s.strip - return bound_side unless column != '' + return bound_side if column.empty? "#{sanitize_bind_key(column)}_#{bound_side}" end @@ -649,12 +871,28 @@ def params_hash_has_key?(params_hash, key) params_hash.key?(key.to_s) || params_hash.key?(key.to_sym) end + def params_hash_value(params_hash, key) + return params_hash[key.to_s] if params_hash.key?(key.to_s) + + params_hash[key.to_sym] + end + + def params_hash_lookup(params_hash, key) + return [false, nil] unless params_hash.respond_to?(:key?) + return [true, params_hash[key.to_s]] if params_hash.key?(key.to_s) + return [true, params_hash[key.to_sym]] if params_hash.key?(key.to_sym) + + [false, nil] + end + def having_bind_key(having_filter) having_filter['param'] || "having_#{having_filter['column']}" end - def append_bind_spec(bind_spec, key:, column:, cast: nil) - bind_spec << { key: key, column: column, cast: cast } + def append_bind_spec(bind_spec, key:, column:, cast: nil, policy_filter_index: nil) + bind = { key: key, column: column, cast: cast } + bind[:policy_filter_index] = policy_filter_index unless policy_filter_index.nil? + bind_spec << bind end def placeholder_for_adapter(index) @@ -846,7 +1084,8 @@ def normalize_params_with_model(intent) key = f['param'] || col next unless key - raw = params[key.to_s] || params[key.to_sym] + present, raw = params_hash_lookup(params, key) + next unless present next if raw.nil? # Map Rails enum string to integer diff --git a/lib/code_to_query/guardrails/sql_linter.rb b/lib/code_to_query/guardrails/sql_linter.rb index d080414..a82c5b2 100644 --- a/lib/code_to_query/guardrails/sql_linter.rb +++ b/lib/code_to_query/guardrails/sql_linter.rb @@ -3,19 +3,49 @@ module CodeToQuery module Guardrails class SqlLinter + DANGEROUS_FUNCTIONS = %w[ + load_file outfile dumpfile + sys_exec sys_eval + benchmark sleep pg_sleep + version user database schema + current_user current_database current_schema + inet_server_addr inet_client_addr + ].freeze + + # PostgreSQL's complete built-in server-side XML export family. These + # functions read a relation, query, cursor, schema, or the whole current + # database without putting the exported relations in a FROM/JOIN clause, + # so every member can bypass this linter's table allowlist. + POSTGRES_SERVER_SIDE_XML_EXPORT_FUNCTIONS = %w[ + table_to_xml table_to_xmlschema table_to_xml_and_xmlschema + query_to_xml query_to_xmlschema query_to_xml_and_xmlschema + cursor_to_xml cursor_to_xmlschema + schema_to_xml schema_to_xmlschema schema_to_xml_and_xmlschema + database_to_xml database_to_xmlschema database_to_xml_and_xmlschema + ].freeze + + POSTGRES_QUOTED_FUNCTION_IDENTIFIER = / + (?(?:""|[^"])*)"\s*\( + /x + + POSTGRES_UNICODE_FUNCTION_IDENTIFIER = / + (?(?:""|[^"])*)" + /ix + def initialize(config, allow_tables: nil) @config = config - # normalize allowlist to lowercase for case-insensitive comparison - @allow_tables = Array(allow_tables).compact.map { |t| t.to_s.downcase } + @allow_tables = Array(allow_tables).compact.map(&:to_s) end def check!(sql) normalized = sql.to_s.strip.gsub(/\s+/, ' ') check_statement_type!(normalized) + check_unsupported_table_query_expressions!(normalized) check_dangerous_patterns!(normalized) check_required_limit!(normalized) check_table_allowlist!(normalized) if @allow_tables.any? + check_no_postgres_server_side_xml_export_functions!(normalized) check_no_literals!(normalized) check_no_dangerous_functions!(normalized) check_no_subqueries!(normalized) if @config.block_subqueries @@ -41,6 +71,17 @@ def check_statement_type!(sql) end end + def check_unsupported_table_query_expressions!(sql) + database = case @config.adapter.to_sym + when :postgres, :postgresql then 'PostgreSQL' + when :mysql then 'MySQL' + else return + end + return unless Query::SqlScanner.new(adapter: @config.adapter).table_query_expression?(sql) + + raise SecurityError, "#{database} TABLE query expressions are not supported" + end + def check_dangerous_patterns!(sql) patterns = build_dangerous_patterns patterns.each do |pattern| @@ -209,12 +250,21 @@ def check_table_allowlist!(sql) referenced_tables = extract_table_names(sql) referenced_tables.each do |table| - unless @allow_tables.include?(table.to_s.downcase) + unless table_allowed?(table) raise SecurityError, "Table '#{table}' is not in the allowed list: #{@allow_tables.join(', ')}" end end end + def table_allowed?(table) + # MySQL table-name case semantics vary by lower_case_table_names and + # host filesystem. Fail closed with exact matching when they are not + # explicitly available to this legacy linter. + return @allow_tables.include?(table.to_s) if @config.adapter.to_sym == :mysql + + @allow_tables.any? { |allowed| IdentifierSemantics.ascii_case_insensitive?(allowed, table) } + end + def check_no_literals!(sql) # Block string literals (except in very specific contexts) # Allow some specific cases like LIKE patterns that might be pre-sanitized @@ -234,20 +284,176 @@ def check_no_literals!(sql) end def check_no_dangerous_functions!(sql) - dangerous_functions = %w[ - load_file outfile dumpfile - sys_exec sys_eval - benchmark sleep pg_sleep - version user database schema - current_user current_database current_schema - inet_server_addr inet_client_addr - ] - - dangerous_functions.each do |func| + DANGEROUS_FUNCTIONS.each do |func| raise SecurityError, "Dangerous function '#{func}' is not allowed" if sql.match?(/\b#{func}\s*\(/i) end end + def check_no_postgres_server_side_xml_export_functions!(sql) + return unless %i[postgres postgresql].include?(@config.adapter.to_sym) + + scanner = Query::SqlScanner.new + quoted_searchable = scanner.mask_literals_and_comments(sql) + executable_searchable = scanner.mask_literals_comments_and_identifier_contents(sql) + + POSTGRES_SERVER_SIDE_XML_EXPORT_FUNCTIONS.each do |func| + next unless postgres_unquoted_function_call?(executable_searchable, func) + + raise SecurityError, "Dangerous function '#{func}' is not allowed" + end + + quoted_searchable.scan(POSTGRES_QUOTED_FUNCTION_IDENTIFIER) do + identifier = Regexp.last_match[:identifier].gsub('""', '"') + next unless POSTGRES_SERVER_SIDE_XML_EXPORT_FUNCTIONS.include?(identifier) + + raise SecurityError, "Dangerous function '#{identifier}' is not allowed" + end + + quoted_searchable.scan(POSTGRES_UNICODE_FUNCTION_IDENTIFIER) do + match = Regexp.last_match + escape, function_call = postgres_unicode_function_escape(sql, match.end(0)) + next unless function_call + + unless escape + raise SecurityError, 'Inconclusive PostgreSQL Unicode function identifier' + end + + identifier = decode_postgres_unicode_identifier(match[:identifier], escape) + unless identifier + raise SecurityError, 'Inconclusive PostgreSQL Unicode function identifier' + end + next unless POSTGRES_SERVER_SIDE_XML_EXPORT_FUNCTIONS.include?(identifier) + + raise SecurityError, "Dangerous function '#{identifier}' is not allowed" + end + end + + def postgres_unicode_function_escape(sql, index) + whitespace_start = index + index += 1 while sql[index]&.match?(/\s/) + escape = '\\' + + if index > whitespace_start && sql[index, 7]&.casecmp?('UESCAPE') && + !postgres_identifier_continuation?(sql[index + 7]) + index += 7 + index += 1 while sql[index]&.match?(/\s/) + escape, index = postgres_uescape_literal(sql, index) + return [nil, true] unless index + + index += 1 while sql[index]&.match?(/\s/) + end + + [escape, sql[index] == '('] + end + + def postgres_uescape_literal(sql, index) + escape_string = sql[index]&.casecmp?('E') && sql[index + 1] == "'" + index += 1 if escape_string + return [nil, nil] unless sql[index] == "'" + + index += 1 + value = +'' + while index < sql.length + if sql[index] == "'" && sql[index + 1] == "'" + value << "'" + index += 2 + elsif sql[index] == "'" + return [value.length == 1 ? value : nil, index + 1] + elsif escape_string && sql[index] == '\\' + character, index = postgres_escape_string_character(sql, index + 1) + return [nil, nil] unless index + + value << character + else + value << sql[index] + index += 1 + end + end + [nil, nil] + end + + def postgres_escape_string_character(sql, index) + return [nil, nil] unless sql[index] + + simple = { 'b' => "\b", 'f' => "\f", 'n' => "\n", 'r' => "\r", 't' => "\t" } + return [simple.fetch(sql[index], sql[index]), index + 1] unless sql[index].match?(/[0-7xXuU]/) + + pattern, base = case sql[index] + when /[0-7]/ then [/[0-7]{1,3}/, 8] + when /[xX]/ then [/[0-9A-F]{1,2}/i, 16] + when 'u' then [/[0-9A-F]{4}/i, 16] + when 'U' then [/[0-9A-F]{8}/i, 16] + end + digit_index = sql[index].match?(/[xXuU]/) ? index + 1 : index + digits = sql[digit_index..]&.match(/\A#{pattern}/)&.[](0) + return [nil, nil] unless digits + + [digits.to_i(base).chr(Encoding::UTF_8), digit_index + digits.length] + rescue RangeError + [nil, nil] + end + + def postgres_unquoted_function_call?(sql, function) + sql.to_enum(:scan, /#{Regexp.escape(function)}\s*\(/i).any? do + match = Regexp.last_match + previous = match.begin(0).positive? ? sql[match.begin(0) - 1] : nil + following = sql[match.begin(0) + function.length] + + !postgres_identifier_continuation?(previous) && !postgres_identifier_continuation?(following) + end + end + + def postgres_identifier_continuation?(character) + character && (!character.ascii_only? || character.match?(/[A-Z0-9_$]/i)) + end + + def decode_postgres_unicode_identifier(identifier, escape) + return if escape.match?(/[0-9A-F+'"\s]/i) + + characters = identifier.gsub('""', '"').chars + decoded = +'' + index = 0 + + while index < characters.length + if characters[index] != escape + decoded << characters[index] + index += 1 + next + end + + codepoint, consumed = postgres_unicode_escape(characters, index, escape) + return unless codepoint + + if codepoint.between?(0xD800, 0xDBFF) + low_surrogate, low_consumed = postgres_unicode_escape(characters, index + consumed, escape) + return unless low_surrogate&.between?(0xDC00, 0xDFFF) + + codepoint = 0x10000 + ((codepoint - 0xD800) << 10) + low_surrogate - 0xDC00 + consumed += low_consumed + end + + decoded << codepoint.chr(Encoding::UTF_8) + index += consumed + end + + decoded + rescue RangeError + nil + end + + def postgres_unicode_escape(characters, index, escape) + return unless characters[index] == escape + return [escape.ord, 2] if characters[index + 1] == escape + + extended = characters[index + 1] == '+' + digits = extended ? 6 : 4 + start = index + (extended ? 2 : 1) + hexadecimal = characters[start, digits]&.join + return unless hexadecimal&.match?(/\A[0-9A-F]{#{digits}}\z/i) + + [hexadecimal.to_i(16), digits + (extended ? 2 : 1)] + end + def check_no_subqueries!(sql) return unless sql.match?(/\(\s*SELECT\b/i) @@ -313,16 +519,20 @@ def check_time_delay_patterns!(sql) def extract_table_names(sql) tables = [] + relation_modifier = %i[postgres postgresql].include?(@config.adapter.to_sym) ? '(?:ONLY\s+)?' : '' + table_reference = /(?:`([^`]+)`|"([^"]+)"|'([^']+)'|([a-zA-Z0-9_]+)(?:\s+(?:AS\s+)?[a-zA-Z_][a-zA-Z0-9_]*)?)/ # Extract FROM clause tables (improved regex) - from_matches = sql.scan(/\bFROM\s+(?:`([^`]+)`|"([^"]+)"|'([^']+)'|([a-zA-Z0-9_]+)(?:\s+(?:AS\s+)?[a-zA-Z_][a-zA-Z0-9_]*)?)/i) + from_matches = sql.scan(/\bFROM\s+#{relation_modifier}#{table_reference}/i) from_matches.each do |match| table_name = match.compact.first tables << table_name if table_name end # Extract JOIN clause tables (improved regex) - join_matches = sql.scan(/\b(?:INNER\s+|LEFT\s+|RIGHT\s+|FULL\s+|CROSS\s+)?JOIN\s+(?:`([^`]+)`|"([^"]+)"|'([^']+)'|([a-zA-Z0-9_]+)(?:\s+(?:AS\s+)?[a-zA-Z_][a-zA-Z0-9_]*)?)/i) + join_matches = sql.scan( + /\b(?:INNER\s+|LEFT\s+|RIGHT\s+|FULL\s+|CROSS\s+)?JOIN\s+#{relation_modifier}#{table_reference}/i + ) join_matches.each do |match| table_name = match.compact.first tables << table_name if table_name diff --git a/lib/code_to_query/identifier_semantics.rb b/lib/code_to_query/identifier_semantics.rb new file mode 100644 index 0000000..11dc4c3 --- /dev/null +++ b/lib/code_to_query/identifier_semantics.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +module CodeToQuery + # Database identifier folding is deliberately narrower than Ruby's Unicode + # case folding. The supported adapters fold ASCII identifier letters only; + # non-ASCII codepoints remain distinct catalog characters. + module IdentifierSemantics + module_function + + def ascii_fold(value) + value.to_s.tr('A-Z', 'a-z') + end + + def ascii_case_insensitive?(left, right) + ascii_fold(left) == ascii_fold(right) + end + + def ascii_case_insensitive_pattern(value) + value.to_s.each_char.map do |character| + if character.match?(/\A[A-Za-z]\z/) + lower = character.tr('A-Z', 'a-z') + "[#{lower}#{lower.tr('a-z', 'A-Z')}]" + else + Regexp.escape(character) + end + end.join + end + end +end diff --git a/lib/code_to_query/policy_adapter_invoker.rb b/lib/code_to_query/policy_adapter_invoker.rb new file mode 100644 index 0000000..7f141bb --- /dev/null +++ b/lib/code_to_query/policy_adapter_invoker.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +module CodeToQuery + # Invokes policy adapters once, selecting a compatible keyword or positional + # context shape before invocation so adapter exceptions can never alter dispatch. + class PolicyAdapterInvoker + class << self + def call(adapter, current_user, table:, intent: nil) + context = available_context(table: table, intent: intent) + parameters = callable_parameters(adapter) + keywords = supported_keywords(parameters, context) + return adapter.call(current_user, **keywords) unless keywords.empty? + return adapter.call(current_user, context) if accepts_positional_context?(parameters) + + adapter.call(current_user) + end + + private + + def available_context(table:, intent:) + { table: table }.tap do |context| + context[:intent] = intent unless intent.nil? + end + end + + def supported_keywords(parameters, context) + return context if parameters.any? { |type, _name| type == :keyrest } + + keyword_names = parameters.filter_map do |type, name| + name if %i[key keyreq].include?(type) + end + context.slice(*keyword_names) + end + + def accepts_positional_context?(parameters) + return true if parameters.any? { |type, _name| type == :rest } + + parameters.count { |type, _name| %i[req opt].include?(type) } >= 2 + end + + def callable_parameters(adapter) + return adapter.parameters if adapter.is_a?(Proc) || adapter.is_a?(Method) + + adapter.method(:call).parameters + end + end + end +end diff --git a/lib/code_to_query/query.rb b/lib/code_to_query/query.rb index 72d514e..5fc6f21 100644 --- a/lib/code_to_query/query.rb +++ b/lib/code_to_query/query.rb @@ -5,22 +5,37 @@ rescue LoadError end +require_relative 'query/sql_scanning' + module CodeToQuery + # rubocop:disable Metrics/ClassLength class Query - attr_reader :sql, :params, :intent, :metrics - - def initialize(sql:, params:, bind_spec:, intent:, allow_tables:, config:) - @sql = sql - @params = normalize_params_with_between_defaults(params || {}, intent['filters']) - @bind_spec = bind_spec || [] - @intent = intent || {} - @allow_tables = allow_tables + def initialize(sql:, params:, bind_spec:, intent:, allow_tables:, config:, policy_contract: nil) + copied_intent = deep_copy(intent || {}) + copied_params = deep_copy(params || {}) + + @sql = deep_freeze(deep_copy(sql)) + @params = deep_freeze(normalize_params_with_between_defaults(copied_params, copied_intent['filters'])) + @bind_spec = deep_freeze(deep_copy(bind_spec || [])) + @intent = deep_freeze(copied_intent) + @allow_tables = deep_freeze(deep_copy(allow_tables)) @config = config + @policy_contract = policy_contract @safety_checked = false @safety_result = nil - @metrics = extract_metrics_from_intent(@intent) + @metrics = deep_freeze(extract_metrics_from_intent(@intent)) end + # Return mutable copies for backwards compatibility without exposing the + # immutable state used by safety checks and execution. + def sql = deep_copy(@sql) + + def params = deep_copy(@params) + + def intent = deep_copy(@intent) + + def metrics = deep_copy(@metrics) + def binds return [] unless defined?(ActiveRecord::Base) @@ -37,10 +52,10 @@ def binds column_name = bind_info[:column] # Get parameter value (check both string and symbol keys) - value = @params[key.to_s] || @params[key.to_sym] + value = param_value_for_key(@params, key) # Determine the correct ActiveRecord type - type = infer_column_type(connection, @intent['table'], column_name, bind_info[:cast]) + type = infer_column_type(connection, @intent['table'], column_name, bind_info[:cast], key) ActiveRecord::Relation::QueryAttribute.new(column_name.to_s, value, type) end @@ -87,7 +102,7 @@ def explain def to_relation return nil unless defined?(ActiveRecord::Base) - return nil unless @intent['type'] == 'select' + return nil unless relationable? table_name = @intent['table'] model = infer_model_for_table(table_name) @@ -122,6 +137,8 @@ def to_active_record def relationable? return false unless defined?(ActiveRecord::Base) return false unless @intent['type'] == 'select' + return false if @config.policy_adapter && !compiler_policy_contract? + return false if compiler_only_subquery_policy_filters? !!infer_model_for_table(@intent['table']) end @@ -135,7 +152,7 @@ def to_relation! def preview { - sql: @sql, + sql: deep_copy(@sql), params: preview_params, applied_policies: applied_policy_keys, estimated_cost: nil, @@ -145,12 +162,43 @@ def preview def run CodeToQuery::Instrumentation.instrument(:run, telemetry_payload) do + raise SecurityError, 'Query failed safety checks at execution boundary' unless perform_safety_checks + Runner.new(@config).run(sql: @sql, binds: binds) end end private + def deep_copy(value) + case value + when Hash + value.each_with_object({}) { |(key, item), copy| copy[deep_copy(key)] = deep_copy(item) } + when Array + value.map { |item| deep_copy(item) } + when Symbol, Numeric, true, false, nil + value + else + value.dup + end + rescue TypeError + value + end + + def deep_freeze(value) + case value + when Hash + value.each do |key, item| + deep_freeze(key) + deep_freeze(item) + end + when Array + value.each { |item| deep_freeze(item) } + end + + value.freeze + end + def telemetry_payload { table: @intent['table'], @@ -182,17 +230,20 @@ def normalize_between_filter_params(filters, normalized_params) end def hydrate_between_param_defaults(filter, normalized_params) - return if filter['param_start'] || filter['param_end'] - start_key, end_key = between_filter_keys(filter) return if between_param_key_present?(normalized_params, start_key) && between_param_key_present?(normalized_params, end_key) - legacy_start = normalized_params['start'] || normalized_params[:start] - legacy_end = normalized_params['end'] || normalized_params[:end] + legacy_start = param_value_for_key(normalized_params, 'start') + legacy_end = param_value_for_key(normalized_params, 'end') + + if !filter['param_start'] && between_param_key_present?(normalized_params, 'start') && !between_param_key_present?(normalized_params, start_key) + normalized_params[start_key] = legacy_start + end + + return unless !filter['param_end'] && between_param_key_present?(normalized_params, 'end') && !between_param_key_present?(normalized_params, end_key) - normalized_params[start_key] = legacy_start if legacy_start && !between_param_key_present?(normalized_params, start_key) - normalized_params[end_key] = legacy_end if legacy_end && !between_param_key_present?(normalized_params, end_key) + normalized_params[end_key] = legacy_end end def between_filter_keys(filter) @@ -212,6 +263,13 @@ def between_param_key_present?(params, key) params.key?(key) || params.key?(key.to_sym) || params.key?(key.to_s) end + def param_value_for_key(params, key) + return params[key] if params.key?(key) + return params[key.to_s] if params.key?(key.to_s) + + params[key.to_sym] + end + def query_shape [@intent['type'], @intent['table']].compact.join(':') end @@ -221,16 +279,13 @@ def preview_params end def applied_policy_keys - # Policy predicates compiled by CodeToQuery use policy-prefixed bind keys. - # Surface only those keys so preview callers can audit policy application - # without exposing bind values through telemetry. - keys = Array(@bind_spec).filter_map do |bind| + # Policy predicates compiled by CodeToQuery must surface as policy-prefixed + # bind keys. Surface only those bound keys so preview callers can audit + # policy application without trusting raw params or exposing bind values. + Array(@bind_spec).filter_map do |bind| key = bind[:key] key.to_s if key.to_s.start_with?('policy_') - end - - keys.concat(@params.keys.filter_map { |key| key.to_s if key.to_s.start_with?('policy_') }) - keys.uniq + end.uniq end def policy_applied? @@ -238,12 +293,21 @@ def policy_applied? end def preview_would_run? - Guardrails::SqlLinter.new(@config, allow_tables: @allow_tables).check!(@sql) + lint_sql! true rescue SecurityError false end + def compiler_only_subquery_policy_filters? + Array(@intent['filters']).any? do |filter| + next false unless %w[exists not_exists].include?(filter['op'].to_s) + + related_policy_keys = Array(@intent['__policy_expected_keys']).grep(/\Apolicy_subquery_/) + related_policy_keys.any? + end + end + def extract_metrics_from_intent(intent) data = intent.is_a?(Hash) ? intent['_metrics'] : nil return {} unless data.is_a?(Hash) @@ -258,7 +322,7 @@ def extract_metrics_from_intent(intent) def perform_safety_checks # Basic SQL structure checks - Guardrails::SqlLinter.new(@config, allow_tables: @allow_tables).check!(@sql) + lint_sql! # EXPLAIN-based performance checks return false if @config.enable_explain_gate && !Guardrails::ExplainGate.new(@config).allowed?( @@ -287,17 +351,266 @@ def check_policy_compliance # Verify via bind_spec or params keys rather than scanning SQL text. return true unless @config.policy_adapter - policy_in_binds = Array(@bind_spec).any? do |bind| - key = bind[:key] - key.to_s.start_with?('policy_') + expected_keys = expected_policy_keys + return false unless compiler_policy_contract? + return true if expected_keys.empty? + + bind_keys = Array(@bind_spec).filter_map { |bind| bind[:key]&.to_s } + param_keys = @params.keys.map(&:to_s) + + return false unless expected_keys.all? { |key| bind_keys.include?(key) && param_keys.include?(key) } + + Array(@bind_spec).each_with_index.all? do |bind, index| + next true unless expected_keys.include?(bind[:key]&.to_s) && !bind[:key].to_s.start_with?('policy_subquery_') + + sql_scanner.policy_predicate_bind?( + @sql, @intent['table'], bind[:column], index + 1, adapter: @config.adapter + ) + end + end + + def compiler_policy_contract? + Compiler.send( + :valid_policy_contract?, @policy_contract, + sql: @sql, params: @params, bind_spec: @bind_spec, intent: @intent, + allow_tables: @allow_tables, config: @config + ) + end + + def policy_predicates_expected? + expected_policy_keys.any? + end + + def expected_policy_keys + explicit_keys = Array(@intent['__policy_expected_keys']).map(&:to_s) + filter_keys = Array(@intent['filters']).flat_map do |filter| + [filter['param'], filter['param_start'], filter['param_end']] + end.compact.map(&:to_s).select { |key| key.start_with?('policy_') } + + (explicit_keys + filter_keys).uniq + end + + def effective_lint_allow_tables + explicit_tables = Array(@allow_tables).compact.map(&:to_s).uniq + policy_tables = Array(@intent['__policy_allowed_tables']).compact.map(&:to_s).uniq + return @allow_tables unless policy_allowlist_present? + return policy_tables if explicit_tables.empty? + + explicit_tables.select do |table| + policy_tables.any? { |policy_table| allowlist_names_equivalent?(table, policy_table) } + end + end + + def sql_linter_allow_tables + related_tables = Array(@intent['__policy_related_tables']).compact.map(&:to_s).uniq + return effective_lint_allow_tables if related_tables.empty? || !allowlist_sources_present? + + explicit_tables = Array(@allow_tables).compact.map(&:to_s).uniq + if explicit_tables.any? + related_tables.select! do |table| + explicit_tables.any? { |explicit| allowlist_names_equivalent?(table, explicit) } + end + end + + (Array(effective_lint_allow_tables) + related_tables).compact.map(&:to_s).uniq + end + + def allowlist_sources_present? + Array(@allow_tables).compact.any? || policy_allowlist_present? + end + + def policy_allowlist_present? + @intent.key?('__policy_allowed_tables') + end + + def lint_sql! + if allowlist_sources_present? && Array(effective_lint_allow_tables).empty? + raise SecurityError, 'No tables remain after intersecting explicit and policy allowlists' + end + + Guardrails::SqlLinter.new(@config, allow_tables: sql_linter_allow_tables).check!(@sql) + check_top_level_table_allowlist! + check_policy_scoped_related_table_references! + end + + def check_top_level_table_allowlist! + allowed_tables = Array(effective_lint_allow_tables).compact.map(&:to_s) + return if allowed_tables.empty? + + top_level_sql = strip_exists_subqueries(@sql) + + raise SecurityError, 'Top-level common table expressions are not allowed' if top_level_sql.match?(/\A\s*WITH\b/i) + raise SecurityError, 'Top-level derived tables are not allowed' if top_level_sql.match?(/(?:\bFROM\b|\bJOIN\b|,)\s*(?:LATERAL\s+)?\(/i) + + extract_table_identifiers(top_level_sql).each do |table| + next if allowed_tables.any? { |allowed| table_identifier_allowed?(table, allowed) } + + raise SecurityError, "Table '#{table.name}' is not in the allowed list: #{allowed_tables.join(', ')}" + end + end + + def check_policy_scoped_related_table_references! + policy_scoped_related_tables = declared_related_tables + return unless allowlist_sources_present? + + base_intent_table = @intent['table']&.to_s + top_level_sql = strip_exists_subqueries(@sql) + sql_base_table = sql_scanner.extract_base_table_identifier(top_level_sql) + self_reference_base_table = if base_intent_table && sql_base_table && + table_identifier_allowed?(sql_base_table, base_intent_table) + sql_base_table + end + + extract_table_identifiers(top_level_sql).each do |table| + next unless policy_scoped_related_tables.any? { |allowed| table_identifier_allowed?(table, allowed) } + next if self_reference_base_table && table_identifier_allowed?(table, self_reference_base_table.name) + + raise SecurityError, + "Table '#{table.name}' is only allowed inside declared EXISTS/NOT EXISTS filters" + end + + declared_references = Hash.new { |hash, key| hash[key] = [] } + Array(@intent['filters']).each do |filter| + operator = filter['op'].to_s.downcase + table = filter['related_table']&.to_s + declared_references[[operator, table]] << filter if %w[exists not_exists].include?(operator) && table + end + policy_binds_by_declaration = policy_binds_by_related_filter + + sql_references = Hash.new(0) + subqueries = sql_scanner.exists_subqueries(@sql) + subqueries.each do |subquery| + extract_table_identifiers(strip_exists_subqueries(subquery[:sql])).each do |table| + normalized_table = policy_scoped_related_tables.find { |allowed| table_identifier_allowed?(table, allowed) } + unless normalized_table + raise SecurityError, + "Table '#{table.name}' has an undeclared #{subquery[:operator].upcase} reference" + end + + reference = [subquery[:operator], normalized_table] + sql_references[reference] += 1 + declaration = declared_references[reference][sql_references[reference] - 1] + if declaration + required_bind_numbers = policy_binds_by_declaration.fetch(declaration, []) + next if policy_bind_present_in_subquery?(subquery, required_bind_numbers, subqueries, table.name) + + raise SecurityError, + "Table '#{table.name}' has an unscoped #{subquery[:operator].upcase} reference" + end + + raise SecurityError, + "Table '#{table.name}' has an undeclared #{subquery[:operator].upcase} reference" + end + end + end + + def policy_bind_present_in_subquery?(subquery, policy_bind_numbers, subqueries, table) + return true if policy_bind_numbers.empty? # This occurrence has no row predicate to enforce. + + nested_ranges = subqueries.filter_map do |candidate| + next if candidate.equal?(subquery) + next unless candidate[:start] >= subquery[:start] && candidate[:finish] <= subquery[:finish] + + candidate[:start]...candidate[:finish] + end + present_bind_numbers = sql_scanner.bind_placeholder_positions(@sql).filter_map do |position, bind_number| + bind_number if position >= subquery[:start] && position < subquery[:finish] && + nested_ranges.none? { |range| range.cover?(position) } + end + return false unless (policy_bind_numbers - present_bind_numbers).empty? + + policy_bind_numbers.all? do |number| + bind = Array(@bind_spec)[number - 1] + sql_scanner.policy_predicate_bind_numbers( + subquery[:sql], table, bind && bind[:column], + adapter: @config.adapter, + question_bind_number: number, source_sql: @sql, source_offset: subquery[:start] + ).include?(number) + end + end + + def policy_binds_by_related_filter + filters = Array(@intent['filters']) + declarations = filters.select do |filter| + %w[exists not_exists].include?(filter['op'].to_s.downcase) && filter['related_table'] + end + + # Compiler-issued bind metadata is covered by the opaque policy contract. + # Prefer it over lossy policy-key fragments when it is available. + if Array(@bind_spec).any? { |bind| bind.key?(:policy_filter_index) } + return declarations.each_with_object({}.compare_by_identity) do |declaration, result| + declaration_index = filters.index { |filter| filter.equal?(declaration) } + result[declaration] = Array(@bind_spec).each_with_index.filter_map do |bind, index| + index + 1 if bind[:policy_filter_index] == declaration_index + end + end + end + + # Retain compatibility for direct Query construction with legacy bind specs. + declarations_by_table = declarations.group_by { |filter| filter['related_table'].to_s } + + declarations_by_table.each_with_object({}.compare_by_identity) do |(table, table_declarations), result| + table_fragment = table.gsub(/[^a-zA-Z0-9_]/, '_') + bind_numbers = Array(@bind_spec).each_with_index.filter_map do |bind, index| + key = bind[:key]&.to_s + index + 1 if key&.match?(/\Apolicy_subquery_\d+_#{Regexp.escape(table_fragment)}_/) + end + quotient, remainder = bind_numbers.length.divmod(table_declarations.length) + offset = 0 + table_declarations.each_with_index do |declaration, index| + count = quotient + (index < remainder ? 1 : 0) + result[declaration] = bind_numbers.slice(offset, count) + offset += count + end end + end + + def declared_related_tables + Array(@intent['filters']).filter_map do |filter| + op = filter['op'].to_s.downcase + next unless %w[exists not_exists].include?(op) + + filter['related_table']&.to_s + end.compact.uniq + end + + def strip_exists_subqueries(sql) + sql_scanner.strip_exists_subqueries(sql) + end + + def extract_table_names(sql) + sql_scanner.extract_table_names(sql) + end - policy_in_params = @params.keys.any? { |k| k.to_s.start_with?('policy_') } + def extract_table_identifiers(sql) + sql_scanner.extract_table_identifiers(sql) + end + + # Allowlist names are catalog identifiers. Lowercase names denote portable + # unquoted identifiers; mixed/uppercase names denote deliberately cased + # identifiers. SQLite is the exception: it resolves identifiers without + # regard to case even when they are quoted. + def table_identifier_allowed?(identifier, allowed) + return IdentifierSemantics.ascii_case_insensitive?(identifier.name, allowed) if @config.adapter.to_sym == :sqlite + # MySQL table-name case semantics depend on lower_case_table_names and + # the host filesystem. Without server metadata, only exact case is safe. + return identifier.name == allowed if @config.adapter.to_sym == :mysql + return identifier.name == allowed if identifier.quoted + + allowed == IdentifierSemantics.ascii_fold(allowed) && IdentifierSemantics.ascii_fold(identifier.name) == allowed + end + + def allowlist_names_equivalent?(left, right) + return IdentifierSemantics.ascii_case_insensitive?(left, right) if @config.adapter.to_sym == :sqlite + + left == right || (left == IdentifierSemantics.ascii_fold(left) && right == IdentifierSemantics.ascii_fold(right) && IdentifierSemantics.ascii_case_insensitive?(left, right)) + end - policy_in_binds || policy_in_params + def sql_scanner + @sql_scanner ||= SqlScanner.new(adapter: @config.adapter) end - def infer_column_type(connection, table_name, column_name, explicit_cast) + def infer_column_type(connection, table_name, column_name, explicit_cast, param_key = column_name) return explicit_cast if explicit_cast # Try to get column info from ActiveRecord @@ -320,7 +633,7 @@ def infer_column_type(connection, table_name, column_name, explicit_cast) end # Ultimate fallback: infer from parameter value - infer_type_from_value(@params[column_name] || @params[column_name.to_sym]) + infer_type_from_value(param_value_for_key(@params, param_key)) end def infer_type_from_value(value) @@ -368,31 +681,31 @@ def apply_filter_to_scope(scope, filter) case operator when '=' param_key = filter['param'] || column - value = @params[param_key.to_s] || @params[param_key.to_sym] + value = param_value_for_key(@params, param_key) scope.where(column => value) when '!=', '<>' param_key = filter['param'] || column - value = @params[param_key.to_s] || @params[param_key.to_sym] + value = param_value_for_key(@params, param_key) scope.where.not(column => value) when '>', '>=', '<', '<=' param_key = filter['param'] || column - value = @params[param_key.to_s] || @params[param_key.to_sym] + value = param_value_for_key(@params, param_key) scope.where("#{scope.connection.quote_column_name(column)} #{operator} ?", value) when 'between' start_key, end_key = between_filter_keys(filter) start_key, end_key = [start_key, end_key].map(&:to_s) - start_value = @params[start_key] || @params[start_key.to_sym] - end_value = @params[end_key] || @params[end_key.to_sym] - start_value ||= @params['start'] || @params[:start] - end_value ||= @params['end'] || @params[:end] + start_value = param_value_for_key(@params, start_key) + end_value = param_value_for_key(@params, end_key) + start_value = param_value_for_key(@params, 'start') if !filter['param_start'] && !between_param_key_present?(@params, start_key) + end_value = param_value_for_key(@params, 'end') if !filter['param_end'] && !between_param_key_present?(@params, end_key) scope.where(column => (start_value..end_value)) when 'in' param_key = filter['param'] || column - values = @params[param_key.to_s] || @params[param_key.to_sym] + values = param_value_for_key(@params, param_key) scope.where(column => Array(values)) when 'like', 'ilike' param_key = filter['param'] || column - value = @params[param_key.to_s] || @params[param_key.to_sym] + value = param_value_for_key(@params, param_key) scope.where("#{scope.connection.quote_column_name(column)} #{operator.upcase} ?", value) when 'exists', 'not_exists' related_table = filter['related_table'] @@ -415,7 +728,7 @@ def apply_filter_to_scope(scope, filter) rcol = rf['column'] rop = rf['op'] rkey = rf['param'] || rcol - rval = @params[rkey.to_s] || @params[rkey.to_sym] + rval = param_value_for_key(@params, rkey) next if rcol.nil? || rop.nil? case rop @@ -428,10 +741,10 @@ def apply_filter_to_scope(scope, filter) when 'between' start_key, end_key = between_filter_keys(rf) start_key, end_key = [start_key, end_key].map(&:to_s) - start_val = @params[start_key] || @params[start_key.to_sym] - end_val = @params[end_key] || @params[end_key.to_sym] - start_val ||= @params['start'] || @params[:start] - end_val ||= @params['end'] || @params[:end] + start_val = param_value_for_key(@params, start_key) + end_val = param_value_for_key(@params, end_key) + start_val = param_value_for_key(@params, 'start') if !rf['param_start'] && !between_param_key_present?(@params, start_key) + end_val = param_value_for_key(@params, 'end') if !rf['param_end'] && !between_param_key_present?(@params, end_key) subquery = subquery.where("#{related_table}.#{rcol} BETWEEN ? AND ?", start_val, end_val) when 'in' vals = Array(rval) @@ -500,4 +813,5 @@ def format_explain_result(result) end end end + # rubocop:enable Metrics/ClassLength end diff --git a/lib/code_to_query/query/sql_scanning.rb b/lib/code_to_query/query/sql_scanning.rb new file mode 100644 index 0000000..77f402e --- /dev/null +++ b/lib/code_to_query/query/sql_scanning.rb @@ -0,0 +1,718 @@ +# frozen_string_literal: true + +module CodeToQuery + class Query + # Internal SQL scanning helpers used to enforce table allowlists while + # preserving quoted literals, identifiers, and comment boundaries. + class SqlScanner + TableIdentifier = Struct.new(:name, :quoted, keyword_init: true) + + # PostgreSQL keywords that cannot be used as an unquoted relation name. + # Keeping this distinction in the scanner matters because words such as + # IS after `(table` are expression grammar, not the relation operand of a + # TABLE query expression. Quoted words remain valid relation names. + POSTGRES_RESERVED_KEYWORDS = %w[ + ALL ANALYSE ANALYZE AND ANY ARRAY AS ASC ASYMMETRIC BOTH CASE CAST CHECK + COLLATE COLUMN CONSTRAINT CREATE CURRENT_CATALOG CURRENT_DATE CURRENT_ROLE + CURRENT_TIME CURRENT_TIMESTAMP CURRENT_USER DEFAULT DEFERRABLE DESC DISTINCT + DO ELSE END EXCEPT FALSE FETCH FOR FOREIGN FREEZE FROM FULL GRANT GROUP + HAVING ILIKE IN INITIALLY INNER INTERSECT INTO IS ISNULL JOIN LATERAL LEADING + LEFT LIKE LIMIT LOCALTIME LOCALTIMESTAMP NATURAL NOT NOTNULL NULL NULLS OFFSET + ON ONLY OR ORDER OUTER OVERLAPS PLACING PRIMARY REFERENCES RETURNING RIGHT + SELECT SESSION_USER SIMILAR SOME SYMMETRIC TABLE THEN TO TRAILING TRUE UNION + UNIQUE USER USING VARIADIC VERBOSE WHEN WHERE WINDOW WITH + ].freeze + + def initialize(adapter: nil) + @adapter = adapter&.to_sym + end + + def strip_exists_subqueries(sql) + source = sql.to_s + searchable = mask_sql_literals_comments_and_identifier_contents(source) + stripped = +'' + index = 0 + + while index < source.length + match = /\b(?:NOT\s+)?EXISTS\s*\(/i.match(searchable, index) + break unless match + + stripped << source[index...match.begin(0)] + stripped << 'TRUE' + index = skip_parenthesized_sql(source, match.end(0)) + end + + stripped << source[index..] if index < source.length + stripped + end + + def exists_subqueries(sql, source_offset: 0) + source = sql.to_s + searchable = mask_sql_literals_comments_and_identifier_contents(source) + subqueries = [] + index = 0 + + while (match = /\b(NOT\s+)?EXISTS\s*\(/i.match(searchable, index)) + boundary = skip_parenthesized_sql(source, match.end(0)) + body_start = match.end(0) + body = source[body_start...(boundary - 1)] + subqueries << { + operator: match[1] ? 'not_exists' : 'exists', sql: body, + start: source_offset + body_start, finish: source_offset + boundary - 1 + } + subqueries.concat(exists_subqueries(body, source_offset: source_offset + body_start)) + index = boundary + end + + subqueries + end + + def bind_placeholder_positions(sql) + searchable = mask_sql_literals_comments_and_identifier_contents(sql.to_s) + if searchable.match?(/\$\d+/) + searchable.to_enum(:scan, /\$(\d+)/).map do + [Regexp.last_match.begin(0), Regexp.last_match(1).to_i] + end + else + ordinal = 0 + searchable.to_enum(:scan, /\?/).map do + ordinal += 1 + [Regexp.last_match.begin(0), ordinal] + end + end + end + + # PostgreSQL's TABLE relation query is a SELECT-equivalent and can occur + # wherever a query operand is accepted. Tokenize code (with literals, + # comments, and quoted identifier contents masked) so PostgreSQL's broad + # unquoted identifier syntax is handled without mistaking TABLE inside + # data or identifiers. + def table_query_expression?(sql) + tokens = sql_tokens(sql.to_s) + tokens.each_with_index.any? do |token, index| + next false unless keyword_token?(token, 'TABLE') + next false unless query_operand_boundary?(tokens, index) + + relation_finish = table_relation_operand_finish(tokens, index + 1) + relation_finish && table_query_continuation?(tokens[relation_finish]) + end + end + + # Preserve source offsets while hiding literal values and comments. This + # lets callers inspect executable SQL without mistaking function-like text + # in PostgreSQL dollar quotes for code. + def mask_literals_and_comments(sql) + mask_sql_literals_and_comments(sql.to_s) + end + + # Also hide quoted identifier bodies when scanning for executable, + # unquoted tokens. Delimiters stay in place so callers can separately + # inspect exact quoted identifiers without losing source offsets. + def mask_literals_comments_and_identifier_contents(sql) + mask_sql_literals_comments_and_identifier_contents(sql.to_s) + end + + def policy_predicate_bind?(sql, table, column, bind_number, adapter:) + policy_predicate_bind_numbers( + sql, table, column, adapter: adapter, question_bind_number: bind_number, source_sql: sql + ).include?(bind_number) + end + + # Identify binds used as values of the expected qualified policy column. + # A placeholder elsewhere in the query does not enforce policy. + def policy_predicate_bind_numbers(sql, table, column, adapter: :postgres, question_bind_number: nil, + source_sql: sql, source_offset: 0) + return [] if table.to_s.empty? || column.to_s.empty? + + searchable = mask_sql_literals_and_comments(sql.to_s) + where_body, where_offset = top_level_where_body(searchable) + return [] unless where_body + + conjuncts = top_level_conjuncts(where_body) + return [] unless conjuncts + + qualified = "#{policy_identifier_pattern(table, adapter, table: true)}\\s*\\.\\s*" \ + "#{policy_identifier_pattern(column, adapter, table: false)}" + wrappers = ->(predicate) { /\A\s*\(*\s*#{predicate}\s*\)*\s*\z/ } + question_pattern = wrappers.call( + "#{qualified}\\s*(?:=\\s*\\?|(?i:BETWEEN)\\s*\\?\\s+(?i:AND)\\s*\\?)" + ) + placeholder_positions = bind_placeholder_positions(source_sql) + first_placeholder = placeholder_positions.first + question_placeholders = first_placeholder && source_sql.to_s[first_placeholder.first] == '?' + if question_bind_number && question_placeholders + placeholder_ordinals = placeholder_positions.to_h + search_from = 0 + conjuncts.each do |conjunct| + conjunct_offset = where_body.index(conjunct, search_from) + search_from = conjunct_offset + conjunct.length + next unless (match = question_pattern.match(conjunct)) + + match[0].to_enum(:scan, /\?/).each do + predicate_offset = where_offset + conjunct_offset + match.begin(0) + local_position = predicate_offset + Regexp.last_match.begin(0) + ordinal = placeholder_ordinals[source_offset + local_position] + return [ordinal] if ordinal == question_bind_number + end + end + return [] + end + + pattern = wrappers.call( + "#{qualified}\\s*(?:=\\s*\\$(\\d+)|(?i:BETWEEN)\\s*\\$(\\d+)\\s+(?i:AND)\\s*\\$(\\d+))" + ) + conjuncts.flat_map do |conjunct| + match = pattern.match(conjunct) + match ? match.captures.compact.map(&:to_i) : [] + end.uniq + end + + private + + def sql_tokens(source) + searchable = mask_sql_literals_comments_and_identifier_contents(source) + tokens = [] + index = 0 + while index < searchable.length + if searchable[index].match?(/\s/) + index += 1 + elsif postgres_unicode_quoted_identifier_start?(searchable, index) + finish = quoted_identifier_finish(source, index + 2) + tokens << { + type: :identifier, value: source[index...finish], quoted: true, unicode_quoted: true + } + index = finish + elsif (delimiter = quoted_identifier_delimiter(searchable[index])) + finish = quoted_identifier_finish(source, index, delimiter) + tokens << { type: :identifier, value: source[index...finish], quoted: true } + index = finish + elsif postgres_identifier_start?(searchable[index]) + finish = index + 1 + finish += 1 while postgres_identifier_continuation?(searchable[finish]) + tokens << { type: :identifier, value: searchable[index...finish] } + index = finish + else + tokens << { type: :symbol, value: searchable[index] } + index += 1 + end + end + tokens + end + + def quoted_identifier_finish(source, index, delimiter = '"') + index += 1 + while index < source.length + if source[index] == delimiter && source[index + 1] == delimiter + index += 2 + elsif source[index] == delimiter + return index + 1 + else + index += 1 + end + end + source.length + end + + def quoted_identifier_delimiter(character) + return '"' if character == '"' + + '`' if @adapter == :mysql && character == '`' + end + + # PostgreSQL lexes U&"..." (with no whitespace around the ampersand) as + # one Unicode-escaped delimited identifier. Treating U, &, and the quoted + # portion as separate tokens would miss a valid TABLE relation operand. + def postgres_unicode_quoted_identifier_start?(searchable, index) + searchable[index]&.casecmp?('U') && searchable[index + 1] == '&' && searchable[index + 2] == '"' + end + + # PostgreSQL's lexer permits any non-ASCII character in an unquoted + # identifier, including characters Unicode classifies as symbols. Avoid + # a Unicode letter/category allowlist here: it would miss executable + # relation names such as an emoji. + def postgres_identifier_start?(character) + return false unless character + + !character.ascii_only? || character == '_' || character.match?(/[A-Za-z]/) + end + + def postgres_identifier_continuation?(character) + postgres_identifier_start?(character) || character&.match?(/[0-9$]/) + end + + def keyword_token?(token, keyword) + token && token[:type] == :identifier && token[:value].casecmp?(keyword) + end + + def postgres_relation_identifier_token?(token) + return false unless token&.fetch(:type, nil) == :identifier + return true if token[:quoted] + + POSTGRES_RESERVED_KEYWORDS.none? { |keyword| token[:value].casecmp?(keyword) } + end + + def postgres_relation_identifier_finish(tokens, index) + token = tokens[index] + return unless postgres_relation_identifier_token?(token) + + index += 1 + # PostgreSQL permits an optional UESCAPE string after each U& quoted + # identifier. String literal contents are deliberately absent from the + # token stream, so consuming UESCAPE here reaches the next SQL token. + index += 1 if token[:unicode_quoted] && keyword_token?(tokens[index], 'UESCAPE') + index + end + + # Parse TABLE's complete relation operand before deciding whether TABLE + # starts a query expression. At an operand boundary, `(table IS NULL)` + # and `(TABLE accounts)` initially look alike; the token after the + # relation is what distinguishes expression grammar from query grammar. + def table_relation_operand_finish(tokens, index) + index += 1 if keyword_token?(tokens[index], 'ONLY') + parenthesized = tokens[index]&.fetch(:value, nil) == '(' + index += 1 if parenthesized + index = postgres_relation_identifier_finish(tokens, index) + return unless index + + while tokens[index]&.fetch(:value, nil) == '.' + index = postgres_relation_identifier_finish(tokens, index + 1) + return unless index + end + if parenthesized + return unless tokens[index]&.fetch(:value, nil) == ')' + + index += 1 + end + index += 1 if tokens[index]&.fetch(:value, nil) == '*' + index + end + + def table_query_continuation?(token) + return true unless token + return true if token[:value] == ')' + + %w[UNION INTERSECT EXCEPT ORDER LIMIT OFFSET FETCH FOR].any? do |keyword| + keyword_token?(token, keyword) + end + end + + def query_operand_boundary?(tokens, index) + return true if index.zero? + + previous = tokens[index - 1] + return true if previous&.fetch(:value, nil) == '(' + return true if %w[UNION INTERSECT EXCEPT].any? { |keyword| keyword_token?(previous, keyword) } + return true if cte_query_operand_boundary?(tokens, index) + return false unless %w[ALL DISTINCT].any? { |keyword| keyword_token?(previous, keyword) } + + %w[UNION INTERSECT EXCEPT].any? { |keyword| keyword_token?(tokens[index - 2], keyword) } + end + + # At the outer level of a WITH operand, CTE definitions end in a closing + # parenthesis immediately before the main SELECT/TABLE/VALUES expression. + # Walk back over those balanced definitions to distinguish that position + # from an ordinary identifier or alias named `table`. + def cte_query_operand_boundary?(tokens, index) + depth = 0 + cursor = index - 1 + while cursor >= 0 + token = tokens[cursor] + value = token[:value] + if value == ')' + depth += 1 + elsif value == '(' + return false if depth.zero? + + depth -= 1 + elsif depth.zero? + return query_operand_boundary?(tokens, cursor) if keyword_token?(token, 'WITH') + return false if %w[SELECT VALUES TABLE].any? { |keyword| keyword_token?(token, keyword) } + return false if value == ';' + end + cursor -= 1 + end + false + end + + def top_level_where_body(searchable) + structure = mask_sql_literals_comments_and_identifier_contents(searchable) + depth = 0 + where_end = nil + structure.to_enum(:scan, /\bWHERE\b|[()]/i).each do + token = Regexp.last_match(0) + if token == '(' + depth += 1 + elsif token == ')' + return [nil, nil] if depth.zero? + + depth -= 1 + elsif depth.zero? + where_end = Regexp.last_match.end(0) + break + end + end + return [nil, nil] unless where_end + + finish = searchable.length + suffix = structure[where_end..] + suffix.to_enum(:scan, /\b(?:GROUP\s+BY|HAVING|ORDER\s+BY|LIMIT|OFFSET|FETCH|FOR|UNION|INTERSECT|EXCEPT|RETURNING)\b|[()]/i).each do + token = Regexp.last_match(0) + if token == '(' + depth += 1 + elsif token == ')' + return [nil, nil] if depth.zero? + + depth -= 1 + elsif depth.zero? + finish = where_end + Regexp.last_match.begin(0) + break + end + end + return [nil, nil] unless depth.zero? + + [searchable[where_end...finish], where_end] + end + + def policy_identifier_pattern(value, adapter, table:) + escaped = Regexp.escape(value.to_s) + ascii_folded = IdentifierSemantics.ascii_case_insensitive_pattern(value) + boundary = '[\\p{L}\\p{M}\\p{N}\\p{Pc}$]' + exact_unquoted = "(? CodeToQuery.config.default_limit) @@ -123,7 +127,8 @@ def enforce_allowlists!(intent, current_user:, allow_tables:) # Enforce table allowlist if provided (from user input) if Array(allow_tables).any? table = fetch_value(intent, :table) - if (table.to_s.strip != '') && !Array(allow_tables).map { |t| t.to_s.downcase }.include?(table.to_s.downcase) + if (table.to_s.strip != '') && + Array(allow_tables).none? { |allowed| IdentifierSemantics.ascii_case_insensitive?(allowed, table) } raise ArgumentError, "Invalid intent: table '#{table}' not allowed" end end @@ -145,10 +150,14 @@ def enforce_allowlists!(intent, current_user:, allow_tables:) raise CodeToQuery::PolicyAdapterError, message end - allowed_tables = Array(fetch_value(policy_info, :allowed_tables)).map { |t| t.to_s.downcase } - if allowed_tables.any? + policy_has_allowed_tables = policy_info.key?(:allowed_tables) || policy_info.key?('allowed_tables') + if policy_has_allowed_tables + allowed_tables = Array(fetch_value(policy_info, :allowed_tables)).map(&:to_s) + intent[:__policy_allowed_tables] = allowed_tables + intent['__policy_allowed_tables'] = allowed_tables + table = fetch_value(intent, :table) - if (table.to_s.strip != '') && !allowed_tables.include?(table.to_s.downcase) + if (table.to_s.strip != '') && allowed_tables.none? { |allowed| policy_table_allowed?(table, allowed) } raise ArgumentError, "Invalid intent: table '#{table}' not permitted by policy" end end @@ -156,19 +165,22 @@ def enforce_allowlists!(intent, current_user:, allow_tables:) allowed_columns = fetch_value(policy_info, :allowed_columns) || {} return if allowed_columns.nil? || allowed_columns.empty? - # Normalize map keys to strings with lowercase table and column names + # Preserve identifier case because PostgreSQL quotes every identifier. + # Quoted PostgreSQL columns are distinct by case, while MySQL and SQLite + # column identifiers remain case-insensitive. normalized = {} allowed_columns.each do |tbl, cols| - normalized[tbl.to_s.downcase] = Array(cols).map { |c| c.to_s.downcase } + normalized[tbl.to_s] = Array(cols).map(&:to_s) end - main_table = fetch_value(intent, :table).to_s.downcase + main_table = fetch_value(intent, :table).to_s + main_columns = policy_columns_for_table(normalized, main_table) # Columns in SELECT Array(fetch_value(intent, :columns)).each do |col| next if col == '*' - next unless normalized[main_table]&.any? - unless normalized[main_table].include?(col.to_s.downcase) + next unless main_columns&.any? + unless policy_column_allowed?(col, main_columns) raise ArgumentError, "Invalid intent: selecting column '#{col}' not permitted on '#{main_table}'" end end @@ -177,40 +189,54 @@ def enforce_allowlists!(intent, current_user:, allow_tables:) Array(fetch_value(intent, :order)).each do |o| col = fetch_value(o, :column) next if col.nil? - next unless normalized[main_table]&.any? - unless normalized[main_table].include?(col.to_s.downcase) + next unless main_columns&.any? + unless policy_column_allowed?(col, main_columns) raise ArgumentError, "Invalid intent: ordering by column '#{col}' not permitted on '#{main_table}'" end end # DISTINCT ON columns Array(fetch_value(intent, :distinct_on)).each do |col| - next unless normalized[main_table]&.any? - unless normalized[main_table].include?(col.to_s.downcase) + next unless main_columns&.any? + unless policy_column_allowed?(col, main_columns) raise ArgumentError, "Invalid intent: distinct_on column '#{col}' not permitted on '#{main_table}'" end end # GROUP BY Array(fetch_value(intent, :group_by)).each do |col| - next unless normalized[main_table]&.any? - unless normalized[main_table].include?(col.to_s.downcase) + next unless main_columns&.any? + unless policy_column_allowed?(col, main_columns) raise ArgumentError, "Invalid intent: group_by column '#{col}' not permitted on '#{main_table}'" end end + # Aggregation columns + Array(fetch_value(intent, :aggregations)).each do |aggregation| + col = fetch_value(aggregation, :column) + next if col.nil? + next unless main_columns&.any? + unless policy_column_allowed?(col, main_columns) + raise ArgumentError, "Invalid intent: aggregation column '#{col}' not permitted on '#{main_table}'" + end + end + # WHERE filters Array(fetch_value(intent, :filters)).each do |f| op = fetch_value(f, :op).to_s if %w[exists not_exists].include?(op) related_table = fetch_value(f, :related_table) - rel_cols = normalized[related_table.to_s.downcase] + rel_cols = policy_columns_for_table(normalized, related_table) + + ensure_policy_column_allowed!(fetch_value(f, :fk_column), related_table, rel_cols) + ensure_policy_column_allowed!(fetch_value(f, :base_column), main_table, main_columns) + next if rel_cols.nil? || rel_cols.empty? Array(fetch_value(f, :related_filters)).each do |rf| col = fetch_value(rf, :column) next if col.nil? - unless rel_cols.include?(col.to_s.downcase) + unless policy_column_allowed?(col, rel_cols) raise ArgumentError, "Invalid intent: filter column '#{col}' not permitted on '#{related_table}'" end end @@ -218,9 +244,9 @@ def enforce_allowlists!(intent, current_user:, allow_tables:) col = fetch_value(f, :column) next if col.nil? - cols = normalized[main_table] + cols = main_columns next if cols.nil? || cols.empty? - unless cols.include?(col.to_s.downcase) + unless policy_column_allowed?(col, cols) raise ArgumentError, "Invalid intent: filter column '#{col}' not permitted on '#{main_table}'" end end @@ -232,24 +258,38 @@ def enforce_allowlists!(intent, current_user:, allow_tables:) raise ArgumentError, e.message end - def safe_call_policy_adapter(adapter, current_user, table:, intent:) - adapter.call(current_user, table: table, intent: intent) - rescue ArgumentError - begin - adapter.call(current_user, table: table) - rescue ArgumentError - begin - adapter.call(current_user) - rescue StandardError => e - return handle_policy_failure("Policy adapter failed: #{e.message}") if policy_adapter_fail_open? - - raise CodeToQuery::PolicyAdapterError, "Policy adapter failed: #{e.message}" - end - rescue StandardError => e - return handle_policy_failure("Policy adapter failed: #{e.message}") if policy_adapter_fail_open? + def ensure_policy_column_allowed!(column, table, allowed_columns) + return if allowed_columns.nil? || allowed_columns.empty? + return if policy_column_allowed?(column, allowed_columns) + + raise ArgumentError, "Invalid intent: column '#{column}' not permitted on '#{table}'" + end + + def policy_columns_for_table(allowed_columns, table) + entry = allowed_columns.find { |allowed_table, _columns| policy_table_allowed?(table, allowed_table) } + return entry.last if entry - raise CodeToQuery::PolicyAdapterError, "Policy adapter failed: #{e.message}" + return unless %i[postgres postgresql].include?(CodeToQuery.config.adapter.to_sym) + return unless allowed_columns.any? do |allowed_table, _columns| + IdentifierSemantics.ascii_case_insensitive?(table, allowed_table) end + + # PostgreSQL quotes identifiers, so a case-only policy key names a + # different table. Reject the near-match here rather than waiting for a + # column reference, since wildcard and columnless queries have none. + raise ArgumentError, "Invalid intent: policy table key not permitted on '#{table}' due to identifier casing" + end + + def policy_column_allowed?(column, allowed_columns) + if %i[postgres postgresql].include?(CodeToQuery.config.adapter.to_sym) + return allowed_columns.include?(column.to_s) + end + + allowed_columns.any? { |allowed| IdentifierSemantics.ascii_case_insensitive?(allowed, column) } + end + + def safe_call_policy_adapter(adapter, current_user, table:, intent:) + PolicyAdapterInvoker.call(adapter, current_user, table: table, intent: intent) rescue StandardError => e if policy_adapter_fail_open? CodeToQuery.config.logger.warn("[code_to_query] Policy adapter failed: #{e.message}") @@ -259,6 +299,14 @@ def safe_call_policy_adapter(adapter, current_user, table:, intent:) raise CodeToQuery::PolicyAdapterError, "Policy adapter failed: #{e.message}" end + def policy_table_allowed?(table, allowed) + if %i[mysql sqlite].include?(CodeToQuery.config.adapter.to_sym) + return IdentifierSemantics.ascii_case_insensitive?(table, allowed) + end + + table.to_s == allowed.to_s + end + def handle_policy_failure(message) CodeToQuery.config.logger.warn("[code_to_query] #{message}") {} diff --git a/spec/code_to_query/compiler_spec.rb b/spec/code_to_query/compiler_spec.rb index 915c233..0995848 100644 --- a/spec/code_to_query/compiler_spec.rb +++ b/spec/code_to_query/compiler_spec.rb @@ -172,6 +172,50 @@ end end + context 'with related subquery BETWEEN filter' do + let(:intent) do + { + 'type' => 'select', + 'table' => 'questions', + 'columns' => ['*'], + 'filters' => [ + { 'column' => 'archived', 'op' => '=', 'param' => 'archived' }, + { + 'column' => 'id', + 'op' => 'exists', + 'related_table' => 'answers', + 'fk_column' => 'question_id', + 'base_column' => 'id', + 'related_filters' => [ + { 'column' => 'created_at', 'op' => 'between' } + ] + } + ], + 'limit' => 100, + 'params' => { + 'archived' => false, + 'created_at_start' => '2023-01-01', + 'created_at_end' => '2023-12-31' + } + } + end + + it 'preserves subquery BETWEEN placeholders and bind order' do + result = compiler.compile(intent) + + expect(result[:sql]).to eq( + 'SELECT * FROM "questions" WHERE "archived" = $1 AND ' \ + 'EXISTS (SELECT 1 FROM "answers" WHERE "answers"."question_id" = "questions"."id" ' \ + 'AND "answers"."created_at" BETWEEN $2 AND $3) LIMIT 100' + ) + expect(result[:bind_spec]).to eq([ + { key: 'archived', column: 'archived', cast: nil }, + { key: 'created_at_start', column: 'created_at', cast: nil }, + { key: 'created_at_end', column: 'created_at', cast: nil } + ]) + end + end + context 'with WHERE filters' do let(:intent) do { @@ -295,6 +339,74 @@ hash_including(key: 'created_at_end', column: 'created_at') ) end + + it 'aliases legacy symbol keys to derived bind keys for SQL compilation' do + intent['params'] = { start: '2023-01-01', end: '2023-12-31' } + + result = compiler.compile(intent) + + expect(result[:params]['created_at_start']).to eq('2023-01-01') + expect(result[:params]['created_at_end']).to eq('2023-12-31') + end + + it 'preserves falsey legacy symbol values when aliasing derived bind keys' do + intent['params'] = { start: false, end: true } + + result = compiler.compile(intent) + + expect(result[:params]['created_at_start']).to be(false) + expect(result[:params]['created_at_end']).to be(true) + end + end + + context 'with Arel BETWEEN filters lacking explicit param names' do + let(:arel_table) { Arel::Table.new(:orders) } + let(:filter) do + { + 'column' => 'created_at', + 'op' => 'between' + } + end + + it 'reuses derived bind keys and aliases legacy params for Arel compilation' do + bind_spec = [] + params_hash = { 'start' => '2023-01-01', 'end' => '2023-12-31' } + + condition = compiler.__send__(:build_arel_condition, arel_table, filter, bind_spec, params_hash) + + expect(condition).to be_a(Arel::Nodes::Between) + expect(params_hash['created_at_start']).to eq('2023-01-01') + expect(params_hash['created_at_end']).to eq('2023-12-31') + expect(bind_spec).to eq([ + { key: 'created_at_start', column: 'created_at', cast: nil }, + { key: 'created_at_end', column: 'created_at', cast: nil } + ]) + end + + it 'aliases legacy symbol keys for Arel compilation' do + bind_spec = [] + params_hash = { start: '2023-01-01', end: '2023-12-31' } + + compiler.__send__(:build_arel_condition, arel_table, filter, bind_spec, params_hash) + + expect(params_hash['created_at_start']).to eq('2023-01-01') + expect(params_hash['created_at_end']).to eq('2023-12-31') + end + + it 'aliases only the implicit side when one bound keeps an explicit param name' do + bind_spec = [] + params_hash = { 'end' => '2023-12-31' } + mixed_filter = filter.merge('param_start' => 'from_date') + + compiler.__send__(:build_arel_condition, arel_table, mixed_filter, bind_spec, params_hash) + + expect(params_hash).not_to have_key('created_at_start') + expect(params_hash['created_at_end']).to eq('2023-12-31') + expect(bind_spec).to eq([ + { key: 'from_date', column: 'created_at', cast: nil }, + { key: 'created_at_end', column: 'created_at', cast: nil } + ]) + end end context 'with ORDER BY clause' do @@ -691,6 +803,16 @@ expect(result[:bind_spec]).to include(hash_including(key: 'policy_tenant_id', column: 'tenant_id')) end + it 'supports legacy policy adapters that accept only current user' do + config.policy_adapter = ->(user) { { enforced_predicates: { tenant_id: user.fetch(:tenant_id) } } } + allow(config.policy_adapter).to receive(:call).and_call_original + + result = compiler.compile(intent, current_user: { tenant_id: 42 }) + + expect(result[:params]).to include('policy_tenant_id' => 42) + expect(config.policy_adapter).to have_received(:call).once + end + it 'fails closed when the adapter raises' do config.policy_adapter = ->(_user, **) { raise 'policy service unavailable' } @@ -727,6 +849,49 @@ ) end + it 'accepts an empty enforced_predicates contract' do + config.policy_adapter = ->(_user, **) { { enforced_predicates: {} } } + + result = compiler.compile(intent) + + expect(result[:sql]).to eq('SELECT * FROM "orders" LIMIT 100') + expect(result[:bind_spec]).to eq([]) + end + + it 'accepts an empty predicates contract' do + config.policy_adapter = ->(_user, **) { { predicates: {} } } + + result = compiler.compile(intent) + + expect(result[:sql]).to eq('SELECT * FROM "orders" LIMIT 100') + expect(result[:bind_spec]).to eq([]) + end + + it 'does not swallow errors raised inside an intent-aware strict keyword adapter' do + config.policy_adapter = lambda do |_user, table:, intent:| + raise ArgumentError, "policy rejected #{table}" if intent + end + + expect { compiler.compile(intent) }.to raise_error( + CodeToQuery::PolicyAdapterError, /policy rejected orders/ + ) + end + + ['wrong number of arguments', 'unknown keyword: :intent'].each do |message| + it "invokes an intent-aware adapter once and fails closed when it internally raises #{message.inspect}" do + calls = 0 + config.policy_adapter = lambda do |_user, table:, intent:| + calls += 1 + raise ArgumentError, message if table == 'orders' && intent + end + + expect { compiler.compile(intent) }.to raise_error( + CodeToQuery::PolicyAdapterError, /Policy adapter failed: #{Regexp.escape(message)}/ + ) + expect(calls).to eq(1) + end + end + it 'documents explicit availability mode by allowing fail open only when configured' do config.policy_adapter_fail_open = true config.policy_adapter = ->(_user, **) { raise 'policy service unavailable' } @@ -760,12 +925,33 @@ result = compiler.compile(intent) - expect(result[:sql]).to include('WHERE "status" = $1 AND "tenant_id" = $2') + expect(result[:sql]).to include('WHERE "status" = $1 AND "orders"."tenant_id" = $2') expect(result[:params]).to include('status' => 'paid', 'policy_tenant_id' => 42) expect(result[:bind_spec]).to include(hash_including(key: 'status', column: 'status')) expect(result[:bind_spec]).to include(hash_including(key: 'policy_tenant_id', column: 'tenant_id')) end + it 'produces a safe query with adapter-quoted policy identifiers when string building is forced' do + config.policy_adapter = lambda do |_user, **_kwargs| + { allowed_tables: ['AuditEvents'], enforced_predicates: { 'TenantID' => 42 } } + end + allow(compiler).to receive(:use_arel?).and_return(false) + intent = { + 'type' => 'select', 'table' => 'AuditEvents', 'columns' => ['*'], + 'filters' => [], 'limit' => 100, 'params' => {} + } + + result = compiler.compile(intent) + query = CodeToQuery::Query.new( + sql: result[:sql], params: result[:params], bind_spec: result[:bind_spec], + intent: result[:intent], allow_tables: nil, config: config, + policy_contract: result[:policy_contract] + ) + + expect(result[:sql]).to include('"AuditEvents"."TenantID" = $1') + expect(query.safe?).to be true + end + it 'does not let prompt-sourced filters override tenant predicates' do intent = { 'table' => 'orders', @@ -778,7 +964,7 @@ result = compiler.compile(intent) - expect(result[:sql]).to include('"tenant_id" != $1 AND "tenant_id" = $2') + expect(result[:sql]).to include('"tenant_id" != $1 AND "orders"."tenant_id" = $2') expect(result[:params]).to include('attacker_tenant_id' => 42, 'policy_tenant_id' => 42) end end @@ -832,6 +1018,210 @@ def compile_with_related_filters(table:, filters:, params: {}, current_user: nil expect(result[:params]['policy_tenant_id']).to eq(666) expect(result[:params][subquery_key]).to eq(42) expect(result[:bind_spec]).to include(hash_including(key: subquery_key, column: :tenant_id)) + expect(result[:intent]['__policy_expected_keys']).to include(subquery_key) + end + + it 'enforces related-table policy when Arel compilation is selected' do + config.policy_adapter = lambda do |_user, **kwargs| + kwargs[:table] == 'answers' ? { enforced_predicates: { tenant_id: 42 } } : {} + end + allow(compiler).to receive(:use_arel?).and_return(true) + + result = compile_with_related_filters(table: 'questions', filters: [related_filter('answers')]) + + expect(result[:sql]).to include('EXISTS', '"answers"."tenant_id" = $1') + expect(result[:params]).to include('policy_subquery_1_answers_tenant_id' => 42) + expect(result[:bind_spec]).to include(hash_including(key: 'policy_subquery_1_answers_tenant_id')) + end + + it 'discards caller-supplied policy expectation metadata' do + config.policy_adapter = ->(_user, **) { {} } + untrusted_intent = { + 'table' => 'questions', 'columns' => ['*'], 'filters' => [], 'params' => {}, + '__policy_expected_keys' => ['policy_attacker'], + '__policy_allowed_tables' => ['attacker_secrets'], + '__policy_related_tables' => ['attacker_secrets'] + } + + result = compiler.compile(untrusted_intent) + + expect(result[:intent]).not_to have_key('__policy_expected_keys') + expect(result[:intent]).not_to have_key('__policy_allowed_tables') + expect(result[:intent]).not_to have_key('__policy_related_tables') + end + + it 'replaces caller-supplied allowlist metadata with the enforced policy allowlist' do + config.policy_adapter = lambda do |_user, **_kwargs| + { allowed_tables: ['questions'], enforced_predicates: { tenant_id: 42 } } + end + untrusted_intent = { + 'table' => 'questions', 'columns' => ['*'], 'filters' => [], 'params' => {}, + '__policy_allowed_tables' => ['attacker'] + } + + result = compiler.compile(untrusted_intent) + + expect(result[:intent]['__policy_allowed_tables']).to eq(['questions']) + end + + it 'rejects an explicitly empty related policy allowlist' do + config.policy_adapter = lambda do |_user, **kwargs| + kwargs[:table] == 'questions' ? { allowed_tables: ['questions'] } : { allowed_tables: [] } + end + + expect do + compile_with_related_filters(table: 'questions', filters: [related_filter('answers')]) + end.to raise_error(CodeToQuery::PolicyAdapterError, /Policy does not allow related table: answers/) + end + + it 'rejects a related table omitted by its own policy response despite the outer allowlist' do + config.policy_adapter = lambda do |_user, **kwargs| + if kwargs[:table] == 'questions' + { allowed_tables: %w[questions answers] } + else + { allowed_tables: ['questions'] } + end + end + + expect do + compile_with_related_filters(table: 'questions', filters: [related_filter('answers')]) + end.to raise_error(CodeToQuery::PolicyAdapterError, /Policy does not allow related table: answers/) + end + + it 'does not widen top-level authorization with related-table policy metadata' do + config.policy_adapter = lambda do |_user, **kwargs| + case kwargs[:table] + when 'questions' + { allowed_tables: ['questions'] } + when 'answers' + { + allowed_tables: %w[answers users], + enforced_predicates: { tenant_id: 42 } + } + else + {} + end + end + + result = compile_with_related_filters(table: 'questions', filters: [related_filter('answers')]) + query_options = { + params: result[:params], bind_spec: result[:bind_spec], intent: result[:intent], + allow_tables: nil, config: config, policy_contract: result[:policy_contract] + } + correlated_query = CodeToQuery::Query.new(sql: result[:sql], **query_options) + widened_query = CodeToQuery::Query.new( + sql: result[:sql].sub('FROM "questions"', 'FROM "questions" JOIN "users" ON TRUE'), + **query_options + ) + + expect(result[:intent]['__policy_allowed_tables']).to eq(['questions']) + expect(result[:intent]['__policy_related_tables']).to eq(['answers']) + expect(correlated_query.safe?).to be true + expect(widened_query.safe?).to be false + end + + it 'does not widen an explicit caller allowlist with an authorized related table' do + config.policy_adapter = lambda do |_user, **kwargs| + if kwargs[:table] == 'questions' + { allowed_tables: %w[questions answers] } + else + { allowed_tables: ['answers'], enforced_predicates: { tenant_id: 42 } } + end + end + + result = compiler.compile( + { + 'table' => 'questions', 'columns' => ['*'], + 'filters' => [related_filter('answers')], 'limit' => 100, 'params' => {} + }, + allow_tables: ['questions'] + ) + query = CodeToQuery::Query.new( + sql: result[:sql], params: result[:params], bind_spec: result[:bind_spec], + intent: result[:intent], allow_tables: ['questions'], config: config, + policy_contract: result[:policy_contract] + ) + + expect(query.safe?).to be false + end + + # rubocop:disable RSpec/ExampleLength + it 'keeps policy bind ownership distinct for colliding quoted table names' do + config.policy_adapter = lambda do |_user, **kwargs| + case kwargs[:table] + when 'a-b' + { allowed_tables: ['a-b'], enforced_predicates: { tenant_id: 7 } } + when 'a_b' + { allowed_tables: ['a_b'], enforced_predicates: { tenant_id: 9 } } + else + { allowed_tables: %w[questions a-b a_b] } + end + end + + result = compiler.compile( + { + 'table' => 'questions', 'columns' => ['*'], + 'filters' => [ + related_filter('a-b'), + related_filter('a_b', operation: 'not_exists') + ], + 'limit' => 100, 'params' => {} + }, + allow_tables: %w[questions a-b a_b] + ) + query = CodeToQuery::Query.new( + sql: result[:sql], params: result[:params], bind_spec: result[:bind_spec], + intent: result[:intent], allow_tables: %w[questions a-b a_b], config: config, + policy_contract: result[:policy_contract] + ) + + expect(result[:sql]).to include('FROM "a-b"', 'FROM "a_b"') + expect(result[:params]).to include( + 'policy_subquery_1_a_b_tenant_id' => 7, + 'policy_subquery_2_a_b_tenant_id' => 9 + ) + expect(query.safe?).to be true + end + # rubocop:enable RSpec/ExampleLength + + it 'preserves an unrestricted base policy when authorizing a related table' do + config.policy_adapter = lambda do |_user, **kwargs| + if kwargs[:table] == 'answers' + { allowed_tables: %w[answers users], enforced_predicates: { tenant_id: 42 } } + else + {} + end + end + + result = compile_with_related_filters(table: 'questions', filters: [related_filter('answers')]) + query = CodeToQuery::Query.new( + sql: result[:sql], params: result[:params], bind_spec: result[:bind_spec], + intent: result[:intent], allow_tables: nil, config: config, + policy_contract: result[:policy_contract] + ) + + expect(result[:intent]).not_to have_key('__policy_allowed_tables') + expect(result[:intent]['__policy_related_tables']).to eq(['answers']) + expect(query.safe?).to be true + end + + it 'does not mutate the caller intent while recording subquery policy expectations' do + config.policy_adapter = lambda do |_user, **kwargs| + kwargs[:table] == 'answers' ? { enforced_predicates: { tenant_id: 42 } } : {} + end + + original_intent = { + 'table' => 'questions', + 'columns' => ['*'], + 'filters' => [related_filter('answers')], + 'limit' => 100, + 'params' => {} + } + + compiler.compile(original_intent) + + expect(original_intent).not_to have_key('__policy_expected_keys') + expect(original_intent['filters'].first).not_to have_key('__policy_expected_keys') end it 'keeps main-table and related-table policy binds distinct for the same column name' do @@ -850,13 +1240,38 @@ def compile_with_related_filters(table:, filters:, params: {}, current_user: nil subquery_key = 'policy_subquery_1_answers_tenant_id' expect(result[:sql]).to include('"answers"."tenant_id" = $1') - expect(result[:sql]).to include('"tenant_id" = $2') + expect(result[:sql]).to include('"questions"."tenant_id" = $2') expect(result[:params]['policy_tenant_id']).to eq(42) expect(result[:params][subquery_key]).to eq(7) expect(result[:bind_spec]).to include(hash_including(key: subquery_key, column: :tenant_id)) expect(result[:bind_spec]).to include(hash_including(key: 'policy_tenant_id', column: 'tenant_id')) end + it 'produces a safe EXISTS query with qualified base and related policy predicates' do + config.policy_adapter = lambda do |_user, **kwargs| + case kwargs[:table] + when 'questions' + { allowed_tables: %w[questions answers], enforced_predicates: { tenant_id: 42 } } + when 'answers' + { allowed_tables: ['answers'], enforced_predicates: { tenant_id: 7 } } + else + {} + end + end + allow(compiler).to receive(:use_arel?).and_return(true) + + result = compile_with_related_filters(table: 'questions', filters: [related_filter('answers')]) + query = CodeToQuery::Query.new( + sql: result[:sql], params: result[:params], bind_spec: result[:bind_spec], + intent: result[:intent], allow_tables: nil, config: config, + policy_contract: result[:policy_contract] + ) + + expect(result[:sql]).to include('"answers"."tenant_id" = $1') + expect(result[:sql]).to include('"questions"."tenant_id" = $2') + expect(query.safe?).to be true + end + it 'uses separate bind keys for multiple related-table policies on the same column name' do config.policy_adapter = lambda do |_user, **kwargs| case kwargs[:table] @@ -905,6 +1320,24 @@ def compile_with_related_filters(table:, filters:, params: {}, current_user: nil expect(result[:sql]).to include('"line_items"."account_id" = $1') expect(result[:params][subquery_key]).to eq(7) end + + it 'passes the parent intent when applying subquery policies' do + observed_intents = [] + config.policy_adapter = lambda do |_user, **kwargs| + observed_intents << kwargs[:intent] if kwargs[:table] == 'line_items' + + kwargs[:table] == 'line_items' ? { enforced_predicates: { account_id: 7 } } : {} + end + + result = compile_with_related_filters( + table: 'orders', + filters: [related_filter('line_items', fk_column: 'order_id')], + current_user: { account_id: 7 } + ) + + expect(observed_intents).to contain_exactly(include('table' => 'orders', 'filters' => [include('related_table' => 'line_items')])) + expect(result[:params]['policy_subquery_1_line_items_account_id']).to eq(7) + end end end end diff --git a/spec/code_to_query/guardrails/advanced_sql_linter_spec.rb b/spec/code_to_query/guardrails/advanced_sql_linter_spec.rb index f0f295a..a9034db 100644 --- a/spec/code_to_query/guardrails/advanced_sql_linter_spec.rb +++ b/spec/code_to_query/guardrails/advanced_sql_linter_spec.rb @@ -223,8 +223,7 @@ context 'with malformed SQL' do it 'handles missing closing quotes' do sql = "SELECT * FROM users WHERE name = 'unclosed LIMIT 10" - # This should not crash the linter - expect { linter.check!(sql) }.not_to raise_error + expect { linter.check!(sql) }.to raise_error(SecurityError, /Unterminated SQL literal/) end it 'handles unbalanced parentheses' do diff --git a/spec/code_to_query/guardrails/sql_linter_spec.rb b/spec/code_to_query/guardrails/sql_linter_spec.rb index 0c412ce..31f1407 100644 --- a/spec/code_to_query/guardrails/sql_linter_spec.rb +++ b/spec/code_to_query/guardrails/sql_linter_spec.rb @@ -17,6 +17,24 @@ sql = 'SELECT * FROM "users" WHERE "active" = $1 LIMIT 50' expect { linter.check!(sql) }.not_to raise_error end + + it 'passes an ordinary table identifier inside a parenthesized expression' do + sql = 'SELECT (table IS NULL) FROM users LIMIT 1' + + expect { linter.check!(sql) }.not_to raise_error + end + + it 'passes BETWEEN on an ordinary table identifier' do + sql = 'SELECT (table BETWEEN $1 AND $2) FROM users LIMIT 1' + + expect { linter.check!(sql) }.not_to raise_error + end + + it 'passes AT TIME ZONE on an ordinary table identifier' do + sql = 'SELECT (table AT TIME ZONE $1) FROM users LIMIT 1' + + expect { linter.check!(sql) }.not_to raise_error + end end context 'with dangerous queries' do @@ -74,10 +92,133 @@ expect { linter.check!(sql) }.to raise_error(SecurityError, /not in the allowed list/) end + it 'does not allow PostgreSQL ONLY to hide a non-allowlisted relation' do + only_linter = described_class.new(config, allow_tables: ['only']) + + expect { only_linter.check!('SELECT * FROM ONLY "admin_secrets" LIMIT 100') } + .to raise_error(SecurityError, /admin_secrets.*not in the allowed list/) + end + + it 'allows an allowlisted quoted relation after PostgreSQL ONLY' do + expect { linter.check!('SELECT * FROM ONLY "users" LIMIT 100') }.not_to raise_error + end + + it 'does not allow PostgreSQL JOIN ONLY to hide a non-allowlisted relation' do + only_linter = described_class.new(config, allow_tables: %w[users only]) + + expect { only_linter.check!('SELECT * FROM users JOIN ONLY admin_secrets ON TRUE LIMIT 100') } + .to raise_error(SecurityError, /admin_secrets.*not in the allowed list/) + end + it 'validates JOIN table allowlist' do sql = 'SELECT * FROM "users" JOIN "admin_secrets" ON users.id = admin_secrets.user_id LIMIT 100' expect { linter.check!(sql) }.to raise_error(SecurityError, /not in the allowed list/) end + + it 'rejects unsupported PostgreSQL TABLE query expressions inside subqueries' do + sql = 'SELECT * FROM "users" WHERE EXISTS (TABLE "admin_secrets") LIMIT 10' + + expect { linter.check!(sql) }.to raise_error(SecurityError, /TABLE query expressions are not supported/) + end + + it 'rejects TABLE query expressions used as nested set-operation operands' do + sql = 'SELECT * FROM "users" WHERE EXISTS (SELECT * FROM "users" UNION TABLE "admin_secrets") LIMIT 10' + + expect { linter.check!(sql) }.to raise_error(SecurityError, /TABLE query expressions are not supported/) + end + + it 'rejects TABLE query expressions with Unicode PostgreSQL identifiers' do + sql = 'SELECT * FROM "users" WHERE EXISTS (TABLE évil) LIMIT 10' + + expect { linter.check!(sql) }.to raise_error(SecurityError, /TABLE query expressions are not supported/) + end + + it 'rejects TABLE query expressions with PostgreSQL Unicode-escaped quoted identifiers' do + expressions = [ + 'SELECT EXISTS (TABLE U&"secrets") LIMIT 1', + %q(SELECT EXISTS (TABLE U&"s\0065crets") LIMIT 1), + 'SELECT EXISTS (TABLE U&"secr""ets") LIMIT 1' + ] + + expressions.each do |sql| + expect { linter.check!(sql) } + .to raise_error(SecurityError, /TABLE query expressions are not supported/), sql + end + end + + it 'rejects nested TABLE ONLY expressions across comments and whitespace' do + sql = "SELECT * FROM \"users\" WHERE EXISTS ((TABLE /* gap */ ONLY\n(évil))) LIMIT 10" + + expect { linter.check!(sql) }.to raise_error(SecurityError, /TABLE query expressions are not supported/) + end + + it 'rejects a TABLE query expression after a CTE definition' do + sql = 'SELECT * FROM "users" WHERE EXISTS (WITH q AS (SELECT 1) /* gap */ TABLE évil) LIMIT 10' + + expect { linter.check!(sql) }.to raise_error(SecurityError, /TABLE query expressions are not supported/) + end + + it 'rejects a TABLE query expression with an emoji PostgreSQL identifier' do + sql = 'SELECT EXISTS (WITH q AS (SELECT 1) TABLE 💣) LIMIT 1' + + expect { linter.check!(sql) }.to raise_error(SecurityError, /TABLE query expressions are not supported/) + end + + it 'does not treat table in identifiers, strings, or comments as a TABLE query expression' do + scanner = CodeToQuery::Query::SqlScanner.new + + expect(scanner.table_query_expression?('SELECT table FROM users')).to be false + expect(scanner.table_query_expression?('SELECT table.id FROM users AS table')).to be false + expect(scanner.table_query_expression?('SELECT users.table FROM users')).to be false + expect(scanner.table_query_expression?('SELECT 1 AS table FROM users')).to be false + expect(scanner.table_query_expression?('WITH q AS (SELECT 1) SELECT table FROM q AS table')).to be false + expect(scanner.table_query_expression?('SELECT "table" FROM users')).to be false + expect(scanner.table_query_expression?("SELECT 'TABLE évil' FROM users")).to be false + expect(scanner.table_query_expression?('SELECT 1 /* TABLE évil */ FROM users')).to be false + end + + context 'with MySQL identifiers' do + let(:config) { stub_config(adapter: :mysql, max_limit: 1000, max_joins: 2) } + + it 'rejects an unquoted case mismatch' do + expect { linter.check!('SELECT * FROM USERS LIMIT 100') } + .to raise_error(SecurityError, /not in the allowed list/) + end + + it 'rejects a quoted case mismatch' do + expect { linter.check!('SELECT * FROM `USERS` LIMIT 100') } + .to raise_error(SecurityError, /not in the allowed list/) + end + + it 'allows an exact-case unquoted table' do + expect { linter.check!('SELECT * FROM users LIMIT 100') }.not_to raise_error + end + + it 'allows an exact-case quoted table' do + expect { linter.check!('SELECT * FROM `users` LIMIT 100') }.not_to raise_error + end + + it 'rejects TABLE query expressions inside IN subqueries' do + sql = 'SELECT * FROM users WHERE id IN (TABLE admin_secrets) LIMIT 100' + + expect { linter.check!(sql) } + .to raise_error(SecurityError, /MySQL TABLE query expressions are not supported/) + end + + it 'rejects TABLE query expressions used as nested set-operation operands' do + sql = 'SELECT * FROM users WHERE EXISTS (SELECT * FROM users UNION TABLE admin_secrets) LIMIT 100' + + expect { linter.check!(sql) } + .to raise_error(SecurityError, /MySQL TABLE query expressions are not supported/) + end + + it 'rejects TABLE query expressions with backtick-quoted relations' do + sql = 'SELECT * FROM users WHERE EXISTS (TABLE `admin_secrets`) LIMIT 100' + + expect { linter.check!(sql) } + .to raise_error(SecurityError, /MySQL TABLE query expressions are not supported/) + end + end end context 'with JOIN complexity limits' do @@ -113,6 +254,204 @@ sql = 'SELECT * FROM information_schema.tables LIMIT 100' expect { linter.check!(sql) }.to raise_error(SecurityError, /information_schema/) end + + context 'with PostgreSQL server-side query execution' do + let(:config) { stub_config(adapter: :postgres, max_limit: 1000, max_joins: 2) } + + let(:xml_export_functions) do + %w[ + table_to_xml table_to_xmlschema table_to_xml_and_xmlschema + query_to_xml query_to_xmlschema query_to_xml_and_xmlschema + cursor_to_xml cursor_to_xmlschema + schema_to_xml schema_to_xmlschema schema_to_xml_and_xmlschema + database_to_xml database_to_xmlschema database_to_xml_and_xmlschema + ] + end + + it 'blocks query_to_xml from hiding a non-allowlisted table in dollar-quoted SQL' do + sql = 'SELECT query_to_xml($q$TABLE secrets$q$, true, false, $x$$x$) FROM users LIMIT 10' + + expect { linter.check!(sql) }.to raise_error(SecurityError, /query_to_xml/) + end + + it 'blocks query_to_xml schema-generation siblings' do + functions = %w[query_to_xmlschema query_to_xml_and_xmlschema] + + functions.each do |function| + sql = "SELECT #{function}($q$TABLE secrets$q$, true, false, $x$$x$) FROM users LIMIT 10" + + expect { linter.check!(sql) }.to raise_error(SecurityError, /#{function}/), function + end + end + + it 'blocks direct relation, schema, database, and cursor export calls with their real signatures' do + calls = [ + 'table_to_xml(to_regclass($1), true, false, $2)', + 'schema_to_xml($1, true, false, $2)', + 'database_to_xml(true, false, $1)', + 'cursor_to_xml($1, 100, true, false, $2)' + ] + + calls.each do |function_call| + sql = "SELECT #{function_call} FROM users LIMIT 10" + + expect { linter.check!(sql) }.to raise_error(SecurityError), function_call + end + end + + it 'blocks the complete built-in relation, query, cursor, schema, and database XML export family' do + xml_export_functions.each do |function| + sql = "SELECT #{function}($1) FROM users LIMIT 10" + + expect { linter.check!(sql) }.to raise_error(SecurityError, /#{function}/), function + end + end + + it 'blocks every XML export function across quoted and schema-qualified forms' do + xml_export_functions.each do |function| + [ + %("#{function}"), + "public.#{function}", + %(public."#{function}") + ].each do |function_call| + sql = "SELECT #{function_call}($1) FROM users LIMIT 10" + + expect { linter.check!(sql) }.to raise_error(SecurityError, /#{function}/), function_call + end + end + end + + it 'blocks every XML export function as a Unicode-escaped quoted identifier' do + xml_export_functions.each do |function| + encoded = function.sub('x', '!0078') + calls = [ + %(U&"#{function}"), + %(public.U&"#{encoded}" UESCAPE '!') + ] + + calls.each do |function_call| + sql = "SELECT #{function_call}($1) FROM users LIMIT 10" + + expect { linter.check!(sql) }.to raise_error(SecurityError, /#{function}/), function_call + end + end + end + + it 'blocks schema-qualified and quoted query_to_xml calls' do + calls = ['pg_catalog.query_to_xml', 'pg_catalog."query_to_xml"'] + + calls.each do |function_call| + sql = "SELECT #{function_call}($q$TABLE secrets$q$, true, false, $x$$x$) FROM users LIMIT 10" + + expect { linter.check!(sql) }.to raise_error(SecurityError), function_call + end + end + + it 'blocks Unicode-escaped quoted identifiers for every dynamic-query function' do + calls = { + 'query_to_xml' => 'U&"query_to_!0078ml" UESCAPE \'!\'', + 'query_to_xmlschema' => 'public.U&"query_to_xmlschema" UESCAPE \'!\'', + 'query_to_xml_and_xmlschema' => 'U&"query_to_!0078ml_and_xmlschema" UESCAPE \'!\'' + } + + calls.each do |function, function_call| + sql = "SELECT #{function_call}($q$TABLE secrets$q$, true, false, $x$$x$) FROM users LIMIT 10" + + expect { linter.check!(sql) }.to raise_error(SecurityError, /#{function}/), function_call + end + end + + it 'blocks Unicode identifiers with escape string UESCAPE clauses beside parameterized LIKE operators' do + %w[LIKE ILIKE].each do |operator| + sql = %(SELECT U&"query_to_!0078ml" UESCAPE E'!'($q$TABLE secrets$q$, true, false, $x$$x$) FROM users WHERE name #{operator} $3 LIMIT 10) + + expect { linter.check!(sql) }.to raise_error(SecurityError, /query_to_xml/), operator + end + end + + it 'accepts standard and escape-string UESCAPE forms for benign Unicode function identifiers' do + ["'!'", "E'!'", "e'!'"].each do |uescape| + sql = %(SELECT U&"safe_function" UESCAPE #{uescape}($1) FROM users WHERE name LIKE $2 LIMIT 10) + + expect { linter.check!(sql) }.not_to raise_error, uescape + end + end + + it 'fails closed when a Unicode function identifier UESCAPE is inconclusive' do + sql = %(SELECT U&"safe_function" UESCAPE E'!!'($1) FROM users WHERE name LIKE $2 LIMIT 10) + + expect { linter.check!(sql) }.to raise_error(SecurityError, /Inconclusive PostgreSQL Unicode/) + end + + it 'does not treat denied-looking calls in dollar-quoted literals as functions' do + calls = ['query_to_xml($1)', '"query_to_xml"($1)'] + + calls.each do |call| + sql = "SELECT $q$#{call}$q$ FROM users LIMIT 10" + + expect { linter.check!(sql) }.not_to raise_error, call + end + end + + it 'does not overmatch denied text after a doubled quote in a quoted identifier' do + sql = 'SELECT "prefix""query_to_xml"($1) FROM users LIMIT 10' + + expect { linter.check!(sql) }.not_to raise_error + end + + it 'does not overmatch denied text after a doubled quote in a Unicode quoted identifier' do + sql = 'SELECT U&"prefix""query_to_xml"($1) FROM users LIMIT 10' + + expect { linter.check!(sql) }.not_to raise_error + end + + it 'does not overmatch a longer Unicode quoted function identifier' do + sql = 'SELECT U&"query_to_xml_backup"($1) FROM users LIMIT 10' + + expect { linter.check!(sql) }.not_to raise_error + end + + it 'does not overmatch longer identifiers based on any XML export function name' do + xml_export_functions.each do |function| + calls = [ + "#{function}_backup", + %("#{function}_backup"), + "public.#{function}_backup", + %(public.U&"#{function}_backup") + ] + + calls.each do |function_call| + sql = "SELECT #{function_call}($1) FROM users LIMIT 10" + + expect { linter.check!(sql) }.not_to raise_error, function_call + end + end + end + + it 'does not scan denied-looking text inside an ordinary quoted identifier as an unquoted call' do + sql = 'SELECT "prefix query_to_xml ( suffix" FROM users LIMIT 10' + + expect { linter.check!(sql) }.not_to raise_error + end + + it 'does not scan denied-looking text inside a Unicode quoted identifier as an unquoted call' do + sql = 'SELECT U&"prefix query_to_xml ( suffix" FROM users LIMIT 10' + + expect { linter.check!(sql) }.not_to raise_error + end + end + + it 'does not apply the PostgreSQL server-side XML export denylist to other adapters' do + mysql_config = stub_config(adapter: :mysql, max_limit: 1000, max_joins: 2) + mysql_linter = described_class.new(mysql_config, allow_tables: %w[users]) + calls = ['query_to_xml', 'U&"query_to_\+000078ml"'] + + calls.each do |function_call| + sql = "SELECT #{function_call}($1, true, false, $2) FROM users LIMIT 10" + + expect { mysql_linter.check!(sql) }.not_to raise_error + end + end end end end diff --git a/spec/code_to_query/policy_adapter_invoker_spec.rb b/spec/code_to_query/policy_adapter_invoker_spec.rb new file mode 100644 index 0000000..f781e03 --- /dev/null +++ b/spec/code_to_query/policy_adapter_invoker_spec.rb @@ -0,0 +1,88 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe CodeToQuery::PolicyAdapterInvoker do + let(:current_user) { { id: 7 } } + let(:intent) { { 'table' => 'orders' } } + + def invoke(adapter) + described_class.call(adapter, current_user, table: 'orders', intent: intent) + end + + def adapter_forms(implementation) + proc_adapter = implementation + + method_owner = Object.new + method_owner.define_singleton_method(:call, &implementation) + + callable_class = Class.new do + define_method(:call, &implementation) + end + + { + proc: proc_adapter, + method: method_owner.method(:call), + callable_object: callable_class.new + } + end + + it 'passes table and intent to explicit keyword adapters in every callable form' do + implementation = proc { |user, table:, intent:| [user, table, intent] } + adapter_forms(implementation).each do |form, candidate| + expect(invoke(candidate)).to eq([current_user, 'orders', intent]), form.to_s + end + end + + it 'passes the complete context to keyrest adapters in every callable form' do + implementation = proc { |user, **context| [user, context] } + adapter_forms(implementation).each do |form, candidate| + expect(invoke(candidate)).to eq([current_user, { table: 'orders', intent: intent }]), form.to_s + end + end + + it 'passes context as a positional hash to rest adapters in every callable form' do + implementation = proc { |*args| args } + adapter_forms(implementation).each do |form, candidate| + expect(invoke(candidate)).to eq([current_user, { table: 'orders', intent: intent }]), form.to_s + end + end + + it 'passes context as a positional hash to legacy optional-options adapters in every callable form' do + implementation = proc { |user, options = {}| [user, options] } + adapter_forms(implementation).each do |form, candidate| + expect(invoke(candidate)).to eq([current_user, { table: 'orders', intent: intent }]), form.to_s + end + end + + it 'selects only table for table-only keyword adapters in every callable form' do + implementation = proc { |user, table:| [user, table] } + adapter_forms(implementation).each do |form, candidate| + expect(invoke(candidate)).to eq([current_user, 'orders']), form.to_s + end + end + + it 'passes only current user to user-only adapters in every callable form' do + implementation = proc { |user| user } + adapter_forms(implementation).each do |form, candidate| + expect(invoke(candidate)).to eq(current_user), form.to_s + end + end + + it 'invokes each adapter only once when it raises a signature-like ArgumentError internally' do + calls = Hash.new(0) + current_form = nil + + implementation = proc do |_user, table:, intent:| + calls[current_form] += 1 + raise ArgumentError, 'wrong number of arguments' if table == 'orders' && intent + end + + adapter_forms(implementation).each do |form, candidate| + current_form = form + + expect { invoke(candidate) }.to raise_error(ArgumentError, 'wrong number of arguments') + expect(calls[form]).to eq(1) + end + end +end diff --git a/spec/code_to_query/policy_metadata_casing_spec.rb b/spec/code_to_query/policy_metadata_casing_spec.rb new file mode 100644 index 0000000..c00186c --- /dev/null +++ b/spec/code_to_query/policy_metadata_casing_spec.rb @@ -0,0 +1,75 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'policy table metadata casing pipeline' do + let(:config) { stub_config(adapter: :postgres, default_limit: 100) } + let(:validator) { CodeToQuery::Validator.new } + let(:compiler) { CodeToQuery::Compiler.new(config) } + let(:policy_tables) { %w[AuditEvents auditevents] } + + before do + config.policy_adapter = ->(_user, **) { { allowed_tables: policy_tables } } + end + + after do + config.policy_adapter = nil + end + + def query_from_policy(table) + validated = validator.validate( + { 'type' => 'select', 'table' => table, 'columns' => ['*'] } + ).deep_stringify_keys + compiled = compiler.compile(validated) + query = CodeToQuery::Query.new( + sql: compiled[:sql], + params: compiled[:params], + bind_spec: compiled[:bind_spec], + intent: compiled[:intent], + allow_tables: nil, + config: config, + policy_contract: compiled[:policy_contract] + ) + + [validated, compiled, query] + end + + it 'preserves a deliberately cased quoted PostgreSQL table from policy through query safety' do + validated, compiled, query = query_from_policy('AuditEvents') + + expect(validated['__policy_allowed_tables']).to eq(policy_tables) + expect(compiled[:intent]['__policy_allowed_tables']).to eq(policy_tables) + expect(compiled[:sql]).to include('FROM "AuditEvents"') + expect(query.safe?).to be true + end + + it 'keeps a distinct lowercase PostgreSQL table authorized independently' do + validated, compiled, query = query_from_policy('auditevents') + + expect(validated['__policy_allowed_tables']).to eq(policy_tables) + expect(compiled[:intent]['__policy_allowed_tables']).to eq(policy_tables) + expect(compiled[:sql]).to include('FROM "auditevents"') + expect(query.safe?).to be true + end + + it 'does not authorize a lowercase related table from a deliberately cased policy entry' do + config.policy_adapter = lambda do |_user, **kwargs| + if kwargs[:table] == 'accounts' + { allowed_tables: %w[accounts AuditEvents] } + else + { allowed_tables: ['AuditEvents'] } + end + end + intent = { + 'type' => 'select', 'table' => 'accounts', 'columns' => ['*'], + 'filters' => [{ + 'op' => 'exists', 'related_table' => 'auditevents', + 'fk_column' => 'account_id', 'base_column' => 'id' + }] + } + validated = validator.validate(intent).deep_stringify_keys + + expect { compiler.compile(validated) } + .to raise_error(CodeToQuery::PolicyAdapterError, /does not allow related table: auditevents/) + end +end diff --git a/spec/code_to_query/query/sql_scanning_spec.rb b/spec/code_to_query/query/sql_scanning_spec.rb new file mode 100644 index 0000000..d8bd964 --- /dev/null +++ b/spec/code_to_query/query/sql_scanning_spec.rb @@ -0,0 +1,332 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe CodeToQuery::Query::SqlScanner do + subject(:scanner) { described_class.new } + + describe '#bind_placeholder_positions' do + it 'ignores placeholders in untagged PostgreSQL dollar-quoted literals' do + sql = 'SELECT $$spoof $1$$, $2' + + expect(scanner.bind_placeholder_positions(sql)).to eq([[sql.index('$2'), 2]]) + end + + it 'ignores placeholders in tagged PostgreSQL dollar-quoted literals' do + sql = 'SELECT $body$spoof $1 and ?$body$, $2' + + expect(scanner.bind_placeholder_positions(sql)).to eq([[sql.index('$2'), 2]]) + end + + it 'preserves placeholder offsets after dollar-quoted literals' do + sql = 'SELECT $tag$($9) and $other$ text$tag$, $1' + + expect(scanner.bind_placeholder_positions(sql)).to eq([[sql.index('$1'), 1]]) + end + + it 'fails closed for unterminated dollar-quoted literals' do + expect { scanner.bind_placeholder_positions('SELECT $tag$spoof $1') } + .to raise_error(SecurityError, 'Unterminated SQL literal or comment') + end + + it 'ignores question marks in quoted SQLite and MySQL identifiers' do + sql = 'SELECT "sqlite?name", `mysql?name`, ?' + + expect(scanner.bind_placeholder_positions(sql)).to eq([[sql.rindex('?'), 1]]) + end + + it 'honors backslash-escaped apostrophes in PostgreSQL E strings' do + %w[E e].each do |prefix| + sql = "SELECT #{prefix}'prefix\\' query_to_xml($1) suffix', $2" + + expect(scanner.bind_placeholder_positions(sql)).to eq([[sql.index('$2'), 2]]) + end + end + + it 'distinguishes ordinary strings from PostgreSQL E strings' do + sql = %q(SELECT 'ordinary\' , $1) + + expect(scanner.bind_placeholder_positions(sql)).to eq([[sql.index('$1'), 1]]) + end + end + + describe '#mask_literals_comments_and_identifier_contents' do + it 'masks denied-looking text in literals and quoted identifier bodies' do + sql = "SELECT E'prefix\\' query_to_xml($1)', \"query_to_xml ( suffix\", " \ + 'U&"query_to_xml ( decoy", query_to_xml($2)' + masked = scanner.mask_literals_comments_and_identifier_contents(sql) + + expect(masked.scan('query_to_xml').length).to eq(1) + expect(masked.index('query_to_xml')).to eq(sql.rindex('query_to_xml')) + end + end + + describe '#exists_subqueries' do + it 'ignores parentheses inside dollar-quoted literals when finding the boundary' do + sql = 'SELECT 1 WHERE EXISTS (SELECT $$)$$ FROM "answers")' + + expect(scanner.exists_subqueries(sql).first[:sql]).to eq('SELECT $$)$$ FROM "answers"') + end + + it 'does not let EXISTS text in a quoted identifier swallow a real subquery' do + sql = 'SELECT 1 AS "x EXISTS (" WHERE EXISTS (SELECT 1 FROM "answers")' + + expect(scanner.exists_subqueries(sql).map { |entry| entry[:sql] }) + .to eq(['SELECT 1 FROM "answers"']) + expect(scanner.strip_exists_subqueries(sql)).to eq('SELECT 1 AS "x EXISTS (" WHERE TRUE') + end + end + + describe '#table_query_expression?' do + it 'does not mistake an ordinary table identifier in an expression for a TABLE query' do + expect(scanner.table_query_expression?('SELECT (table IS NULL) FROM users LIMIT 1')).to be false + expect(scanner.table_query_expression?('SELECT (table = $1) FROM users LIMIT 1')).to be false + expect(scanner.table_query_expression?('SELECT (table NOT IN ($1)), table.id FROM users AS table LIMIT 1')).to be false + expect(scanner.table_query_expression?('SELECT table($1) FROM users LIMIT 1')).to be false + end + + it 'uses the relation operand boundary to reject expression operators' do + expressions = [ + 'table BETWEEN $1 AND $2', + 'table AT TIME ZONE $1', + 'table IS DISTINCT FROM $1', + 'table = $1', + 'table <> $1', + 'table IN ($1)', + 'table NOT IN ($1)', + 'table + $1', + 'table::text' + ] + + expressions.each do |expression| + expect(scanner.table_query_expression?("SELECT (#{expression}) FROM users LIMIT 1")).to be(false), expression + end + end + + it 'still recognizes relation identifiers in TABLE query expressions' do + expect(scanner.table_query_expression?('SELECT EXISTS (TABLE "IS")')).to be true + expect(scanner.table_query_expression?('SELECT EXISTS (TABLE /* gap */ ONLY (💣))')).to be true + expect(scanner.table_query_expression?('SELECT EXISTS (WITH q AS (SELECT 1) TABLE évil)')).to be true + expect(scanner.table_query_expression?('SELECT EXISTS (SELECT 1 UNION ALL TABLE schema_name.relation_name)')).to be true + end + + it 'recognizes PostgreSQL Unicode-escaped quoted relation identifiers' do + expressions = [ + 'SELECT EXISTS (TABLE U&"secrets") LIMIT 1', + %q(SELECT EXISTS (TABLE U&"s\0065crets") LIMIT 1), + 'SELECT EXISTS (TABLE U&"schema".U&"secr""ets") LIMIT 1', + %q(SELECT EXISTS (TABLE U&"s!0065crets" UESCAPE '!') LIMIT 1) + ] + + expressions.each do |expression| + expect(scanner.table_query_expression?(expression)).to be(true), expression + end + end + + it 'does not combine spaced U, ampersand, and quoted identifier tokens' do + expect(scanner.table_query_expression?('SELECT (table U & "secrets") FROM users LIMIT 1')).to be false + expect(scanner.table_query_expression?('SELECT (table U& "secrets") FROM users LIMIT 1')).to be false + end + + it 'recognizes valid TABLE relation completion and query-level continuation' do + expressions = [ + 'TABLE accounts', + 'TABLE ONLY (archive.accounts) *', + '(TABLE accounts)', + 'TABLE accounts UNION SELECT * FROM users', + 'TABLE accounts INTERSECT ALL TABLE active_accounts', + 'TABLE accounts EXCEPT TABLE archived_accounts', + 'TABLE accounts ORDER BY id', + 'TABLE accounts LIMIT 1', + 'TABLE accounts OFFSET 1', + 'TABLE accounts FETCH FIRST 1 ROW ONLY', + 'TABLE accounts FOR UPDATE' + ] + + expressions.each do |expression| + expect(scanner.table_query_expression?(expression)).to be(true), expression + end + end + + it 'recognizes MySQL backtick-quoted TABLE relations when configured for MySQL' do + mysql_scanner = described_class.new(adapter: :mysql) + + expect(mysql_scanner.table_query_expression?('SELECT EXISTS (TABLE `admin_secrets`) LIMIT 1')).to be true + expect(mysql_scanner.table_query_expression?('SELECT EXISTS (TABLE `schema`.`admin_secrets`) LIMIT 1')).to be true + end + end + + describe '#policy_predicate_bind_numbers' do + it 'derives SQLite and MySQL question-mark bind identity from the global SQL position' do + full_sql = 'SELECT ? FROM questions WHERE EXISTS ' \ + '(SELECT ? FROM answers WHERE answers.kind = ? AND answers.tenant_id = ?)' + body_start = full_sql.index('SELECT ?', full_sql.index('EXISTS')) + body = full_sql[body_start...full_sql.rindex(')')] + + expect(scanner.policy_predicate_bind_numbers( + body, 'answers', 'tenant_id', + question_bind_number: 3, source_sql: full_sql, source_offset: body_start + )).to eq([]) + expect(scanner.policy_predicate_bind_numbers( + body, 'answers', 'tenant_id', + question_bind_number: 4, source_sql: full_sql, source_offset: body_start + )).to eq([4]) + end + + it 'ignores question marks in literals, comments, and dollar quotes when deriving identity' do + full_sql = "SELECT '?', $$?$$, ? /* ? */ WHERE EXISTS " \ + '(SELECT 1 FROM answers WHERE answers.tenant_id = ?)' + body_start = full_sql.index('SELECT 1') + body = full_sql[body_start...full_sql.rindex(')')] + + expect(scanner.policy_predicate_bind_numbers( + body, 'answers', 'tenant_id', + question_bind_number: 2, source_sql: full_sql, source_offset: body_start + )).to eq([2]) + end + + it 'accepts binds only in the expected qualified predicate' do + expect(scanner.policy_predicate_bind_numbers('SELECT $1 FROM "answers" WHERE TRUE', 'answers', 'tenant_id')).to eq([]) + expect(scanner.policy_predicate_bind_numbers('SELECT "answers"."tenant_id" = $1 FROM "answers" WHERE TRUE', 'answers', 'tenant_id')).to eq([]) + expect(scanner.policy_predicate_bind_numbers('SELECT 1 FROM "answers" WHERE $1 = $1', 'answers', 'tenant_id')).to eq([]) + expect(scanner.policy_predicate_bind_numbers('SELECT 1 FROM "answers" WHERE "answers"."tenant_id" = $1 OR TRUE', 'answers', 'tenant_id')).to eq([]) + expect(scanner.policy_predicate_bind_numbers('SELECT 1 FROM "answers" WHERE "answers"."tenant_id" = $1', 'answers', 'tenant_id')).to eq([1]) + end + + it 'rejects predicates in non-enforcing boolean and expression contexts' do + expect(scanner.policy_predicate_bind_numbers( + 'SELECT 1 FROM "answers" WHERE NOT ("answers"."tenant_id" = $1)', 'answers', 'tenant_id' + )).to eq([]) + expect(scanner.policy_predicate_bind_numbers( + 'SELECT 1 FROM "answers" WHERE CASE WHEN "answers"."tenant_id" = $1 THEN TRUE ELSE TRUE END', + 'answers', 'tenant_id' + )).to eq([]) + end + + it 'requires identifier boundaries for unquoted policy columns' do + expect(scanner.policy_predicate_bind_numbers( + 'SELECT 1 FROM answers WHERE notanswers.tenant_id = $1', 'answers', 'tenant_id' + )).to eq([]) + expect(scanner.policy_predicate_bind_numbers( + 'SELECT 1 FROM answers WHERE answers.tenant_id_suffix = $1', 'answers', 'tenant_id' + )).to eq([]) + end + + it 'requires Unicode identifier boundaries for unquoted policy identifiers' do + expect(scanner.policy_predicate_bind_numbers( + 'SELECT 1 FROM answers WHERE éanswers.tenant_id = $1', 'answers', 'tenant_id' + )).to eq([]) + expect(scanner.policy_predicate_bind_numbers( + 'SELECT 1 FROM answers WHERE answersé.tenant_id = $1', 'answers', 'tenant_id' + )).to eq([]) + expect(scanner.policy_predicate_bind_numbers( + 'SELECT 1 FROM answers WHERE answers.étenant_id = $1', 'answers', 'tenant_id' + )).to eq([]) + expect(scanner.policy_predicate_bind_numbers( + 'SELECT 1 FROM answers WHERE answers.tenant_idé = $1', 'answers', 'tenant_id' + )).to eq([]) + end + + it 'does not count a predicate placed in a nested scope' do + sql = 'SELECT 1 FROM "answers" WHERE EXISTS ' \ + '(SELECT 1 FROM "decoys" WHERE "answers"."tenant_id" = $1)' + + expect(scanner.policy_predicate_bind_numbers(sql, 'answers', 'tenant_id')).to eq([]) + end + + it 'does not Unicode-case-fold policy identifiers on case-insensitive adapters' do + %i[sqlite mysql].each do |adapter| + sql = 'SELECT 1 FROM kids WHERE kids.Kind = $1' + + expect(scanner.policy_predicate_bind_numbers(sql, 'kids', 'kind', adapter: adapter)).to eq([]) + end + end + + it 'limits PostgreSQL unquoted policy identifier folding to ASCII' do + sql = 'SELECT 1 FROM kids WHERE KIDS.KIND = $1' + + expect(scanner.policy_predicate_bind_numbers(sql, 'kids', 'kind', adapter: :postgres)).to eq([]) + expect(scanner.policy_predicate_bind_numbers( + 'SELECT 1 FROM kids WHERE KIDS.KIND = $1', 'kids', 'kind', adapter: :postgres + )).to eq([1]) + end + + it 'accepts compiler-shaped top-level conjuncts including BETWEEN' do + sql = 'SELECT 1 FROM answers WHERE answers.question_id = questions.id ' \ + 'AND answers.created_at BETWEEN $2 AND $3' + + expect(scanner.policy_predicate_bind_numbers(sql, 'answers', 'created_at')).to eq([2, 3]) + end + end + + describe '#extract_table_names' do + it 'treats PostgreSQL ONLY as a relation modifier' do + postgres_scanner = described_class.new(adapter: :postgres) + + expect(postgres_scanner.extract_table_names('SELECT * FROM ONLY admin_secrets')).to eq(['admin_secrets']) + end + + it 'extracts a quoted relation after PostgreSQL ONLY' do + postgres_scanner = described_class.new(adapter: :postgres) + + expect(postgres_scanner.extract_table_names('SELECT * FROM ONLY "Admin Secrets"')).to eq(['Admin Secrets']) + end + + it 'treats PostgreSQL ONLY as a JOIN relation modifier' do + postgres_scanner = described_class.new(adapter: :postgres) + + expect(postgres_scanner.extract_table_names('SELECT * FROM users JOIN ONLY admin_secrets ON TRUE')) + .to eq(%w[users admin_secrets]) + end + + it 'keeps ONLY as the relation name on adapters where it is not a modifier' do + %i[mysql sqlite].each do |adapter| + adapter_scanner = described_class.new(adapter: adapter) + + expect(adapter_scanner.extract_table_names('SELECT * FROM ONLY admin_secrets')).to eq(['ONLY']) + expect(adapter_scanner.extract_table_names('SELECT * FROM users JOIN ONLY admin_secrets ON TRUE')) + .to eq(%w[users ONLY]) + end + end + + it 'fails closed for qualified table references instead of discarding the qualifier' do + expect { scanner.extract_table_names('SELECT * FROM evil.users') } + .to raise_error(SecurityError, /Unsupported FROM reference/) + end + + it 'does not let identifier quotes inside string literals hide executable table references' do + expect(scanner.extract_table_names(%q(SELECT '"' FROM"forbidden"))).to eq(['forbidden']) + expect(scanner.extract_table_names("SELECT '`' FROM`forbidden`")).to eq(['forbidden']) + end + + it 'does not let a string quote inside a quoted identifier hide a later table reference' do + expect(scanner.extract_table_names(%q(SELECT 1 AS "a'b" FROM "forbidden"))).to eq(['forbidden']) + end + + it 'masks doubled delimiters inside a quoted identifier without hiding a later table reference' do + expect(scanner.extract_table_names('SELECT 1 AS "a""b" FROM "forbidden"')).to eq(['forbidden']) + end + + it 'does not let identifier quotes inside dollar-quoted literals hide executable table references' do + sql = 'SELECT $body$`$body$ FROM "allowed" JOIN"forbidden" ON TRUE' + + expect(scanner.extract_table_names(sql)).to eq(%w[allowed forbidden]) + end + + it 'extracts quoted FROM and JOIN identifiers without intervening whitespace' do + sql = 'SELECT * FROM"questions" JOIN"answers" ON "answers"."question_id" = "questions"."id"' + + expect(scanner.extract_table_names(sql)).to eq(%w[questions answers]) + end + + it 'does not treat keyword text inside quoted identifiers as SQL structure' do + sql = 'SELECT * FROM "questions" AS "JOIN decoy" JOIN"answers WHERE decoy" ON TRUE' + + expect(scanner.extract_table_names(sql)).to eq(['questions', 'answers WHERE decoy']) + end + + it 'does not split a FROM reference on a comma inside a quoted identifier' do + expect(scanner.extract_table_names('SELECT * FROM "questions, archived"')).to eq(['questions, archived']) + end + end +end diff --git a/spec/code_to_query/query_spec.rb b/spec/code_to_query/query_spec.rb index e146eca..d1d14fd 100644 --- a/spec/code_to_query/query_spec.rb +++ b/spec/code_to_query/query_spec.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require 'spec_helper' +require 'weakref' RSpec.describe CodeToQuery::Query do let(:config) { stub_config(adapter: :postgres) } @@ -19,78 +20,1512 @@ ) end + def build_scope_backed_user_model(scope) + stub_const('ActiveRecord', Module.new) unless defined?(ActiveRecord) + stub_const('ActiveRecord::Base', Class.new) + + user_model = Class.new(ActiveRecord::Base) do + def self.table_name = 'users' + def self.all = @scope + + class << self + attr_writer :scope + end + end + + user_model.scope = scope + stub_const('User', user_model) + end + + def build_scope_backed_question_model(scope) + stub_const('ActiveRecord', Module.new) unless defined?(ActiveRecord) + stub_const('ActiveRecord::Base', Class.new) unless defined?(ActiveRecord::Base) + + question_model = Class.new(ActiveRecord::Base) do + def self.table_name = 'questions' + def self.all = @scope + + class << self + attr_writer :scope + end + end + + question_model.scope = scope + stub_const('Question', question_model) + end + + def build_policy_query(config) + described_class.new( + sql: 'SELECT * FROM "users" WHERE "active" = $1 AND "tenant_id" = $2 LIMIT 100', + params: { 'active' => true, 'policy_tenant_id' => 42 }, + bind_spec: [ + { key: 'active', column: 'active', cast: nil }, + { key: 'policy_tenant_id', column: 'tenant_id', cast: nil } + ], + intent: { + 'table' => 'users', + 'type' => 'select', + 'filters' => [ + { 'column' => 'active', 'op' => '=', 'param' => 'active' }, + { 'column' => 'tenant_id', 'op' => '=', 'param' => 'policy_tenant_id' } + ], + 'limit' => 100 + }, + allow_tables: ['users'], + config: config + ) + end + + def build_subquery_policy_query(config) + described_class.new( + sql: 'SELECT * FROM "questions" WHERE EXISTS (SELECT 1 FROM "answers" WHERE "answers"."tenant_id" = $1)', + params: { 'policy_subquery_1_answers_tenant_id' => 42 }, + bind_spec: [{ key: 'policy_subquery_1_answers_tenant_id', column: 'tenant_id', cast: nil }], + intent: { + 'table' => 'questions', + 'type' => 'select', + 'filters' => [ + { + 'column' => 'id', + 'op' => 'exists', + 'related_table' => 'answers', + 'fk_column' => 'question_id', + 'base_column' => 'id', + 'related_filters' => [] + } + ], + '__policy_expected_keys' => ['policy_subquery_1_answers_tenant_id'] + }, + allow_tables: ['questions'], + config: config + ) + end + + def build_compiled_policy_query(config, policy) + config.policy_adapter = ->(_user, **) { policy } + compiled = CodeToQuery::Compiler.new(config).compile( + { 'table' => 'users', 'type' => 'select', 'columns' => ['*'], 'limit' => 100 }, + allow_tables: ['users'] + ) + described_class.new( + sql: compiled[:sql], params: compiled[:params], bind_spec: compiled[:bind_spec], + intent: compiled[:intent], allow_tables: ['users'], config: config, + policy_contract: compiled[:policy_contract] + ) + end + describe '#sql' do it 'returns the SQL string' do expect(query.sql).to eq(sql) end - end - describe '#params' do - it 'returns the parameters hash' do - expect(query.params).to eq(params) + it 'keeps internal SQL isolated from constructor and accessor mutations' do + mutable_sql = sql.dup + q = described_class.new( + sql: mutable_sql, params: params, bind_spec: bind_spec, intent: intent, + allow_tables: ['users'], config: config + ) + expect(q.safe?).to be true + + mutable_sql.replace('DROP TABLE users') + returned_sql = q.sql + returned_sql.replace('DROP TABLE users') + + expect(q.sql).to eq(sql) + end + end + + describe '#params' do + it 'returns the parameters hash' do + expect(query.params).to eq(params) + end + + it 'keeps internal nested parameters isolated from constructor and accessor mutations' do + nested_params = { 'filters' => [{ 'value' => 'active'.dup }] } + q = described_class.new( + sql: sql, params: nested_params, bind_spec: [], intent: intent, + allow_tables: ['users'], config: config + ) + + nested_params['filters'].first['value'].replace('tampered') + returned_params = q.params + returned_params['filters'].first['value'].replace('tampered') + + expect(q.params.dig('filters', 0, 'value')).to eq('active') + end + + it 'adds between defaults derived from column names for legacy start/end params' do + q = described_class.new( + sql: 'SELECT * FROM "orders" WHERE "created_at" BETWEEN $1 AND $2', + params: { 'start' => '2023-01-01', 'end' => '2023-12-31' }, + bind_spec: [{ key: 'created_at_start', column: 'created_at' }, { key: 'created_at_end', column: 'created_at' }], + intent: { + 'table' => 'orders', + 'type' => 'select', + 'filters' => [ + { 'column' => 'created_at', 'op' => 'between' } + ] + }, + allow_tables: ['orders'], + config: config + ) + + expect(q.params['created_at_start']).to eq('2023-01-01') + expect(q.params['created_at_end']).to eq('2023-12-31') + end + + it 'adds between defaults derived from column names for legacy symbol-key params' do + q = described_class.new( + sql: 'SELECT * FROM "orders" WHERE "created_at" BETWEEN $1 AND $2', + params: { start: '2023-01-01', end: '2023-12-31' }, + bind_spec: [{ key: 'created_at_start', column: 'created_at' }, { key: 'created_at_end', column: 'created_at' }], + intent: { + 'table' => 'orders', + 'type' => 'select', + 'filters' => [ + { 'column' => 'created_at', 'op' => 'between' } + ] + }, + allow_tables: ['orders'], + config: config + ) + + expect(q.params['created_at_start']).to eq('2023-01-01') + expect(q.params['created_at_end']).to eq('2023-12-31') + end + + it 'preserves false-valued legacy between params when deriving column-based keys' do + q = described_class.new( + sql: 'SELECT * FROM "orders" WHERE "created_at" BETWEEN $1 AND $2', + params: { 'start' => false, 'end' => true }, + bind_spec: [{ key: 'created_at_start', column: 'created_at' }, { key: 'created_at_end', column: 'created_at' }], + intent: { + 'table' => 'orders', + 'type' => 'select', + 'filters' => [ + { 'column' => 'created_at', 'op' => 'between' } + ] + }, + allow_tables: ['orders'], + config: config + ) + + expect(q.params['created_at_start']).to be(false) + expect(q.params['created_at_end']).to be(true) + end + + it 'hydrates only the implicit side when one bound keeps an explicit param name' do + q = described_class.new( + sql: 'SELECT * FROM "orders" WHERE "created_at" BETWEEN $1 AND $2', + params: { 'end' => '2023-12-31' }, + bind_spec: [{ key: 'from_date', column: 'created_at' }, { key: 'created_at_end', column: 'created_at' }], + intent: { + 'table' => 'orders', + 'type' => 'select', + 'filters' => [ + { 'column' => 'created_at', 'op' => 'between', 'param_start' => 'from_date' } + ] + }, + allow_tables: ['orders'], + config: config + ) + + expect(q.params['created_at_end']).to eq('2023-12-31') + expect(q.params).not_to have_key('created_at_start') + end + end + + describe '#intent' do + it 'keeps internal nested intent isolated from constructor and accessor mutations' do + mutable_intent = { + 'table' => 'users'.dup, + 'type' => 'select', + 'filters' => [{ 'column' => 'active'.dup, 'op' => '=' }] + } + q = described_class.new( + sql: sql, params: params, bind_spec: bind_spec, intent: mutable_intent, + allow_tables: ['users'], config: config + ) + + mutable_intent['filters'].first['column'].replace('admin') + returned_intent = q.intent + returned_intent['filters'].first['column'].replace('admin') + + expect(q.intent.dig('filters', 0, 'column')).to eq('active') + end + end + + describe '#binds' do + it 'preserves false values stored under string keys' do + mock_connection = double('Connection') + ar_base = Class.new do + def self.connection = @mock_connection + + class << self + attr_writer :mock_connection + end + end + ar_base.mock_connection = mock_connection + stub_const('ActiveRecord::Base', ar_base) + stub_const('ActiveRecord::Relation::QueryAttribute', Struct.new(:name, :value, :type)) + + q = described_class.new( + sql: 'SELECT * FROM "users" WHERE "archived" = $1', + params: { 'archived' => false }, + bind_spec: [{ key: 'archived', column: 'archived', cast: nil }], + intent: { 'table' => 'users', 'type' => 'select' }, + allow_tables: ['users'], + config: config + ) + allow(q).to receive(:infer_column_type).and_return(:boolean) + + binds = q.binds + + expect(binds.first.value).to be(false) + end + end + + describe 'false-valued param lookups' do + it 'preserves false values when applying equality filters to a scope' do + scoped_query = described_class.new( + sql: 'SELECT * FROM "users" WHERE "archived" = $1', + params: { 'archived' => false }, + bind_spec: [], + intent: { + 'table' => 'users', + 'type' => 'select', + 'filters' => [{ 'column' => 'archived', 'op' => '=' }] + }, + allow_tables: ['users'], + config: config + ) + scope = spy('scope') + + scoped_query.send(:apply_filter_to_scope, scope, { 'column' => 'archived', 'op' => '=' }) + + expect(scope).to have_received(:where).with('archived' => false) + end + + it 'does not fall back to legacy start when between start uses an explicit param name' do + scoped_query = described_class.new( + sql: 'SELECT * FROM "orders" WHERE "created_at" BETWEEN $1 AND $2', + params: { 'start' => false, 'created_at_end' => '2023-12-31' }, + bind_spec: [], + intent: { + 'table' => 'orders', + 'type' => 'select', + 'filters' => [{ 'column' => 'created_at', 'op' => 'between', 'param_start' => 'from_date' }] + }, + allow_tables: ['orders'], + config: config + ) + scope = spy('scope') + + scoped_query.send(:apply_filter_to_scope, scope, { 'column' => 'created_at', 'op' => 'between', 'param_start' => 'from_date' }) + + expect(scope).to have_received(:where).with('created_at' => (nil..'2023-12-31')) + end + + it 'infers boolean type from false values stored under custom bind keys' do + typed_query = described_class.new( + sql: 'SELECT * FROM "users" WHERE "archived" = $1', + params: { 'is_archived' => false }, + bind_spec: [], + intent: { 'table' => 'users', 'type' => 'select' }, + allow_tables: ['users'], + config: config + ) + + hide_const('ActiveRecord::Base') + + inferred_type = typed_query.send(:infer_column_type, nil, nil, 'archived', nil, 'is_archived') + expect(inferred_type).to be_a(ActiveRecord::Type::Boolean) + end + end + + describe '#safe?' do + context 'with valid query' do + it 'returns true for safe queries' do + q = described_class.new( + sql: sql, + params: params, + bind_spec: bind_spec, + intent: intent, + allow_tables: ['users'], + config: config + ) + allow(q).to receive(:perform_safety_checks).and_return(true) + + expect(q.safe?).to be true + end + end + + context 'with unsafe query' do + let(:sql) { 'DROP TABLE users' } + + it 'returns false for unsafe queries' do + expect(query.safe?).to be false + end + end + + it 'caches the safety check result' do + q = described_class.new( + sql: sql, + params: params, + bind_spec: bind_spec, + intent: intent, + allow_tables: ['users'], + config: config + ) + allow(q).to receive(:perform_safety_checks).and_return(true) + + q.safe? + q.safe? + + expect(q).to have_received(:perform_safety_checks).once + end + + it 'returns true for a compiler-verified allowlist-only policy with no injected predicates' do + config.policy_adapter = ->(_user, **) { { allowed_tables: ['users'] } } + compiled = CodeToQuery::Compiler.new(config).compile( + { 'table' => 'users', 'type' => 'select', 'columns' => ['*'], 'limit' => 100 }, + allow_tables: ['users'] + ) + q = described_class.new( + sql: compiled[:sql], params: compiled[:params], bind_spec: compiled[:bind_spec], + intent: compiled[:intent], allow_tables: ['users'], config: config, + policy_contract: compiled[:policy_contract] + ) + + expect(q.safe?).to be true + ensure + config.policy_adapter = nil + end + + it 'requires opaque compiler evidence for nonempty policy keys' do + config.policy_adapter = ->(_user, **) { { enforced_predicates: { tenant_id: 42 } } } + q = described_class.new( + sql: 'SELECT * FROM "users" WHERE "users"."tenant_id" = $1 LIMIT 100', + params: { 'policy_tenant_id' => 42 }, + bind_spec: [{ key: 'policy_tenant_id', column: 'tenant_id', cast: nil }], + intent: { + 'table' => 'users', 'type' => 'select', + '__policy_expected_keys' => ['policy_tenant_id'] + }, + allow_tables: ['users'], config: config + ) + + expect(q.safe?).to be false + ensure + config.policy_adapter = nil + end + + it 'accepts a compiler-produced nonempty policy contract' do + config.policy_adapter = ->(_user, **) { { enforced_predicates: { tenant_id: 42 } } } + compiled = CodeToQuery::Compiler.new(config).compile( + { 'table' => 'users', 'type' => 'select', 'columns' => ['*'], 'limit' => 100 }, + allow_tables: ['users'] + ) + q = described_class.new( + sql: compiled[:sql], params: compiled[:params], bind_spec: compiled[:bind_spec], + intent: compiled[:intent], allow_tables: ['users'], config: config, + policy_contract: compiled[:policy_contract] + ) + + expect(q.safe?).to be true + ensure + config.policy_adapter = nil + end + + it 'keeps nonempty policy evidence alive while the query is alive across forced GC' do + q = build_compiled_policy_query(config, enforced_predicates: { tenant_id: 42 }) + + GC.start(full_mark: true, immediate_sweep: true) + + expect(q.safe?).to be true + ensure + config.policy_adapter = nil + end + + it 'keeps allowlist-only policy evidence alive while the query is alive across forced GC' do + q = build_compiled_policy_query(config, allowed_tables: ['users'], enforced_predicates: {}) + + GC.start(full_mark: true, immediate_sweep: true) + + expect(q.safe?).to be true + ensure + config.policy_adapter = nil + end + + it 'retains adapter identity for the contract lifetime and rejects a replacement after GC churn' do + issuing_adapter_ref = WeakRef.new( + config.policy_adapter = ->(_user, **) { { allowed_tables: ['users'] } } + ) + compiled = CodeToQuery::Compiler.new(config).compile( + { 'table' => 'users', 'type' => 'select', 'columns' => ['*'], 'limit' => 100 }, + allow_tables: ['users'] + ) + attributes = { + sql: compiled[:sql], params: compiled[:params], bind_spec: compiled[:bind_spec], + intent: compiled[:intent], allow_tables: ['users'], config: config, + policy_contract: compiled[:policy_contract] + } + + config.policy_adapter = ->(_user, **) { { allowed_tables: ['users'] } } + 50_000.times { Object.new } + GC.start(full_mark: true, immediate_sweep: true) + + expect(issuing_adapter_ref).to be_weakref_alive + expect(described_class.new(**attributes).safe?).to be false + + config.policy_adapter = issuing_adapter_ref.__getobj__ + expect(described_class.new(**attributes).safe?).to be true + ensure + config.policy_adapter = nil + end + + it 'rejects forged and replayed policy capabilities' do + config.policy_adapter = ->(_user, **) { { allowed_tables: ['users'] } } + compiled = CodeToQuery::Compiler.new(config).compile( + { 'table' => 'users', 'type' => 'select', 'columns' => ['*'], 'limit' => 100 }, + allow_tables: ['users'] + ) + attributes = { + sql: compiled[:sql], params: compiled[:params], bind_spec: compiled[:bind_spec], + intent: compiled[:intent], allow_tables: ['users'], config: config + } + forged = described_class.new(**attributes, policy_contract: Object.new) + replayed = described_class.new( + **attributes, sql: "#{compiled[:sql]} ", policy_contract: compiled[:policy_contract] + ) + + expect(forged.safe?).to be false + expect(replayed.safe?).to be false + ensure + config.policy_adapter = nil + end + + it 'binds policy evidence to params, intent, and explicit allowlists' do + config.policy_adapter = ->(_user, **) { { enforced_predicates: { tenant_id: 42 } } } + compiled = CodeToQuery::Compiler.new(config).compile( + { 'table' => 'users', 'type' => 'select', 'columns' => ['*'], 'limit' => 100 }, + allow_tables: ['users'] + ) + attributes = { + sql: compiled[:sql], params: compiled[:params], bind_spec: compiled[:bind_spec], + intent: compiled[:intent], allow_tables: ['users'], config: config, + policy_contract: compiled[:policy_contract] + } + changed_params = described_class.new( + **attributes, params: compiled[:params].merge('policy_tenant_id' => 7) + ) + changed_binds = described_class.new( + **attributes, bind_spec: compiled[:bind_spec].map { |bind| bind.merge(cast: :tampered) } + ) + changed_intent = described_class.new(**attributes, intent: compiled[:intent].merge('limit' => 99)) + changed_allowlist = described_class.new(**attributes, allow_tables: %w[users admins]) + + expect(changed_params.safe?).to be false + expect(changed_binds.safe?).to be false + expect(changed_intent.safe?).to be false + expect(changed_allowlist.safe?).to be false + ensure + config.policy_adapter = nil + end + + it 'does not expose a publicly constructible policy contract class' do + expect { CodeToQuery::Compiler::PolicyContract }.to raise_error(NameError) + end + + it 'fails closed when a directly constructed query has no compiler policy contract' do + config.policy_adapter = ->(_user, **) { { enforced_predicates: { tenant_id: 42 } } } + + expect(query.safe?).to be false + ensure + config.policy_adapter = nil + end + + it 'fails closed when policy enforcement is enabled after compilation' do + compiled = CodeToQuery::Compiler.new(config).compile( + { 'table' => 'users', 'type' => 'select', 'columns' => ['*'], 'limit' => 100 }, + allow_tables: ['users'] + ) + config.policy_adapter = ->(_user, **) { { enforced_predicates: { tenant_id: 42 } } } + q = described_class.new( + sql: compiled[:sql], params: compiled[:params], bind_spec: compiled[:bind_spec], + intent: compiled[:intent], allow_tables: ['users'], config: config, + policy_contract: compiled[:policy_contract] + ) + + expect(q.safe?).to be false + ensure + config.policy_adapter = nil + end + + it 'rejects a differently cased quoted PostgreSQL identifier from a lowercase allowlist' do + q = described_class.new( + sql: 'SELECT * FROM "USERS" LIMIT 100', params: {}, bind_spec: [], + intent: { 'table' => 'users', 'type' => 'select' }, allow_tables: ['users'], config: config + ) + + expect(q.safe?).to be false + end + + it 'accepts uppercase unquoted PostgreSQL and SQLite identifiers from a lowercase allowlist' do + %i[postgres sqlite].each do |adapter| + q = described_class.new( + sql: 'SELECT * FROM USERS LIMIT 100', params: {}, bind_spec: [], + intent: { 'table' => 'users', 'type' => 'select' }, allow_tables: ['users'], + config: stub_config(adapter: adapter) + ) + + expect(q.safe?).to be(true), "expected #{adapter} unquoted identifier folding to be honored" + end + end + + it 'rejects a differently cased unquoted MySQL identifier from a lowercase allowlist' do + q = described_class.new( + sql: 'SELECT * FROM USERS LIMIT 100', params: {}, bind_spec: [], + intent: { 'table' => 'users', 'type' => 'select' }, allow_tables: ['users'], + config: stub_config(adapter: :mysql) + ) + + expect(q.safe?).to be false + end + + it 'rejects a differently cased quoted MySQL identifier from a lowercase allowlist' do + q = described_class.new( + sql: 'SELECT * FROM `USERS` LIMIT 100', params: {}, bind_spec: [], + intent: { 'table' => 'users', 'type' => 'select' }, allow_tables: ['users'], + config: stub_config(adapter: :mysql) + ) + + expect(q.safe?).to be false + end + + it 'accepts exact-case unquoted MySQL identifiers from the allowlist' do + q = described_class.new( + sql: 'SELECT * FROM AuditEvents LIMIT 100', params: {}, bind_spec: [], + intent: { 'table' => 'AuditEvents', 'type' => 'select' }, allow_tables: ['AuditEvents'], + config: stub_config(adapter: :mysql) + ) + + expect(q.safe?).to be true + end + + it 'accepts explicitly allowlisted quoted mixed-case identifiers on every adapter' do + { postgres: '"AuditEvents"', sqlite: '"AuditEvents"', mysql: '`AuditEvents`' }.each do |adapter, table| + q = described_class.new( + sql: "SELECT * FROM #{table} LIMIT 100", params: {}, bind_spec: [], + intent: { 'table' => 'AuditEvents', 'type' => 'select' }, allow_tables: ['AuditEvents'], + config: stub_config(adapter: adapter) + ) + + expect(q.safe?).to be(true), "expected quoted #{adapter} identifier case to be preserved" + end + end + + it 'uses SQLite case-insensitive semantics for quoted identifiers' do + q = described_class.new( + sql: 'SELECT * FROM "AUDITEVENTS" LIMIT 100', params: {}, bind_spec: [], + intent: { 'table' => 'AuditEvents', 'type' => 'select' }, allow_tables: ['AuditEvents'], + config: stub_config(adapter: :sqlite) + ) + + expect(q.safe?).to be true + end + + it 'accepts uppercase unquoted related tables in PostgreSQL EXISTS filters' do + q = described_class.new( + sql: 'SELECT * FROM QUESTIONS WHERE EXISTS (SELECT 1 FROM ANSWERS)', params: {}, bind_spec: [], + intent: { + 'table' => 'questions', 'type' => 'select', + 'filters' => [{ 'column' => 'id', 'op' => 'exists', 'related_table' => 'answers' }] + }, + allow_tables: %w[questions answers], config: config + ) + + expect(q.safe?).to be true + end + + it 'accepts a correlated self-referential EXISTS on the allowlisted SQL base table' do + { + postgres: 'SELECT * FROM "questions" AS "parent" WHERE EXISTS (SELECT 1 FROM "questions" AS "child" WHERE "child"."parent_id" = "parent"."id") LIMIT 100', + mysql: 'SELECT * FROM `questions` AS `parent` WHERE EXISTS (SELECT 1 FROM `questions` AS `child` WHERE `child`.`parent_id` = `parent`.`id`) LIMIT 100' + }.each do |adapter, sql| + q = described_class.new( + sql: sql, + params: {}, + bind_spec: [], + intent: { + 'table' => 'questions', + 'type' => 'select', + 'filters' => [ + { + 'column' => 'id', + 'op' => 'exists', + 'related_table' => 'questions', + 'fk_column' => 'parent_id', + 'base_column' => 'id', + 'related_filters' => [] + } + ] + }, + allow_tables: ['questions'], + config: stub_config(adapter: adapter) + ) + + expect(q.safe?).to be(true), "expected correlated #{adapter} self-reference to be accepted" + end + end + + it 'does not let a mismatched but allowlisted intent table exempt a related-table JOIN' do + q = described_class.new( + sql: 'SELECT * FROM "questions" JOIN "answers" ON "answers"."question_id" = "questions"."id" WHERE EXISTS (SELECT 1 FROM "answers") LIMIT 100', + params: {}, + bind_spec: [], + intent: { + 'table' => 'answers', + 'type' => 'select', + 'filters' => [{ 'column' => 'id', 'op' => 'exists', 'related_table' => 'answers' }] + }, + allow_tables: %w[questions answers], + config: config + ) + + expect(q.safe?).to be false + end + + it 'fails closed when an ambiguous FROM list includes the allowlisted intent table' do + q = described_class.new( + sql: 'SELECT * FROM "questions", "answers" WHERE EXISTS (SELECT 1 FROM "answers") LIMIT 100', + params: {}, + bind_spec: [], + intent: { + 'table' => 'answers', + 'type' => 'select', + 'filters' => [{ 'column' => 'id', 'op' => 'exists', 'related_table' => 'answers' }] + }, + allow_tables: %w[questions answers], + config: config + ) + + expect(q.safe?).to be false + end + + it 'rejects a self-referential intent table outside the top-level allowlist' do + q = described_class.new( + sql: 'SELECT * FROM "answers" WHERE EXISTS (SELECT 1 FROM "answers") LIMIT 100', + params: {}, + bind_spec: [], + intent: { + 'table' => 'answers', + 'type' => 'select', + 'filters' => [ + { + 'column' => 'id', + 'op' => 'exists', + 'related_table' => 'answers', + 'fk_column' => 'parent_id', + 'base_column' => 'id', + 'related_filters' => [] + } + ] + }, + allow_tables: ['questions'], + config: config + ) + + expect(q.safe?).to be false + end + + it 'rejects an undeclared EXISTS table even when the top-level allowlist permits it' do + q = described_class.new( + sql: 'SELECT * FROM "questions" WHERE EXISTS (SELECT 1 FROM "answers")', + params: {}, bind_spec: [], intent: { 'table' => 'questions', 'type' => 'select', 'filters' => [] }, + allow_tables: %w[questions answers], config: config + ) + + expect(q.safe?).to be false + end + + it 'rejects an undeclared NOT EXISTS table when the intent declares no related tables' do + q = described_class.new( + sql: 'SELECT * FROM "questions" WHERE NOT EXISTS (SELECT 1 FROM "answers")', + params: {}, bind_spec: [], intent: { 'table' => 'questions', 'type' => 'select', 'filters' => [] }, + allow_tables: %w[questions answers], config: config + ) + + expect(q.safe?).to be false + end + + it 'returns false when expected subquery policy keys are missing from binds and params' do + config.policy_adapter = ->(_user, **) { { allowed_tables: ['users'] } } + + q = described_class.new( + sql: 'SELECT * FROM "questions" WHERE NOT EXISTS (SELECT 1 FROM "answers" WHERE "answers"."tenant_id" = $1)', + params: {}, + bind_spec: [], + intent: { + 'table' => 'questions', + 'type' => 'select', + 'filters' => [ + { + 'column' => 'id', + 'op' => 'not_exists', + 'related_table' => 'answers', + 'fk_column' => 'question_id', + 'base_column' => 'id', + 'related_filters' => [] + } + ], + '__policy_expected_keys' => ['policy_subquery_1_answers_tenant_id'] + }, + allow_tables: ['questions'], + config: config + ) + + expect(q.safe?).to be false + ensure + config.policy_adapter = nil + end + + it 'rejects synthetic subquery policy metadata even when binds are present' do + config.policy_adapter = ->(_user, **) { { allowed_tables: ['users'] } } + + q = described_class.new( + sql: 'SELECT * FROM "questions" WHERE EXISTS (SELECT 1 FROM "answers" WHERE "answers"."tenant_id" = $1)', + params: { 'policy_subquery_1_answers_tenant_id' => 42 }, + bind_spec: [{ key: 'policy_subquery_1_answers_tenant_id', column: 'tenant_id', cast: nil }], + intent: { + 'table' => 'questions', + 'type' => 'select', + 'filters' => [ + { + 'column' => 'id', + 'op' => 'exists', + 'related_table' => 'answers', + 'fk_column' => 'question_id', + 'base_column' => 'id', + 'related_filters' => [] + } + ], + '__policy_expected_keys' => ['policy_subquery_1_answers_tenant_id'] + }, + allow_tables: %w[questions answers], + config: config + ) + + expect(q.safe?).to be false + ensure + config.policy_adapter = nil + end + + it 'rejects main-table policy binds outside a mandatory qualified WHERE predicate' do + config.policy_adapter = ->(_user, **) { { allowed_tables: ['questions'] } } + + [ + 'SELECT $1 AS leaked_policy_value FROM "questions" WHERE TRUE LIMIT 10', + 'SELECT * FROM "questions" WHERE "questions"."tenant_id" = $1 OR TRUE LIMIT 10', + 'SELECT * FROM "questions" WHERE "tenant_id" = $1 LIMIT 10', + 'SELECT * FROM "questions" WHERE "other"."tenant_id" = $1 LIMIT 10' + ].each do |adversarial_sql| + q = described_class.new( + sql: adversarial_sql, + params: { 'policy_tenant_id' => 42 }, + bind_spec: [{ key: 'policy_tenant_id', column: 'tenant_id', cast: nil }], + intent: { + 'table' => 'questions', 'type' => 'select', + '__policy_expected_keys' => ['policy_tenant_id'] + }, + allow_tables: ['questions'], config: config + ) + + expect(q.safe?).to be(false), "expected to reject #{adversarial_sql.inspect}" + end + ensure + config.policy_adapter = nil + end + + it 'recognizes compiler-shaped main-table policy predicates with trailing clauses' do + scanner = CodeToQuery::Query::SqlScanner.new + sql = 'SELECT * FROM "questions" WHERE "questions"."tenant_id" = $1 ORDER BY "questions"."id" LIMIT 10' + + expect(scanner.policy_predicate_bind?(sql, 'questions', 'tenant_id', 1, adapter: :postgres)).to be true + end + + it 'uses adapter identifier casing semantics while scanning policy predicates' do + cases = { + postgres: 'QUESTIONS.TENANT_ID', + sqlite: '"QUESTIONS"."TENANT_ID"', + mysql: '`questions`.`TENANT_ID`' + } + scanner = CodeToQuery::Query::SqlScanner.new + + cases.each do |adapter, qualified_column| + sql = "SELECT * FROM questions WHERE #{qualified_column} = $1 LIMIT 10" + + expect(scanner.policy_predicate_bind?(sql, 'questions', 'tenant_id', 1, adapter: adapter)) + .to be(true), "expected #{adapter} casing semantics to be honored" + end + end + + it 'rejects a Unicode-confusable SQLite policy column predicate' do + policy_adapter = ->(_user, **) { { allowed_tables: ['kids'] } } + q = described_class.new( + sql: 'SELECT * FROM "kids" WHERE "kids"."Kind" = ? LIMIT 10', + params: { 'policy_kind' => 'student' }, + bind_spec: [{ key: 'policy_kind', column: 'kind', cast: nil }], + intent: { 'table' => 'kids', 'type' => 'select', '__policy_expected_keys' => ['policy_kind'] }, + allow_tables: ['kids'], config: stub_config(adapter: :sqlite, policy_adapter: policy_adapter) + ) + + expect(q.safe?).to be false + end + + it 'rejects a differently cased quoted PostgreSQL policy column' do + config.policy_adapter = ->(_user, **) { { allowed_tables: %w[questions answers] } } + q = described_class.new( + sql: 'SELECT * FROM "questions" WHERE EXISTS (SELECT 1 FROM "answers" WHERE "answers"."TENANT_ID" = $1)', + params: { 'policy_subquery_1_answers_tenant_id' => 42 }, + bind_spec: [{ key: 'policy_subquery_1_answers_tenant_id', column: 'tenant_id', cast: nil }], + intent: { + 'table' => 'questions', 'type' => 'select', + 'filters' => [{ 'column' => 'id', 'op' => 'exists', 'related_table' => 'answers', 'fk_column' => 'question_id' }], + '__policy_expected_keys' => ['policy_subquery_1_answers_tenant_id'] + }, + allow_tables: %w[questions answers], config: config + ) + + expect(q.safe?).to be false + ensure + config.policy_adapter = nil + end + + it 'rejects policy binds used only in SELECT or tautological expressions' do + config.policy_adapter = ->(_user, **) { { allowed_tables: %w[questions answers] } } + + [ + 'SELECT $1 FROM "answers" WHERE TRUE', + 'SELECT 1 FROM "answers" WHERE $1 = $1' + ].each do |body| + q = described_class.new( + sql: "SELECT * FROM \"questions\" WHERE EXISTS (#{body})", + params: { 'policy_subquery_1_answers_tenant_id' => 42 }, + bind_spec: [{ key: 'policy_subquery_1_answers_tenant_id', column: 'tenant_id', cast: nil }], + intent: { + 'table' => 'questions', 'type' => 'select', + 'filters' => [{ 'column' => 'id', 'op' => 'exists', 'related_table' => 'answers', 'fk_column' => 'question_id' }], + '__policy_expected_keys' => ['policy_subquery_1_answers_tenant_id'] + }, + allow_tables: %w[questions answers], config: config + ) + + expect(q.safe?).to be false + end + ensure + config.policy_adapter = nil + end + + it 'rejects additional unscoped OR EXISTS and NOT EXISTS references to a policy-scoped table' do + config.policy_adapter = ->(_user, **) { { allowed_tables: %w[questions answers] } } + + ['EXISTS', 'NOT EXISTS'].each do |operator| + q = described_class.new( + sql: %(SELECT * FROM "questions" WHERE EXISTS (SELECT 1 FROM "answers" WHERE "answers"."tenant_id" = $1) OR #{operator} (SELECT 1 FROM "answers")), + params: { 'policy_subquery_1_answers_tenant_id' => 42 }, + bind_spec: [{ key: 'policy_subquery_1_answers_tenant_id', column: 'tenant_id', cast: nil }], + intent: { + 'table' => 'questions', 'type' => 'select', + 'filters' => [{ 'column' => 'id', 'op' => 'exists', 'related_table' => 'answers', 'fk_column' => 'question_id' }], + '__policy_expected_keys' => ['policy_subquery_1_answers_tenant_id'] + }, + # Explicitly allowlisting the related table must not bypass the + # occurrence-level policy scope checks. + allow_tables: %w[questions answers], config: config + ) + + expect(q.safe?).to be false + end + ensure + config.policy_adapter = nil + end + + it 'rejects two declared same-table EXISTS occurrences when only one is policy scoped' do + config.policy_adapter = ->(_user, **) { { allowed_tables: %w[questions answers] } } + q = described_class.new( + sql: 'SELECT * FROM "questions" WHERE EXISTS (SELECT 1 FROM "answers" WHERE "answers"."tenant_id" = $1 AND "answers"."account_id" = $2) OR EXISTS (SELECT 1 FROM "answers")', + params: { 'policy_subquery_1_answers_tenant_id' => 42, 'policy_subquery_2_answers_account_id' => 7 }, + bind_spec: [ + { key: 'policy_subquery_1_answers_tenant_id', column: 'tenant_id', cast: nil }, + { key: 'policy_subquery_2_answers_account_id', column: 'account_id', cast: nil } + ], + intent: { + 'table' => 'questions', 'type' => 'select', + 'filters' => Array.new(2) { { 'column' => 'id', 'op' => 'exists', 'related_table' => 'answers', 'fk_column' => 'question_id' } }, + '__policy_expected_keys' => %w[policy_subquery_1_answers_tenant_id policy_subquery_2_answers_account_id] + }, + allow_tables: ['questions'], config: config + ) + + expect(q.safe?).to be false + ensure + config.policy_adapter = nil + end + + it 'rejects reuse of the first occurrence policy bind by a second same-table EXISTS' do + config.policy_adapter = ->(_user, **) { { allowed_tables: %w[questions answers] } } + q = described_class.new( + sql: 'SELECT * FROM "questions" WHERE EXISTS (SELECT 1 FROM "answers" WHERE "answers"."tenant_id" = $1) OR EXISTS (SELECT 1 FROM "answers" WHERE "answers"."tenant_id" = $1)', + params: { 'policy_subquery_1_answers_tenant_id' => 42, 'policy_subquery_2_answers_tenant_id' => 42 }, + bind_spec: [ + { key: 'policy_subquery_1_answers_tenant_id', column: 'tenant_id', cast: nil }, + { key: 'policy_subquery_2_answers_tenant_id', column: 'tenant_id', cast: nil } + ], + intent: { + 'table' => 'questions', 'type' => 'select', + 'filters' => Array.new(2) { { 'column' => 'id', 'op' => 'exists', 'related_table' => 'answers', 'fk_column' => 'question_id' } }, + '__policy_expected_keys' => %w[policy_subquery_1_answers_tenant_id policy_subquery_2_answers_tenant_id] + }, + allow_tables: ['questions'], config: config + ) + + expect(q.safe?).to be false + ensure + config.policy_adapter = nil + end + + it 'returns false when expected subquery policy keys are present only in params' do + config.policy_adapter = ->(_user, **) { { allowed_tables: ['users'] } } + + q = described_class.new( + sql: 'SELECT * FROM "questions" WHERE EXISTS (SELECT 1 FROM "answers" WHERE "answers"."tenant_id" = $1)', + params: { 'policy_subquery_1_answers_tenant_id' => 42 }, + bind_spec: [], + intent: { + 'table' => 'questions', + 'type' => 'select', + 'filters' => [ + { + 'column' => 'id', + 'op' => 'exists', + 'related_table' => 'answers', + 'fk_column' => 'question_id', + 'base_column' => 'id', + 'related_filters' => [] + } + ], + '__policy_expected_keys' => ['policy_subquery_1_answers_tenant_id'] + }, + allow_tables: %w[questions answers], + config: config + ) + + expect(q.safe?).to be false + ensure + config.policy_adapter = nil + end + + it 'returns false for NOT EXISTS when expected policy binds have no parameter value' do + config.policy_adapter = ->(_user, **) { { allowed_tables: %w[questions answers] } } + q = described_class.new( + sql: 'SELECT * FROM "questions" WHERE NOT EXISTS (SELECT 1 FROM "answers" WHERE "answers"."tenant_id" = $1)', + params: {}, + bind_spec: [{ key: 'policy_subquery_1_answers_tenant_id', column: 'tenant_id', cast: nil }], + intent: { + 'table' => 'questions', 'type' => 'select', + 'filters' => [{ 'column' => 'id', 'op' => 'not_exists', 'related_table' => 'answers', 'fk_column' => 'question_id' }], + '__policy_expected_keys' => ['policy_subquery_1_answers_tenant_id'] + }, + allow_tables: %w[questions answers], config: config + ) + + expect(q.safe?).to be false + ensure + config.policy_adapter = nil + end + + it 'does not let EXISTS-like text in a literal hide a disallowed table' do + q = described_class.new( + sql: %(SELECT 'EXISTS (ignored)' FROM "questions" JOIN "answers" ON TRUE), + params: {}, bind_spec: [], intent: { 'table' => 'questions', 'type' => 'select' }, + allow_tables: ['questions'], config: config + ) + + expect(q.safe?).to be false end - it 'adds between defaults derived from column names for legacy start/end params' do + it 'ignores parentheses in EXISTS comments while identifying the subquery boundary' do + sql = 'SELECT * FROM "questions" WHERE EXISTS (SELECT 1 FROM "answers" /* ) */ WHERE TRUE)' q = described_class.new( - sql: 'SELECT * FROM "orders" WHERE "created_at" BETWEEN $1 AND $2', - params: { 'start' => '2023-01-01', 'end' => '2023-12-31' }, - bind_spec: [{ key: 'created_at_start', column: 'created_at' }, { key: 'created_at_end', column: 'created_at' }], + sql: sql, + params: {}, bind_spec: [], + intent: { 'table' => 'questions', 'type' => 'select', 'filters' => [{ 'op' => 'exists', 'related_table' => 'answers' }] }, + allow_tables: ['questions'], config: config + ) + + expect(q.send(:strip_exists_subqueries, sql)).to eq('SELECT * FROM "questions" WHERE TRUE') + expect(q.safe?).to be false # SqlLinter intentionally rejects SQL comments. + end + + it 'parses PostgreSQL ONLY in the central top-level allowlist check' do + q = described_class.new( + sql: 'SELECT * FROM ONLY "questions" LIMIT 1', params: {}, bind_spec: [], + intent: { 'table' => 'questions', 'type' => 'select' }, allow_tables: ['questions'], config: config + ) + + expect(q.safe?).to be true + end + + it 'does not let allow_tables only hide another relation from the central allowlist check' do + q = described_class.new( + sql: 'SELECT * FROM ONLY admin_secrets LIMIT 1', params: {}, bind_spec: [], + intent: { 'table' => 'only', 'type' => 'select' }, allow_tables: ['only'], config: config + ) + + expect { q.send(:check_top_level_table_allowlist!) } + .to raise_error(SecurityError, /admin_secrets.*not in the allowed list/) + end + + it 'rejects a disallowed FROM table continued on a second line' do + q = described_class.new( + sql: "SELECT * FROM \"questions\",\n\"answers\"", params: {}, bind_spec: [], + intent: { 'table' => 'questions', 'type' => 'select' }, allow_tables: ['questions'], config: config + ) + + expect(q.safe?).to be false + end + + it 'does not enforce a partial related-table allowlist when no explicit allow_tables are provided' do + q = described_class.new( + sql: 'SELECT * FROM "questions" WHERE EXISTS (SELECT 1 FROM "answers" WHERE "answers"."question_id" = "questions"."id")', + params: {}, + bind_spec: [], intent: { - 'table' => 'orders', + 'table' => 'questions', 'type' => 'select', 'filters' => [ - { 'column' => 'created_at', 'op' => 'between' } + { + 'column' => 'id', + 'op' => 'exists', + 'related_table' => 'answers', + 'fk_column' => 'question_id', + 'base_column' => 'id', + 'related_filters' => [] + } ] }, - allow_tables: ['orders'], + allow_tables: nil, config: config ) - expect(q.params['created_at_start']).to eq('2023-01-01') - expect(q.params['created_at_end']).to eq('2023-12-31') + expect(q.safe?).to be true end - end - describe '#safe?' do - context 'with valid query' do - it 'returns true for safe queries' do - q = described_class.new( - sql: sql, - params: params, - bind_spec: bind_spec, - intent: intent, - allow_tables: ['users'], - config: config - ) - allow(q).to receive(:perform_safety_checks).and_return(true) + it 'enforces policy-derived related-table allowlists when no explicit allow_tables are provided' do + q = described_class.new( + sql: 'SELECT * FROM "questions" WHERE EXISTS (SELECT 1 FROM "answers" WHERE "answers"."question_id" = "questions"."id")', + params: {}, + bind_spec: [], + intent: { + 'table' => 'questions', + 'type' => 'select', + '__policy_allowed_tables' => ['questions'], + 'filters' => [ + { + 'column' => 'id', + 'op' => 'exists', + 'related_table' => 'answers', + 'fk_column' => 'question_id', + 'base_column' => 'id', + 'related_filters' => [] + } + ] + }, + allow_tables: nil, + config: config + ) - expect(q.safe?).to be true - end + expect(q.safe?).to be false end - context 'with unsafe query' do - let(:sql) { 'DROP TABLE users' } + it 'enforces policy-derived allowlists even when explicit allow_tables casing differs' do + q = described_class.new( + sql: 'SELECT * FROM "questions" WHERE EXISTS (SELECT 1 FROM "answers" WHERE "answers"."question_id" = "questions"."id")', + params: {}, + bind_spec: [], + intent: { + 'table' => 'questions', + 'type' => 'select', + '__policy_allowed_tables' => ['questions'], + 'filters' => [ + { + 'column' => 'id', + 'op' => 'exists', + 'related_table' => 'answers', + 'fk_column' => 'question_id', + 'base_column' => 'id', + 'related_filters' => [] + } + ] + }, + allow_tables: ['Questions'], + config: config + ) - it 'returns false for unsafe queries' do - expect(query.safe?).to be false - end + expect(q.safe?).to be false end - it 'caches the safety check result' do + it 'fails closed when explicit and policy allowlists intersect to an empty set' do q = described_class.new( - sql: sql, - params: params, - bind_spec: bind_spec, - intent: intent, - allow_tables: ['users'], + sql: 'SELECT * FROM "questions" WHERE EXISTS (SELECT 1 FROM "answers" WHERE "answers"."question_id" = "questions"."id")', + params: {}, + bind_spec: [], + intent: { + 'table' => 'questions', + 'type' => 'select', + '__policy_allowed_tables' => ['answers'], + 'filters' => [ + { + 'column' => 'id', + 'op' => 'exists', + 'related_table' => 'answers', + 'fk_column' => 'question_id', + 'base_column' => 'id', + 'related_filters' => [] + } + ] + }, + allow_tables: ['questions'], config: config ) - allow(q).to receive(:perform_safety_checks).and_return(true) - q.safe? - q.safe? + expect(q.safe?).to be false + end - expect(q).to have_received(:perform_safety_checks).once + it 'still blocks related tables when SQL references them outside the declared EXISTS subquery' do + q = described_class.new( + sql: 'SELECT * FROM "questions" JOIN "answers" ON "answers"."question_id" = "questions"."id" WHERE EXISTS (SELECT 1 FROM "answers" WHERE "answers"."tenant_id" = $1)', + params: { 'policy_subquery_1_answers_tenant_id' => 42 }, + bind_spec: [{ key: 'policy_subquery_1_answers_tenant_id', column: 'tenant_id', cast: nil }], + intent: { + 'table' => 'questions', + 'type' => 'select', + 'filters' => [ + { + 'column' => 'id', + 'op' => 'exists', + 'related_table' => 'answers', + 'fk_column' => 'question_id', + 'base_column' => 'id', + 'related_filters' => [] + } + ], + '__policy_expected_keys' => ['policy_subquery_1_answers_tenant_id'] + }, + allow_tables: ['questions'], + config: config + ) + + expect(q.safe?).to be false + end + + it 'still blocks related tables when SQL references them outside the declared EXISTS subquery with a quoted alias' do + q = described_class.new( + sql: 'SELECT * FROM "questions" JOIN "answers" AS "a" ON "a"."question_id" = "questions"."id" WHERE EXISTS (SELECT 1 FROM "answers" WHERE "answers"."tenant_id" = $1)', + params: { 'policy_subquery_1_answers_tenant_id' => 42 }, + bind_spec: [{ key: 'policy_subquery_1_answers_tenant_id', column: 'tenant_id', cast: nil }], + intent: { + 'table' => 'questions', + 'type' => 'select', + 'filters' => [ + { + 'column' => 'id', + 'op' => 'exists', + 'related_table' => 'answers', + 'fk_column' => 'question_id', + 'base_column' => 'id', + 'related_filters' => [] + } + ], + '__policy_expected_keys' => ['policy_subquery_1_answers_tenant_id'] + }, + allow_tables: ['questions'], + config: config + ) + + expect(q.safe?).to be false + end + + it 'still blocks related tables when SQL references them through a LEFT OUTER JOIN' do + q = described_class.new( + sql: 'SELECT * FROM "questions" LEFT OUTER JOIN "answers" ON "answers"."question_id" = "questions"."id" WHERE EXISTS (SELECT 1 FROM "answers" WHERE "answers"."tenant_id" = $1)', + params: { 'policy_subquery_1_answers_tenant_id' => 42 }, + bind_spec: [{ key: 'policy_subquery_1_answers_tenant_id', column: 'tenant_id', cast: nil }], + intent: { + 'table' => 'questions', + 'type' => 'select', + 'filters' => [ + { + 'column' => 'id', + 'op' => 'exists', + 'related_table' => 'answers', + 'fk_column' => 'question_id', + 'base_column' => 'id', + 'related_filters' => [] + } + ], + '__policy_expected_keys' => ['policy_subquery_1_answers_tenant_id'] + }, + allow_tables: ['questions'], + config: config + ) + + expect(q.safe?).to be false + end + + it 'still blocks related tables when SQL references them through a NATURAL JOIN' do + q = described_class.new( + sql: 'SELECT * FROM "questions" NATURAL JOIN "answers" WHERE EXISTS (SELECT 1 FROM "answers" WHERE "answers"."tenant_id" = $1)', + params: { 'policy_subquery_1_answers_tenant_id' => 42 }, + bind_spec: [{ key: 'policy_subquery_1_answers_tenant_id', column: 'tenant_id', cast: nil }], + intent: { + 'table' => 'questions', + 'type' => 'select', + 'filters' => [ + { + 'column' => 'id', + 'op' => 'exists', + 'related_table' => 'answers', + 'fk_column' => 'question_id', + 'base_column' => 'id', + 'related_filters' => [] + } + ], + '__policy_expected_keys' => ['policy_subquery_1_answers_tenant_id'] + }, + allow_tables: ['questions'], + config: config + ) + + expect(q.safe?).to be false + end + + it 'still blocks related tables when SQL references them as an extra top-level FROM source' do + q = described_class.new( + sql: 'SELECT * FROM "questions", "answers" WHERE EXISTS (SELECT 1 FROM "answers" WHERE "answers"."tenant_id" = $1)', + params: { 'policy_subquery_1_answers_tenant_id' => 42 }, + bind_spec: [{ key: 'policy_subquery_1_answers_tenant_id', column: 'tenant_id', cast: nil }], + intent: { + 'table' => 'questions', + 'type' => 'select', + 'filters' => [ + { + 'column' => 'id', + 'op' => 'exists', + 'related_table' => 'answers', + 'fk_column' => 'question_id', + 'base_column' => 'id', + 'related_filters' => [] + } + ], + '__policy_expected_keys' => ['policy_subquery_1_answers_tenant_id'] + }, + allow_tables: ['questions'], + config: config + ) + + expect(q.safe?).to be false + end + + it 'still blocks schema-qualified related tables when SQL references them as an extra top-level FROM source' do + q = described_class.new( + sql: 'SELECT * FROM "questions", public."answers" WHERE EXISTS (SELECT 1 FROM "answers" WHERE "answers"."tenant_id" = $1)', + params: { 'policy_subquery_1_answers_tenant_id' => 42 }, + bind_spec: [{ key: 'policy_subquery_1_answers_tenant_id', column: 'tenant_id', cast: nil }], + intent: { + 'table' => 'questions', + 'type' => 'select', + 'filters' => [ + { + 'column' => 'id', + 'op' => 'exists', + 'related_table' => 'answers', + 'fk_column' => 'question_id', + 'base_column' => 'id', + 'related_filters' => [] + } + ], + '__policy_expected_keys' => ['policy_subquery_1_answers_tenant_id'] + }, + allow_tables: ['questions'], + config: config + ) + + expect(q.safe?).to be false + end + + it 'still blocks related tables when SQL reuses them in a non-EXISTS nested subquery' do + q = described_class.new( + sql: 'SELECT * FROM "questions" WHERE id IN (SELECT "answers"."question_id" FROM "answers" WHERE "answers"."tenant_id" = $1) AND EXISTS (SELECT 1 FROM "answers" WHERE "answers"."tenant_id" = $1)', + params: { 'policy_subquery_1_answers_tenant_id' => 42 }, + bind_spec: [{ key: 'policy_subquery_1_answers_tenant_id', column: 'tenant_id', cast: nil }], + intent: { + 'table' => 'questions', + 'type' => 'select', + '__policy_allowed_tables' => %w[questions answers], + 'filters' => [ + { + 'column' => 'id', + 'op' => 'exists', + 'related_table' => 'answers', + 'fk_column' => 'question_id', + 'base_column' => 'id', + 'related_filters' => [] + } + ], + '__policy_expected_keys' => ['policy_subquery_1_answers_tenant_id'] + }, + allow_tables: nil, + config: config + ) + + expect(q.safe?).to be false + end + + it 'still blocks top-level derived tables that reference related tables outside the declared EXISTS subquery' do + q = described_class.new( + sql: 'SELECT * FROM "questions", (SELECT * FROM "answers") leaked WHERE EXISTS (SELECT 1 FROM "answers" WHERE "answers"."tenant_id" = $1)', + params: { 'policy_subquery_1_answers_tenant_id' => 42 }, + bind_spec: [{ key: 'policy_subquery_1_answers_tenant_id', column: 'tenant_id', cast: nil }], + intent: { + 'table' => 'questions', + 'type' => 'select', + 'filters' => [ + { + 'column' => 'id', + 'op' => 'exists', + 'related_table' => 'answers', + 'fk_column' => 'question_id', + 'base_column' => 'id', + 'related_filters' => [] + } + ], + '__policy_expected_keys' => ['policy_subquery_1_answers_tenant_id'] + }, + allow_tables: ['questions'], + config: config + ) + + expect(q.safe?).to be false + end + + it 'still blocks top-level lateral derived tables in FROM sources' do + q = described_class.new( + sql: 'SELECT * FROM LATERAL (SELECT * FROM "answers") leaked, "questions" WHERE EXISTS (SELECT 1 FROM "answers" WHERE "answers"."tenant_id" = $1)', + params: { 'policy_subquery_1_answers_tenant_id' => 42 }, + bind_spec: [{ key: 'policy_subquery_1_answers_tenant_id', column: 'tenant_id', cast: nil }], + intent: { + 'table' => 'questions', + 'type' => 'select', + 'filters' => [ + { + 'column' => 'id', + 'op' => 'exists', + 'related_table' => 'answers', + 'fk_column' => 'question_id', + 'base_column' => 'id', + 'related_filters' => [] + } + ], + '__policy_expected_keys' => ['policy_subquery_1_answers_tenant_id'] + }, + allow_tables: ['questions'], + config: config + ) + + expect(q.safe?).to be false + end + + it 'still blocks comma lateral derived tables that reference related tables outside the declared EXISTS subquery' do + q = described_class.new( + sql: 'SELECT * FROM "questions", LATERAL (SELECT * FROM "answers") leaked WHERE EXISTS (SELECT 1 FROM "answers" WHERE "answers"."tenant_id" = $1)', + params: { 'policy_subquery_1_answers_tenant_id' => 42 }, + bind_spec: [{ key: 'policy_subquery_1_answers_tenant_id', column: 'tenant_id', cast: nil }], + intent: { + 'table' => 'questions', + 'type' => 'select', + 'filters' => [ + { + 'column' => 'id', + 'op' => 'exists', + 'related_table' => 'answers', + 'fk_column' => 'question_id', + 'base_column' => 'id', + 'related_filters' => [] + } + ], + '__policy_expected_keys' => ['policy_subquery_1_answers_tenant_id'] + }, + allow_tables: ['questions'], + config: config + ) + + expect(q.safe?).to be false + end + + it 'still blocks top-level common table expressions that reference related tables outside the declared EXISTS subquery' do + q = described_class.new( + sql: 'WITH leaked AS (SELECT * FROM "answers") SELECT * FROM "questions" WHERE EXISTS (SELECT 1 FROM "answers" WHERE "answers"."tenant_id" = $1)', + params: { 'policy_subquery_1_answers_tenant_id' => 42 }, + bind_spec: [{ key: 'policy_subquery_1_answers_tenant_id', column: 'tenant_id', cast: nil }], + intent: { + 'table' => 'questions', + 'type' => 'select', + 'filters' => [ + { + 'column' => 'id', + 'op' => 'exists', + 'related_table' => 'answers', + 'fk_column' => 'question_id', + 'base_column' => 'id', + 'related_filters' => [] + } + ], + '__policy_expected_keys' => ['policy_subquery_1_answers_tenant_id'] + }, + allow_tables: ['questions'], + config: config + ) + + expect(q.safe?).to be false + end + + it 'still blocks top-level common table expressions with leading whitespace' do + q = described_class.new( + sql: ' WITH leaked AS (SELECT * FROM "answers") SELECT * FROM "questions" WHERE EXISTS (SELECT 1 FROM "answers" WHERE "answers"."tenant_id" = $1)', + params: { 'policy_subquery_1_answers_tenant_id' => 42 }, + bind_spec: [{ key: 'policy_subquery_1_answers_tenant_id', column: 'tenant_id', cast: nil }], + intent: { + 'table' => 'questions', + 'type' => 'select', + 'filters' => [ + { + 'column' => 'id', + 'op' => 'exists', + 'related_table' => 'answers', + 'fk_column' => 'question_id', + 'base_column' => 'id', + 'related_filters' => [] + } + ], + '__policy_expected_keys' => ['policy_subquery_1_answers_tenant_id'] + }, + allow_tables: ['questions'], + config: config + ) + + expect(q.safe?).to be false end end @@ -162,6 +1597,37 @@ class << self expect(query.to_relation).to be_nil end end + + it 'applies compiler-injected policy filters from the query intent' do + scope = double('scope') + allow(scope).to receive_messages(where: scope, order: scope, limit: scope) + build_scope_backed_user_model(scope) + + build_policy_query(config).to_relation + + expect(scope).to have_received(:where).with('active' => true).ordered + expect(scope).to have_received(:where).with('tenant_id' => 42).ordered + expect(scope).to have_received(:limit).with(100) + end + + it 'returns nil for a directly constructed query when a policy adapter is configured' do + scope = double('scope') + allow(scope).to receive_messages(where: scope, order: scope, limit: scope) + build_scope_backed_user_model(scope) + config.policy_adapter = ->(_context) { { filters: [] } } + + expect(query.to_relation).to be_nil + expect(scope).not_to have_received(:where) + end + + it 'returns nil when relation semantics would drop compiler-only subquery policy predicates' do + scope = double('scope') + allow(scope).to receive_messages(where: scope, order: scope, limit: scope) + build_scope_backed_question_model(scope) + + expect(build_subquery_policy_query(config).to_relation).to be_nil + expect(scope).not_to have_received(:where) + end end describe '#to_active_record' do @@ -176,6 +1642,19 @@ class << self q = described_class.new(sql: sql, params: params, bind_spec: bind_spec, intent: { 'type' => 'insert' }, allow_tables: ['users'], config: config) expect(q.relationable?).to be false end + + it 'returns false when relation semantics would drop compiler-only subquery policy predicates' do + build_scope_backed_question_model(double('scope')) + + expect(build_subquery_policy_query(config).relationable?).to be false + end + + it 'returns false without a compiler policy contract when a policy adapter is configured' do + build_scope_backed_user_model(double('scope')) + config.policy_adapter = ->(_context) { { filters: [] } } + + expect(query.relationable?).to be false + end end describe '#to_relation!' do @@ -186,6 +1665,28 @@ class << self end describe '#run' do + it 'executes the protected SQL after constructor and accessor values are mutated' do + mutable_sql = sql.dup + mutable_intent = intent.transform_values { |value| value.is_a?(String) ? value.dup : value } + mutable_allow_tables = ['users'.dup] + q = described_class.new( + sql: mutable_sql, params: params, bind_spec: [], intent: mutable_intent, + allow_tables: mutable_allow_tables, config: config + ) + runner = instance_double(CodeToQuery::Runner, run: :result) + allow(CodeToQuery::Runner).to receive(:new).with(config).and_return(runner) + + expect(q.safe?).to be true + mutable_sql.replace('DROP TABLE users') + mutable_intent['table'].replace('admins') + mutable_allow_tables.first.replace('admins') + q.sql.replace('DROP TABLE users') + q.intent['table'].replace('admins') + + expect(q.run).to eq(:result) + expect(runner).to have_received(:run).with(sql: sql, binds: []) + end + it 'emits run instrumentation and delegates to Runner' do events = [] subscriber = ActiveSupport::Notifications.subscribe('code_to_query.run') do |_name, _started, _finished, _id, payload| @@ -211,6 +1712,28 @@ class << self ensure ActiveSupport::Notifications.unsubscribe(subscriber) if subscriber end + + it 're-runs safety checks at the execution boundary instead of trusting the cached result' do + allow(query).to receive(:perform_safety_checks).and_return(true, false) + allow(CodeToQuery::Runner).to receive(:new) + + expect(query.safe?).to be true + + expect { query.run }.to raise_error(SecurityError, /failed safety checks/) + expect(query).to have_received(:perform_safety_checks).twice + expect(CodeToQuery::Runner).not_to have_received(:new) + end + + it 'fails closed when an unsafe query is run directly' do + unsafe_query = described_class.new( + sql: 'DROP TABLE users', params: {}, bind_spec: [], intent: intent, + allow_tables: ['users'], config: config + ) + allow(CodeToQuery::Runner).to receive(:new) + + expect { unsafe_query.run }.to raise_error(SecurityError, /failed safety checks/) + expect(CodeToQuery::Runner).not_to have_received(:new) + end end describe '#preview' do @@ -250,7 +1773,7 @@ class << self end end - describe '#binds' do + describe '#binds fallback behavior' do context 'without ActiveRecord' do it 'returns empty array' do hide_const('ActiveRecord') diff --git a/spec/code_to_query/validator_adversarial_fixture_spec.rb b/spec/code_to_query/validator_adversarial_fixture_spec.rb index 685fe3b..2c0ff8f 100644 --- a/spec/code_to_query/validator_adversarial_fixture_spec.rb +++ b/spec/code_to_query/validator_adversarial_fixture_spec.rb @@ -25,10 +25,10 @@ it 'rejects the intent with the expected reason' do policy = test_case['policy'] policy_adapter = policy && lambda do |_user, **_context| - { - allowed_tables: policy.fetch('allowed_tables', []), - allowed_columns: policy.fetch('allowed_columns', {}) - } + {}.tap do |policy_info| + policy_info[:allowed_tables] = policy['allowed_tables'] if policy.key?('allowed_tables') + policy_info[:allowed_columns] = policy['allowed_columns'] if policy.key?('allowed_columns') + end end stub_config(policy_adapter: policy_adapter, policy_adapter_fail_open: false) diff --git a/spec/code_to_query/validator_spec.rb b/spec/code_to_query/validator_spec.rb index ab59372..1966e19 100644 --- a/spec/code_to_query/validator_spec.rb +++ b/spec/code_to_query/validator_spec.rb @@ -31,6 +31,35 @@ end end + context 'with policy table restrictions' do + let(:intent) do + { 'type' => 'select', 'table' => 'users', 'columns' => ['*'], + '__policy_allowed_tables' => ['users'] } + end + + it 'strips caller-supplied policy allowlist metadata before consulting policy' do + CodeToQuery.config.policy_adapter = ->(_user, **) { { allowed_tables: ['orders'] } } + + expect { validator.validate(intent) }.to raise_error(ArgumentError, /not permitted by policy/) + end + + it 'treats an explicitly empty policy allowlist as deny all' do + CodeToQuery.config.policy_adapter = ->(_user, **) { { allowed_tables: [] } } + + expect { validator.validate(intent) }.to raise_error(ArgumentError, /not permitted by policy/) + end + + it 'leaves tables unrestricted when the policy table allowlist is absent' do + CodeToQuery.config.policy_adapter = ->(_user, **) { { allowed_columns: {} } } + + result = validator.validate(intent) + + expect(result[:table]).to eq('users') + expect(result).not_to have_key(:__policy_allowed_tables) + expect(result).not_to have_key('__policy_allowed_tables') + end + end + context 'with missing required fields' do it 'raises ArgumentError when type is missing' do intent = { 'table' => 'users', 'columns' => ['*'] } @@ -151,6 +180,313 @@ end end + context 'with policy column restrictions on related subqueries' do + before do + config.policy_adapter = lambda do |_user, **_kwargs| + { + allowed_tables: %w[users orders], + allowed_columns: { + 'users' => %w[id email], + 'orders' => %w[user_id status] + } + } + end + end + + def related_subquery_intent(operator, fk_column: 'user_id', base_column: 'id') + { + 'type' => 'select', + 'table' => 'users', + 'columns' => ['email'], + 'filters' => [ + { + 'op' => operator, + 'related_table' => 'orders', + 'fk_column' => fk_column, + 'base_column' => base_column, + 'related_filters' => [ + { 'column' => 'status', 'op' => '=', 'param' => 'status' } + ] + } + ] + } + end + + %w[exists not_exists].each do |op| + it "allows #{op} when both correlated columns are policy-permitted" do + result = validator.validate(related_subquery_intent(op)) + + expect(result[:filters].first).to include(fk_column: 'user_id', base_column: 'id') + end + + it "rejects #{op} when fk_column is not permitted on the related table" do + expect do + validator.validate(related_subquery_intent(op, fk_column: 'secret_user_id')) + end.to raise_error(ArgumentError, /column 'secret_user_id' not permitted on 'orders'/) + end + + it "rejects #{op} when base_column is not permitted on the main table" do + expect do + validator.validate(related_subquery_intent(op, base_column: 'secret_id')) + end.to raise_error(ArgumentError, /column 'secret_id' not permitted on 'users'/) + end + + it "uses and checks the default id base_column for #{op} when it is omitted" do + subquery_intent = related_subquery_intent(op) + subquery_intent['filters'].first.delete('base_column') + + result = validator.validate(subquery_intent) + + expect(result[:filters].first[:base_column]).to eq('id') + end + + it "rejects #{op} when the omitted base_column defaults to a disallowed id" do + config.policy_adapter = lambda do |_user, **_kwargs| + { + allowed_tables: %w[users orders], + allowed_columns: { + 'users' => ['email'], + 'orders' => %w[user_id status] + } + } + end + subquery_intent = related_subquery_intent(op) + subquery_intent['filters'].first.delete('base_column') + + expect do + validator.validate(subquery_intent) + end.to raise_error(ArgumentError, /column 'id' not permitted on 'users'/) + end + + it "does not restrict #{op} correlation columns for nil or empty per-table lists" do + config.policy_adapter = lambda do |_user, **_kwargs| + { + allowed_tables: %w[users orders], + allowed_columns: { 'users' => nil, 'orders' => [] } + } + end + + result = validator.validate( + related_subquery_intent(op, fk_column: 'legacy_user_key', base_column: 'legacy_id') + ) + + expect(result[:filters].first).to include( + fk_column: 'legacy_user_key', base_column: 'legacy_id' + ) + end + end + end + + context 'with adapter-specific policy column casing' do + let(:column_policy) do + { + allowed_tables: %w[users orders], + allowed_columns: { + 'users' => %w[id UserCode PublicTotal], + 'orders' => %w[user_id User_ID status StatusCode] + } + } + end + let(:related_intent) do + { + 'type' => 'select', 'table' => 'users', 'columns' => ['UserCode'], + 'filters' => [{ + 'op' => 'exists', 'related_table' => 'orders', + 'fk_column' => 'user_id', 'base_column' => 'id', + 'related_filters' => [{ 'column' => 'StatusCode', 'op' => '=', 'param' => 'status' }] + }] + } + end + + before do + config.policy_adapter = ->(_user, **_kwargs) { column_policy } + end + + it 'allows distinct exact-case PostgreSQL columns' do + expect { validator.validate(related_intent) }.not_to raise_error + + mixed_case_fk = related_intent.dup + mixed_case_fk['filters'] = related_intent['filters'].map { |filter| filter.merge('fk_column' => 'User_ID') } + expect { validator.validate(mixed_case_fk) }.not_to raise_error + end + + { + 'selected column' => ->(intent) { intent['columns'] = ['usercode'] }, + 'ORDER BY column' => lambda do |intent| + intent['order'] = [{ 'column' => 'usercode', 'dir' => 'asc' }] + end, + 'DISTINCT ON column' => ->(intent) { intent['distinct_on'] = ['usercode'] }, + 'GROUP BY column' => ->(intent) { intent['group_by'] = ['usercode'] }, + 'aggregation column' => lambda do |intent| + intent['aggregations'] = [{ 'type' => 'sum', 'column' => 'publictotal' }] + end, + 'main-table filter column' => lambda do |intent| + intent['filters'] = [{ 'column' => 'usercode', 'op' => '=', 'param' => 'code' }] + end, + 'related fk_column' => ->(intent) { intent['filters'].first['fk_column'] = 'USER_ID' }, + 'main-table base_column' => ->(intent) { intent['filters'].first['base_column'] = 'ID' }, + 'related filter column' => ->(intent) { intent['filters'].first['related_filters'].first['column'] = 'statuscode' } + }.each do |path, change_case| + it "rejects a case-only PostgreSQL mismatch in the #{path}" do + intent = Marshal.load(Marshal.dump(related_intent)) + change_case.call(intent) + + expect { validator.validate(intent) }.to raise_error(ArgumentError, /not permitted/) + end + end + + %i[mysql sqlite].each do |adapter| + it "retains #{adapter} case-insensitive column semantics" do + config.adapter = adapter + intent = Marshal.load(Marshal.dump(related_intent)) + intent['columns'] = ['usercode'] + intent['filters'].first['fk_column'] = 'USER_ID' + intent['filters'].first['base_column'] = 'ID' + intent['filters'].first['related_filters'].first['column'] = 'statuscode' + + expect { validator.validate(intent) }.not_to raise_error + end + + it "does not Unicode-case-fold #{adapter} policy identifiers" do + config.adapter = adapter + config.policy_adapter = lambda do |_user, **_kwargs| + { allowed_tables: ['kids'], allowed_columns: { 'kids' => ['kind'] } } + end + + expect do + validator.validate({ 'type' => 'select', 'table' => 'Kids', 'columns' => ['kind'] }) + end.to raise_error(ArgumentError, /not permitted by policy/) + expect do + validator.validate({ 'type' => 'select', 'table' => 'kids', 'columns' => ['Kind'] }) + end.to raise_error(ArgumentError, /not permitted/) + end + end + + it 'uses case-insensitive MySQL policy table keys when enforcing columns' do + config.adapter = :mysql + config.policy_adapter = lambda do |_user, **_kwargs| + { allowed_columns: { 'Users' => ['id'] } } + end + intent = { 'type' => 'select', 'table' => 'users', 'columns' => ['secret'] } + + expect { validator.validate(intent) } + .to raise_error(ArgumentError, /selecting column 'secret' not permitted on 'users'/) + end + + context 'with a case-only PostgreSQL policy table-key mismatch' do + before do + config.policy_adapter = lambda do |_user, **_kwargs| + { allowed_columns: { 'Users' => ['id'] } } + end + end + + [ + ['a wildcard SELECT', { 'columns' => ['*'] }], + ['an empty SELECT column list', { 'columns' => [] }], + [ + 'a columnless count aggregation', + { 'columns' => [], 'aggregations' => [{ 'type' => 'count' }] } + ] + ].each do |description, attributes| + it "fails closed for #{description} without relying on a column reference" do + intent = { 'type' => 'select', 'table' => 'users' }.merge(attributes) + + expect { validator.validate(intent) } + .to raise_error(ArgumentError, /policy table key not permitted on 'users'/) + end + end + + { + 'selected column' => ->(intent) { intent['columns'] = ['id'] }, + 'filter column' => lambda do |intent| + intent['filters'] = [{ 'column' => 'id', 'op' => '=', 'param' => 'id' }] + end, + 'ORDER BY column' => lambda do |intent| + intent['order'] = [{ 'column' => 'id', 'dir' => 'asc' }] + end, + 'DISTINCT ON column' => ->(intent) { intent['distinct_on'] = ['id'] }, + 'GROUP BY column' => ->(intent) { intent['group_by'] = ['id'] }, + 'aggregation column' => lambda do |intent| + intent['aggregations'] = [{ 'type' => 'sum', 'column' => 'id' }] + end + }.each do |path, add_column_reference| + it "fails closed for the main-table #{path}" do + intent = { 'type' => 'select', 'table' => 'users', 'columns' => ['*'] } + add_column_reference.call(intent) + + expect { validator.validate(intent) }.to raise_error(ArgumentError, /not permitted on 'users'/) + end + end + + %w[fk_column related_filters].each do |path| + it "fails closed for a related-table #{path} path" do + config.policy_adapter = lambda do |_user, **_kwargs| + { allowed_columns: { 'users' => ['id'], 'Orders' => %w[user_id status] } } + end + intent = { + 'type' => 'select', 'table' => 'users', 'columns' => ['*'], + 'filters' => [{ + 'op' => 'exists', 'related_table' => 'orders', + 'fk_column' => 'user_id', 'base_column' => 'id', + 'related_filters' => [{ 'column' => 'status', 'op' => '=', 'param' => 'status' }] + }] + } + + expect { validator.validate(intent) }.to raise_error(ArgumentError, /not permitted on 'orders'/) + end + end + + it 'rejects a related-table key mismatch before checking its columns' do + config.policy_adapter = lambda do |_user, **_kwargs| + { allowed_columns: { 'users' => ['id'], 'Orders' => [] } } + end + intent = { + 'type' => 'select', 'table' => 'users', 'columns' => ['*'], + 'filters' => [{ + 'op' => 'exists', 'related_table' => 'orders', + 'fk_column' => 'legacy_user_id', 'base_column' => 'id' + }] + } + + expect { validator.validate(intent) } + .to raise_error(ArgumentError, /policy table key not permitted on 'orders'/) + end + + it 'fails closed for a main-table base_column path' do + config.policy_adapter = lambda do |_user, **_kwargs| + { allowed_columns: { 'Users' => ['id'], 'orders' => ['user_id'] } } + end + intent = { + 'type' => 'select', 'table' => 'users', 'columns' => ['*'], + 'filters' => [{ + 'op' => 'exists', 'related_table' => 'orders', + 'fk_column' => 'user_id', 'base_column' => 'id' + }] + } + + expect { validator.validate(intent) }.to raise_error(ArgumentError, /not permitted on 'users'/) + end + + it 'preserves partial policies for truly unrelated absent PostgreSQL table keys' do + config.policy_adapter = lambda do |_user, **_kwargs| + { allowed_columns: { 'accounts' => ['id'] } } + end + intent = { 'type' => 'select', 'table' => 'users', 'columns' => ['secret'] } + + expect { validator.validate(intent) }.not_to raise_error + end + + %i[mysql sqlite].each do |adapter| + it "retains #{adapter} case-insensitive table-key behavior" do + config.adapter = adapter + intent = { 'type' => 'select', 'table' => 'users', 'columns' => ['id'] } + + expect { validator.validate(intent) }.not_to raise_error + end + end + end + end + context 'with order clause' do it 'validates order clause' do intent = { @@ -330,6 +666,17 @@ result = validator.validate(intent) expect(result[:columns]).to eq(['*']) end + + it 'records policy-allowed tables for downstream SQL linting' do + intent = { + 'type' => 'select', + 'table' => 'users', + 'columns' => ['*'] + } + + result = validator.validate(intent) + expect(result[:__policy_allowed_tables]).to eq(%w[users orders]) + end end end @@ -382,8 +729,11 @@ end it 'falls back to simpler call signature' do + allow(adapter).to receive(:call).and_call_original + result = validator.send(:safe_call_policy_adapter, adapter, nil, table: 'orders', intent: {}) expect(result[:allowed_tables]).to eq(['orders']) + expect(adapter).to have_received(:call).once end end @@ -394,6 +744,7 @@ it 'falls back to current-user-only call signature under fail-closed default' do CodeToQuery.config.policy_adapter_fail_open = false + allow(adapter).to receive(:call).and_call_original result = validator.send( :safe_call_policy_adapter, @@ -404,6 +755,7 @@ ) expect(result[:allowed_tables]).to eq(['accounts']) + expect(adapter).to have_received(:call).once end end @@ -430,5 +782,22 @@ expect(result).to eq({}) end end + + context 'when a compatible adapter raises a signature-like ArgumentError internally' do + ['wrong number of arguments', 'unknown keyword: :intent'].each do |message| + it "invokes the adapter once and fails closed for #{message.inspect}" do + calls = 0 + adapter = lambda do |_user, table:, intent:| + calls += 1 + raise ArgumentError, message if table == 'users' && intent + end + + expect do + validator.send(:safe_call_policy_adapter, adapter, nil, table: 'users', intent: {}) + end.to raise_error(CodeToQuery::PolicyAdapterError, /Policy adapter failed: #{Regexp.escape(message)}/) + expect(calls).to eq(1) + end + end + end end end diff --git a/spec/code_to_query_spec.rb b/spec/code_to_query_spec.rb index f583af9..b938da7 100644 --- a/spec/code_to_query_spec.rb +++ b/spec/code_to_query_spec.rb @@ -24,6 +24,43 @@ stub_config(stub_llm: true, provider: :local) end + let(:exists_related_filter) do + { + 'column' => 'id', + 'op' => 'exists', + 'related_table' => 'answers', + 'fk_column' => 'question_id', + 'base_column' => 'id', + 'related_filters' => [] + } + end + + def stub_ask_pipeline(compiled_intent:, allow_tables:, sql:, linter_error: nil) + planner = instance_double(CodeToQuery::Planner) + validator = instance_double(CodeToQuery::Validator) + compiler = instance_double(CodeToQuery::Compiler) + linter = instance_double(CodeToQuery::Guardrails::SqlLinter) + + allow(CodeToQuery::Planner).to receive(:new).and_return(planner) + allow(CodeToQuery::Validator).to receive(:new).and_return(validator) + allow(CodeToQuery::Compiler).to receive(:new).and_return(compiler) + allow(CodeToQuery::Guardrails::SqlLinter).to receive(:new) + .with(described_class.config, allow_tables: allow_tables) + .and_return(linter) + + allow(planner).to receive(:plan).and_return(compiled_intent) + allow(validator).to receive(:validate).and_return(compiled_intent) + allow(compiler).to receive(:compile).and_return( + sql: sql, + params: {}, + bind_spec: [], + intent: compiled_intent + ) + allow(linter).to receive(:check!) do + raise linter_error if linter_error + end + end + # rubocop:disable RSpec/MultipleExpectations,RSpec/ExampleLength it 'emits non-sensitive pipeline instrumentation' do events = [] @@ -69,6 +106,191 @@ expect(query).to be_a(CodeToQuery::Query) end + it 'preserves compiler-augmented intent on the returned query' do + planner = instance_double(CodeToQuery::Planner) + validator = instance_double(CodeToQuery::Validator) + compiler = instance_double(CodeToQuery::Compiler) + linter = instance_double(CodeToQuery::Guardrails::SqlLinter) + + compiled_intent = sample_intent.merge( + 'filters' => [ + { 'column' => 'tenant_id', 'op' => '=', 'param' => 'policy_tenant_id' } + ], + 'params' => { 'policy_tenant_id' => 42 } + ) + + allow(CodeToQuery::Planner).to receive(:new).and_return(planner) + allow(CodeToQuery::Validator).to receive(:new).and_return(validator) + allow(CodeToQuery::Compiler).to receive(:new).and_return(compiler) + allow(CodeToQuery::Guardrails::SqlLinter).to receive(:new).and_return(linter) + + allow(planner).to receive(:plan).and_return(sample_intent) + allow(validator).to receive(:validate).and_return(sample_intent) + allow(compiler).to receive(:compile).and_return( + sql: 'SELECT users.* FROM users WHERE users.tenant_id = $1', + params: { 'policy_tenant_id' => 42 }, + bind_spec: [{ key: 'policy_tenant_id', column: 'tenant_id', cast: nil }], + intent: compiled_intent + ) + allow(linter).to receive(:check!) + + query = described_class.ask(prompt: 'Get users', allow_tables: ['users']) + + expect(query.intent).to eq(compiled_intent) + end + + it 'rejects EXISTS related tables unless they are explicitly allowed' do + compiled_intent = { + 'table' => 'questions', + 'type' => 'select', + 'filters' => [exists_related_filter] + } + + stub_ask_pipeline( + compiled_intent: compiled_intent, + allow_tables: ['questions'], + sql: 'SELECT * FROM "questions" WHERE EXISTS (SELECT 1 FROM "answers" WHERE "answers"."question_id" = "questions"."id")', + linter_error: SecurityError.new("Table 'answers' is not in the allowed list: questions") + ) + + expect { described_class.ask(prompt: 'Get questions', allow_tables: ['questions']) } + .to raise_error(SecurityError, /allowed list/i) + end + + it 'rejects top-level lateral derived tables that reference unallowed tables' do + compiled_intent = { + 'table' => 'questions', + 'type' => 'select', + 'filters' => [exists_related_filter] + } + + stub_ask_pipeline( + compiled_intent: compiled_intent, + allow_tables: ['questions'], + sql: 'SELECT * FROM "questions" JOIN LATERAL (SELECT * FROM "answers") leaked ON TRUE WHERE EXISTS (SELECT 1 FROM "answers" WHERE "answers"."question_id" = "questions"."id")', + linter_error: SecurityError.new('Top-level derived tables are not allowed') + ) + + expect { described_class.ask(prompt: 'Get questions', allow_tables: ['questions']) } + .to raise_error(SecurityError, /derived tables are not allowed/i) + end + + it 'does not invent a partial allowlist when ask is called without allow_tables' do + compiled_intent = { + 'table' => 'questions', + 'type' => 'select', + 'filters' => [exists_related_filter] + } + + stub_ask_pipeline( + compiled_intent: compiled_intent, + allow_tables: nil, + sql: 'SELECT * FROM "questions" WHERE EXISTS (SELECT 1 FROM "answers" WHERE "answers"."question_id" = "questions"."id")' + ) + + expect { described_class.ask(prompt: 'Get questions') }.not_to raise_error + end + + it 'does not widen an explicit caller allowlist through the ask pipeline' do + intent = { + 'table' => 'questions', 'type' => 'select', 'columns' => ['*'], + 'filters' => [exists_related_filter], 'limit' => 100, 'params' => {} + } + planner = instance_double(CodeToQuery::Planner, plan: intent) + validator = instance_double(CodeToQuery::Validator, validate: intent) + allow(CodeToQuery::Planner).to receive(:new).and_return(planner) + allow(CodeToQuery::Validator).to receive(:new).and_return(validator) + described_class.config.policy_adapter = lambda do |_user, **context| + if context[:table] == 'questions' + { allowed_tables: %w[questions answers] } + else + { allowed_tables: ['answers'], enforced_predicates: { tenant_id: 42 } } + end + end + + expect { described_class.ask(prompt: 'Get questions', allow_tables: ['questions']) } + .to raise_error(SecurityError, /allowed list/i) + end + + it 'rejects EXISTS related tables that fall outside the policy adapter allowlist' do + planner = instance_double(CodeToQuery::Planner) + validator = instance_double(CodeToQuery::Validator) + compiler = instance_double(CodeToQuery::Compiler) + + compiled_intent = { + 'table' => 'questions', + 'type' => 'select', + 'filters' => [exists_related_filter], + '__policy_allowed_tables' => ['questions'] + } + + allow(CodeToQuery::Planner).to receive(:new).and_return(planner) + allow(CodeToQuery::Validator).to receive(:new).and_return(validator) + allow(CodeToQuery::Compiler).to receive(:new).and_return(compiler) + allow(planner).to receive(:plan).and_return(compiled_intent) + allow(validator).to receive(:validate).and_return(compiled_intent) + allow(compiler).to receive(:compile).and_return( + sql: 'SELECT * FROM "questions" WHERE EXISTS (SELECT 1 FROM "answers" WHERE "answers"."question_id" = "questions"."id")', + params: {}, + bind_spec: [], + intent: compiled_intent + ) + + expect { described_class.ask(prompt: 'Get questions') } + .to raise_error(SecurityError, /allowed list/i) + end + + context 'with table-scoped policy column metadata for related filters' do + let(:policy_calls) { [] } + + before do + described_class.config.policy_adapter = lambda do |_user, **context| + table = context.fetch(:table) + policy_calls << table + columns = table == 'questions' ? ['id'] : %w[question_id status] + + { allowed_columns: { table => columns } } + end + end + + def ask_with_related_policy_filter(operation:, fk_column: 'question_id', related_columns: ['status']) + intent = { + 'type' => 'select', 'table' => 'questions', 'columns' => ['id'], + 'filters' => [{ + 'op' => operation, 'related_table' => 'answers', + 'fk_column' => fk_column, 'base_column' => 'id', + 'related_filters' => related_columns.map.with_index do |column, index| + { 'column' => column, 'op' => '=', 'param' => "answer_filter_#{index}" } + end + }], + 'params' => related_columns.each_index.to_h { |index| ["answer_filter_#{index}", index] } + } + planner = instance_double(CodeToQuery::Planner, plan: intent) + allow(CodeToQuery::Planner).to receive(:new).and_return(planner) + + described_class.ask( + prompt: 'Get questions by answer attributes', + allow_tables: %w[questions answers] + ) + end + + %w[exists not_exists].each do |operation| + it "rejects a disallowed related fk_column for #{operation}" do + expect do + ask_with_related_policy_filter(operation: operation, fk_column: 'secret_question_id') + end.to raise_error(ArgumentError, /column 'secret_question_id' not permitted on 'answers'/) + expect(policy_calls).to eq(%w[questions questions answers]) + end + + it "checks every related filter column for #{operation}" do + expect do + ask_with_related_policy_filter(operation: operation, related_columns: %w[status secret]) + end.to raise_error(ArgumentError, /filter column 'secret' not permitted on 'answers'/) + expect(policy_calls).to eq(%w[questions questions answers]) + end + end + end + # rubocop:disable RSpec/ExampleLength it 'marks compile/lint telemetry as policy-applied when policy binds are present' do events = [] diff --git a/spec/support/test_helpers.rb b/spec/support/test_helpers.rb index d43ada7..96cbf7c 100644 --- a/spec/support/test_helpers.rb +++ b/spec/support/test_helpers.rb @@ -56,4 +56,23 @@ def sample_intent RSpec.configure do |config| config.include TestHelpers + + # Specs share the production singleton, so restore every setting after each example. + config.around do |example| + singleton_config = CodeToQuery::Configuration.instance + original_state = singleton_config.instance_variables.to_h do |variable| + [variable, singleton_config.instance_variable_get(variable)] + end + + example.run + ensure + if singleton_config && original_state + (singleton_config.instance_variables - original_state.keys).each do |variable| + singleton_config.remove_instance_variable(variable) + end + original_state.each do |variable, value| + singleton_config.instance_variable_set(variable, value) + end + end + end end