Skip to content

fix(query_builder): don't explode plucked scalars on masked doctypes#1

Merged
pipech merged 2 commits into
thspacecode:version-16-hotfixfrom
spc-agent-00:fix/get-values-pluck-masked-fields
Jun 11, 2026
Merged

fix(query_builder): don't explode plucked scalars on masked doctypes#1
pipech merged 2 commits into
thspacecode:version-16-hotfixfrom
spc-agent-00:fix/get-values-pluck-masked-fields

Conversation

@spc-agent-00

Copy link
Copy Markdown
Collaborator

Problem

Fixes frappe#39898

For non-admin users, frappe.db.get_values(..., pluck=True) returns corrupted data when the queried doctype has a masked field — instead of a list of strings it returns a list of per-character tuples:

# Expected (and what Administrator gets):
['P-1156']

# What a non-admin user got:
[('P', '-', '1', '1', '5', '6')]

For numeric plucked fields it was worse — it raised TypeError: 'int' object is not iterable.

Root cause

query_builder/utils.py::execute_query() runs mask_fields() on the result whenever the doctype has masked fields for the current user (get_masked_fields() returns [] for Administrator, which is why admins were unaffected).

With pluck=True the database layer has already reduced each row to a scalar ([r[0] for r in rows]), so result is a flat list of values, not a list of rows. But mask_fields() only knew the as_dict / as_list shapes, so it fell through to mask_list_results(), which does row = list(row) on each element:

  • list("P-1156")['P', '-', '1', '1', '5', '6']('P', '-', ...)
  • list(42)TypeError: 'int' object is not iterable

Fix

Thread a pluck flag through mask_fields() and handle it before the dict/tuple paths with a new mask_pluck_results() helper. Because pluck returns a single column, the helper masks each scalar only when the plucked field is itself a masked field, and otherwise returns values untouched:

  • pluck a non-masked field (e.g. name) → unchanged scalar list
  • pluck a masked field → list of masked scalar values (e.g. ['XXXXXXXX'])

The non-pluck as_dict / as_list masking paths are unchanged.

Tests

Added test_get_values_pluck_with_masked_fields to frappe/tests/test_db_query.py, covering, for a non-admin user on a doctype with a masked field:

  • pluck of a non-masked string field returns the scalar (regression for the char-tuple bug)
  • pluck of a non-masked int field does not raise (regression for the TypeError)
  • pluck of the masked field returns the masked value
  • Administrator remains unmasked

Verification

The fix was verified behaviorally on a live site (before → after):

call before fix after fix
pluck name [('e','d','0',...)] ['ed0pmvgutd']
pluck amount (Int) TypeError [42]
pluck masked secret [('XXXXXXXX','-','1',...)] ['XXXXXXXX']
as_dict / as_list (non-pluck) masked correctly masked correctly (unchanged)

🤖 Generated with Claude Code

`execute_query` runs `mask_fields()` on every result whenever the queried
doctype has masked fields for the current (non-admin) user. With `pluck=True`
the DB layer has already reduced each row to a scalar, so the result is a flat
list of values rather than rows. `mask_list_results` then ran `list(row)` on
each scalar, turning "P-1156" into `('P', '-', '1', '1', '5', '6')` for strings
and raising `TypeError: 'int' object is not iterable` for numeric fields.

Handle `pluck` explicitly: mask each scalar only when the plucked field is
itself a masked field, otherwise return the value untouched. Admin users were
never affected since `get_masked_fields()` returns nothing for them.

Fixes frappe#39898

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment thread frappe/model/utils/mask.py Outdated
Address review feedback on frappe#39898: annotate the helper's arguments and
return type, matching the style of mask_fields().

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@pipech pipech merged commit 815c0b3 into thspacecode:version-16-hotfix Jun 11, 2026
spc-agent-00 added a commit that referenced this pull request Jun 15, 2026
DatabaseQuery.execute returned the plucked scalar list before running
mask_fields, so a masked field leaked its raw value for non-admin users
when callers asked for a flat pluck list. Mask the dict rows first, then
pluck, reusing the existing mask_dict_results path.

This is the legacy db_query counterpart to the query-builder pluck fix in
#1. No core caller routes pluck through this engine on
16.12.2 (get_list/get_all/reportview use qb_query), so this covers direct
DatabaseQuery callers and any code still on the legacy path.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants