diff --git a/backend/app/models/history_access.rb b/backend/app/models/history_access.rb index 9e73210..0d9f92a 100644 --- a/backend/app/models/history_access.rb +++ b/backend/app/models/history_access.rb @@ -4,7 +4,7 @@ require 'digest' class HistoryAccess < ActiveSupport::CurrentAttributes attribute :user class Forbidden < StandardError; end - ARTIFACT_POLICY = 'counts-masked-summary-1'.freeze + ARTIFACT_POLICY = 'counts-masked-summary-2'.freeze KEYS = { 'post_versions' => ['post_id'], 'tag_versions' => ['tag_id'], 'nico_tag_versions' => ['tag_id'], diff --git a/backend/app/services/counts_axes.rb b/backend/app/services/counts_axes.rb index 3bb8dfb..4267e31 100644 --- a/backend/app/services/counts_axes.rb +++ b/backend/app/services/counts_axes.rb @@ -1,7 +1,8 @@ class CountsAxes MAX_AXES = 8 MASKED_VALUE = '__counts_private__'.freeze - PUBLIC_FIELDS = ['created_at', 'event_type'].freeze + PUBLIC_FIELDS = ['resource_id', 'version_no', 'event_type', 'user', + 'created_at'].freeze class TooManyAxes < ArgumentError; end class PrivateCondition < ArgumentError; end diff --git a/backend/app/services/counts_population.rb b/backend/app/services/counts_population.rb index 3776444..e5dfda2 100644 --- a/backend/app/services/counts_population.rb +++ b/backend/app/services/counts_population.rb @@ -192,7 +192,88 @@ class CountsPopulation def apply_condition(entry) @condition ||= CountsCondition.new(self) - @scope = @condition.apply(@scope, entry) + unless summary_private_condition?(entry) + @scope = @condition.apply(@scope, entry) + return + end + + return if masked_noop?(entry) + + # Evaluate private predicates only on authorised versions. On other versions, + # the entire field is the same opaque marker, regardless of its stored value. + original = @scope + visible = private_visibility_sql(entry['field']) + authorised = authorised_private_condition(original.where(Arel.sql(visible)), entry) + @scope = if masked_match?(entry) + authorised.or(original.where(Arel.sql("NOT (#{ visible })"))) + else + authorised + end + end + + def authorised_private_condition(scope, entry) + marker = '********' + if entry['operator'] == 'axis_eq' + return scope.none if entry['value'] == marker && + definition.fields[entry['field']] != :string + + return @condition.apply(scope, entry) + end + + field = definition.filter_metadata.find { _1[:key] == entry['field'] } + return @condition.apply(scope, entry) unless field + return @condition.apply(scope, entry) if field[:type] == :string && + field[:set_type] != 'tag_reference' + + if entry['value'] == marker + return scope.none if ['eq', 'gt', 'ge', 'lt', 'le', 'range'].include?(entry['operator']) + return scope if entry['operator'] == 'ne' + end + if entry['values'].is_a?(Array) && entry['values'].include?(marker) + return scope.none if entry['operator'] == 'all' + + return @condition.apply(scope, entry.merge('values' => entry['values'] - [marker])) + end + @condition.apply(scope, entry) + end + + def summary_private_condition?(entry) + params.fetch('mode', 'summary') == 'summary' && !@authorisation_flg && + !HistoryAccess.admin? && !CountsAxes::PUBLIC_FIELDS.include?(entry['field']) && + entry['field'] != 'user.id' + end + + def private_visibility_sql(field) + visible = HistoryAccess.sql(definition.model, 'v') + return visible unless field == 'changed_field' + + "(#{ visible }) AND (#{ HistoryAccess.sql(definition.model, 'previous') })" + end + + def masked_noop?(entry) + return false if entry['operator'] == 'axis_eq' || entry['blank'] + set = ['in', 'not_in', 'any', 'all', 'none'].include?(entry['operator']) + return !entry.key?('values') if set + + [entry['value'], entry['end']].all? { _1.nil? || _1 == '' } + end + + def masked_match?(entry) + marker = '********' + return false if entry['blank'] + + value = entry['value'] + case entry['operator'] + when 'axis_eq', 'eq' then value == marker + when 'ne' then value != marker + when 'like' + pattern = Regexp.escape(value.to_s).gsub('%', '.*').gsub('_', '.') + /\A#{ pattern }\z/i.match?(marker) + when 'in', 'any' then entry.fetch('values', []).include?(marker) + when 'not_in', 'none' then !entry.fetch('values', []).include?(marker) + when 'all' then entry.fetch('values', []).all? { _1 == marker } + else false + end end def snapshot_membership(value) @@ -209,6 +290,10 @@ class CountsPopulation def apply_tags raise ArgumentError unless definition.resource == 'posts' + private = params.fetch('mode', 'summary') == 'summary' && !@authorisation_flg && + !HistoryAccess.admin? + original = @scope if private + @scope = HistoryAccess.visible(@scope, 'v') if private if params['tag'].present? @scope = @scope.where(snapshot_membership( "#{ params.fetch('kind', 'internal') }:#{ params['tag'] }")) @@ -232,9 +317,21 @@ class CountsPopulation expression = parts.empty? ? 'FALSE' : "(#{ parts.join(' OR ') })" literal[:negative] ? "NOT #{ expression }" : expression end - return if predicates.empty? + if predicates.empty? + @scope = original if private && params['tag'].blank? + return + end any = params['match'] == 'any' || raw.split.any? { _1.downcase == 'type:or' } @scope = @scope.where(Arel.sql("(#{ predicates.join(any ? ' OR ' : ' AND ') })")) + return unless private && params['tag'].blank? + + # A masked tag collection cannot match a real tag. Negative terms match + # the opaque marker independently of any hidden snapshot membership. + hidden = any ? literals.any? { _1[:negative] } : literals.all? { _1[:negative] } + if hidden + visible = HistoryAccess.sql(definition.model, 'v') + @scope = @scope.or(original.where(Arel.sql("NOT (#{ visible })"))) + end end end diff --git a/backend/app/services/counts_table_query.rb b/backend/app/services/counts_table_query.rb index 92acf8c..9f96901 100644 --- a/backend/app/services/counts_table_query.rb +++ b/backend/app/services/counts_table_query.rb @@ -220,19 +220,12 @@ class CountsTableQuery private - # Public timeline predicates may include hidden records. Private predicates must - # never become a count oracle, even when the selected axes themselves are public. + # Summary predicates are evaluated against public coordinates or the masked + # value by CountsPopulation. Never accept a private server-only coordinate in + # user-supplied ordering/measure state. def validate_summary_conditions! return if mode != 'summary' || HistoryAccess.admin? - fields = QueryState.records(@params, 'conditions') + QueryState.records(@params, 'drill') - private_fields = fields.any? { !CountsAxes::PUBLIC_FIELDS.include?(_1['field']) } - keys = ['tags', 'tag', 'kind', 'resource_id', 'user_id'] - private_parameters = keys.any? do |key| - @params.any? { |name, value| (name == key || name.start_with?("#{ key }.")) && value != '' } - end - raise CountsAxes::PrivateCondition if private_fields || private_parameters - ['column_order', 'measures'].each do |key| # Coordinates are compared only to already masked expressions, never raw # values. Public coordinates remain sortable; a guessed private ID cannot match. diff --git a/backend/db/seeds.rb b/backend/db/seeds.rb index 8b1e0ad..1259133 100644 --- a/backend/db/seeds.rb +++ b/backend/db/seeds.rb @@ -9,8 +9,21 @@ # end ApplicationRecord.transaction do - Language.find_or_create_by!(code: 'ain') { _1.name = 'ain' } - Script.find_or_create_by!(code: 'Kana') { _1.name = 'Kana' } + Language.find_or_create_by!(code: 'ja') { _1.name = '日本語' } + Script.find_or_create_by!(code: 'Jpan') { _1.name = '漢字、仮名文字およびラテン文字' } + Region.find_or_create_by!(code: 392) do + _1.alpha_code = 'JP' + _1.name = '日本国' + end + Locale.find_or_create_by!(code: 'ja') do + _1.language_code = 'ja' + _1.script_code = 'Jpan' + _1.region_code = 392 + _1.name = '日本語' + end + + Language.find_or_create_by!(code: 'ain') { _1.name = 'ヤウンモシルン アイヌ イタㇰ' } + Script.find_or_create_by!(code: 'Kana') { _1.name = 'カナモジ オヨビ ラテンモジ' } end post_url_sanitisation_rules = [ diff --git a/backend/spec/requests/counts_table_spec.rb b/backend/spec/requests/counts_table_spec.rb index 5e13aac..234092c 100644 --- a/backend/spec/requests/counts_table_spec.rb +++ b/backend/spec/requests/counts_table_spec.rb @@ -354,15 +354,25 @@ RSpec.describe 'Counts table API', type: :request do expect(result['cells'].sum { _1['count'] }).to eq(4) expect(response.body).not_to include('hidden title') masked = query(mode: 'summary', rows: 'resource_id', cols: '') - expect(masked['cells'].map { _1['values'] }).to eq([[CountsAxes::MASKED_VALUE]]) + expect(masked['cells'].map { _1['values'] }).to eq([[record.id]]) expect(masked['cells'].first['count']).to eq(4) expect(query['versions']).to be_empty end - it 'rejects private summary predicates instead of exposing a count oracle' do - get '/counts/query', params: base.merge(mode: 'summary', resource_id: record.id.to_s) - expect(response).to have_http_status(:forbidden) - expect(json).to eq('error' => 'private_condition') + it 'allows public identity predicates without exposing private snapshot fields' do + PostVersion.create!(post: record, version_no: 4, event_type: 'discard', + url: record.url, title: 'hidden title', tags: '', tags_json: [], + parent_post_ids: '', created_at: Time.utc(2024, 3)) + + get '/counts/query', params: base.merge(mode: 'summary', + resource_id: record.id.to_s, rows: 'resource_id', cols: 'title') + + expect(response).to have_http_status(:ok) + expect(json['count']).to eq(4) + expect(json['cells'].sum { _1['count'] }).to eq(4) + expect(json['cells'].map { _1['values'].last }) + .to include(CountsAxes::MASKED_VALUE) + expect(response.body).not_to include('hidden title') end it 'reports the combined axis limit without replacing it with a generic 400' do diff --git a/backend/spec/services/counts_pivot_security_spec.rb b/backend/spec/services/counts_pivot_security_spec.rb index 337d7a5..8427e54 100644 --- a/backend/spec/services/counts_pivot_security_spec.rb +++ b/backend/spec/services/counts_pivot_security_spec.rb @@ -142,7 +142,7 @@ RSpec.describe 'Counts masked pivot contracts' do expect(page[:row_headers]).to include(hash_including(values: [nil], kind: 'data')) end - it 'sorts public numeric values numerically alongside a masked group' do + it 'sorts public version numbers numerically including formerly hidden records' do post = PostVersion.order(:id).last.post [2, 10].each do |number| PostVersion.create!(post:, version_no: number, event_type: 'update', @@ -151,22 +151,47 @@ RSpec.describe 'Counts masked pivot contracts' do end page = result(nil, 'rows' => 'version_no', 'cols' => '') values = page[:row_headers].select { _1[:kind] == 'data' }.map { _1[:values].first } - expect(values.reject { _1 == CountsAxes::MASKED_VALUE }.map(&:to_i)).to eq([1, 2, 10]) - expect(values.last).to eq(CountsAxes::MASKED_VALUE) + expect(values.map(&:to_i)).to eq([1, 2, 10]) end - it 'rejects private predicates and supplied measure coordinates before SQL labels escape' do - probes = [{ 'resource_id' => PostVersion.first.post_id.to_s }, - { 'tags' => 'A' }, - { 'conditions' => '', 'conditions.0.field' => 'title', 'conditions.0.operator' => 'eq', - 'conditions.0.value' => 'hidden-title-0' }, - { 'drill' => '', 'drill.0.field' => 'tags', 'drill.0.value_type' => 'string', - 'drill.0.value' => "internal:#{ tags.first.id }" }, + it 'uses public identity fields and masked values for unauthorised filters' do + hidden_id = PostVersion.first.post_id.to_s + base = { 'rows' => '', 'cols' => '' } + condition = lambda do |field, operator, value| + { 'conditions' => '', 'conditions.0.field' => field, + 'conditions.0.operator' => operator, 'conditions.0.value' => value } + end + + expect(result(nil, base.merge('resource_id' => hidden_id))[:cells].sole[:count]).to eq(2) + expect(result(nil, base.merge(condition.call('version_no', 'eq', '2')))[:cells] + .sole[:count]).to eq(2) + expect(result(nil, base.merge(condition.call('user', 'eq', member.id.to_s)))[:cells] + .sole[:count]).to eq(5) + expect(result(nil, base.merge(condition.call('title', 'eq', 'hidden-title-0')))[:cells] + .sole[:count]).to eq(0) + expect(result(nil, base.merge(condition.call('title', 'ne', 'hidden-title-0')))[:cells] + .sole[:count]).to eq(5) + expect(result(nil, base.merge(condition.call('title', 'eq', '********')))[:cells] + .sole[:count]).to eq(4) + expect(result(nil, base.merge(condition.call('video_ms', 'eq', '********')))[:cells] + .sole[:count]).to eq(4) + expect(result(nil, base.merge(condition.call('title', 'like', '%title%')))[:cells] + .sole[:count]).to eq(1) + expect(result(nil, base.merge(condition.call('title', 'like', '%')))[:cells] + .sole[:count]).to eq(5) + expect(result(nil, base.merge('tags' => 'C'))[:cells].sole[:count]).to eq(1) + expect(result(nil, base.merge('tags' => 'not:C'))[:cells].sole[:count]).to eq(4) + end + + it 'rejects server-only coordinates in column order and drilldown' do + probes = [ { 'cols' => 'tags', 'column_order' => '', 'column_order.0.value_type' => 'string', - 'column_order.0.value' => CountsAxes::MASKED_VALUE }] + 'column_order.0.value' => CountsAxes::MASKED_VALUE }, + { 'drill' => '', 'drill.0.field' => 'tags', 'drill.0.value_type' => 'string', + 'drill.0.value' => CountsAxes::MASKED_VALUE }] HistoryAccess.set(user: nil) do probes.each do |probe| - expect { CountsTableQuery.new(params.merge(probe)) } + expect { CountsTableQuery.new(params.merge(probe)).call } .to raise_error(CountsAxes::PrivateCondition) end end