pingcap / pingcap/tidb

column masking policy: MASK_PARTIAL has incorrect type signature allowing wrong parameter order

Open
#67,222 2 comments 0 reactions 0 assignees View on GitHub
component/ddl component/expression severity/moderate type/bug
Dominant language
Go
Stars
40.5k
Forks
6.2k
PR merge metrics
PR metrics pending

Description

## Bug Report

The `MASK_PARTIAL` function has an incorrect type signature in its implementation, which allows invalid parameter orders to pass type checking.

## Error/Incorrect Behavior

When creating a masking policy with incorrect parameter order:

\`\`\`sql
-- This INCORRECT syntax currently succeeds (should fail)
CREATE MASKING POLICY p_test ON t(credit_card)
AS MASK_PARTIAL(credit_card, '*', 4, 4);
\`\`\`

**Expected behavior**: Should fail with a type error
**Actual behavior**: Succeeds (but may produce incorrect results at runtime)

The correct syntax should be:
\`\`\`sql
CREATE MASKING POLICY p_test ON t(credit_card)
AS MASK_PARTIAL(credit_card, 4, 4, '*');
\`\`\`

## Root Cause

In `pkg/expression/builtin_masking.go`, the `maskPartialFunctionClass.getFunction` has an incorrect type signature:

\`\`\`go
// Line 337 - INCORRECT (has extra ETString)
bf, err := newBaseBuiltinFuncWithTp(ctx, c.funcName, args,
types.ETString, // args[0]: str
types.ETString, // <-- EXTRA TYPE SIGNATURE (should not exist)
types.ETInt, // args[1]: preserveLeft
types.ETInt, // args[2]: preserveRight
types.ETString) // args[3]: pad
\`\`\`

The function signature should be:
\`\`\`go
bf, err := newBaseBuiltinFuncWithTp(ctx, c.funcName, args,
types.ETString, // args[0]: str
types.ETInt, // args[1]: preserveLeft
types.ETInt, // args[2]: preserveRight
types.ETString) // args[3]: pad
\`\`\`

## Impact

1. **Silent errors**: Users can create masking policies with wrong parameter orders that only fail at runtime
2. **Incorrect masking**: The function may produce unexpected results instead of properly masking data
3. **Poor UX**: Error messages don't catch the issue at policy creation time

## Verification

### Current Behavior (Bug)

\`\`\`sql
CREATE TABLE t(credit_card VARCHAR(20));
CREATE MASKING POLICY p_test ON t(credit_card)
AS MASK_PARTIAL(credit_card, '*', 4, 4);
-- Succeeds (SHOULD FAIL)
\`\`\`

### Expected Behavior (Fix)

\`\`\`sql
CREATE TABLE t(credit_card VARCHAR(20));
CREATE MASKING POLICY p_test ON t(credit_card)
AS MASK_PARTIAL(credit_card, '*', 4, 4);
-- Should fail with type error: argument 2 should be int, got string
\`\`\`

\`\`\`sql
CREATE TABLE t(credit_card VARCHAR(20));
CREATE MASKING POLICY p_test ON t(credit_card)
AS MASK_PARTIAL(credit_card, 4, 4, '*');
-- Should succeed (correct parameter order)
\`\`\`

## Design Documentation

According to \`docs/design/2026-02-27-column-level-masking.md\`:

> - \`MASK_PARTIAL(col, preserve_left, preserve_right, mask_char)\` - Partially masks string values while preserving both ends
> - \`preserve_left\`: Number of leading characters to keep
> - \`preserve_right\`: Number of trailing characters to keep
> - \`mask_char\`: Single character used for masking
> - Example: \`MASK_PARTIAL(credit_card, 6, 4, '*')\` keeps first 6 and last 4 characters

## Fix Required

In \`pkg/expression/builtin_masking.go\`, line 337:

**Current (incorrect):**
\`\`\`go
bf, err := newBaseBuiltinFuncWithTp(ctx, c.funcName, args, types.ETString, types.ETString, types.ETInt, types.ETInt, types.ETString)
\`\`\`

**Should be:**
\`\`\`go
bf, err := newBaseBuiltinFuncWithTp(ctx, c.funcName, args, types.ETString, types.ETInt, types.ETInt, types.ETString)
\`\`\`

Remove the extra \`types.ETString\` in the type signature.

## Test Cases Needed

1. Test that \`MASK_PARTIAL(col, 'char', 1, 1)\` fails with type error
2. Test that \`MASK_PARTIAL(col, 1, 1, 'char')\` succeeds
3. Test that \`MASK_PARTIAL(col, 'char', 'char', 1)\` fails with type error
4. Verify actual masking behavior works correctly with proper parameters

## Component

- \`component/expression\`
- \`component/ddl\`

## Related Files

- Implementation: \`pkg/expression/builtin_masking.go\` (line 337)
- Test: \`pkg/expression/builtin_masking_test.go\`
- Design: \`docs/design/2026-02-27-column-level-masking.md\`

## Related Issues

- #67217: Policy name uniqueness test coverage
- #67218: CTE test coverage
- #67219: CREATE OR REPLACE test coverage
- #67221: Dynamic privileges not implemented

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.