#444

Reviewed-on: #441

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'],

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
 

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

backend/app/services/counts_table_query.rb

@@ -220,19 +220,12 @@ class CountsTableQuery
 
   private
 
-  # Public timeline predicates may include hidden records. Private predicates must
+  # Summary predicates are evaluated against public coordinates or the masked
-  # never become a count oracle, even when the selected axes themselves are public.
+  # 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.

backend/db/seeds.rb

@@ -9,8 +9,21 @@
 #   end
 
 ApplicationRecord.transaction do
-  Language.find_or_create_by!(code: 'ain') { _1.name = 'ain' }
+  Language.find_or_create_by!(code: 'ja') { _1.name = '日本語' }
-  Script.find_or_create_by!(code: 'Kana') { _1.name = 'Kana' }
+  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 = [

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
+  it 'allows public identity predicates without exposing private snapshot fields' do
-    get '/counts/query', params: base.merge(mode: 'summary', resource_id: record.id.to_s)
+    PostVersion.create!(post: record, version_no: 4, event_type: 'discard',
-    expect(response).to have_http_status(:forbidden)
+        url: record.url, title: 'hidden title', tags: '', tags_json: [],
-    expect(json).to eq('error' => 'private_condition')
+        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

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.map(&:to_i)).to eq([1, 2, 10])
-    expect(values.last).to eq(CountsAxes::MASKED_VALUE)
   end
 
-  it 'rejects private predicates and supplied measure coordinates before SQL labels escape' do
+  it 'uses public identity fields and masked values for unauthorised filters' do
-    probes = [{ 'resource_id' => PostVersion.first.post_id.to_s },
+    hidden_id = PostVersion.first.post_id.to_s
-        { 'tags' => 'A' },
+    base = { 'rows' => '', 'cols' => '' }
-        { 'conditions' => '', 'conditions.0.field' => 'title', 'conditions.0.operator' => 'eq',
+    condition = lambda do |field, operator, value|
-          'conditions.0.value' => 'hidden-title-0' },
+      { 'conditions' => '', 'conditions.0.field' => field,
-        { 'drill' => '', 'drill.0.field' => 'tags', 'drill.0.value_type' => 'string',
+        'conditions.0.operator' => operator, 'conditions.0.value' => value }
-          'drill.0.value' => "internal:#{ tags.first.id }" },
+    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