Skip to content

Commit 63bcbf6

Browse files
authored
fix(pii): post-merge review fixes + live NER e2e for the privacy-filter tier (#10401)
* fix(pii): post-merge review fixes + live NER e2e for the privacy-filter tier Follow-up to the NER tier engine (#10360), already on master. This carries only the incremental review fixes and tests that postdate that merge — the feature itself is not re-introduced. Review fixes: - openai_completion.go: remove the dead `elem >= 0` conjunct in applyAnyText (the `elem < 0` guard above already returns). - application.go: collapse ResolvePIIPolicy's inline re-implementation of PIIIsEnabled to a single cfg.PIIIsEnabled() call (sole source of the "explicit pii.enabled wins, else cloud-proxy default" rule) and return true past the !enabled guard where it is provable. - pattern.go: hoist the triple `appConfig != nil && EnableTracing` check in patternDetector.Detect into one local. - grammar.go: MaxQuantifier was 4096, but Go's regexp/syntax rejects repeat bounds above 1000 at Parse time, so walk()'s {n,m} guard could never fire — dead code shadowed by the parser. Lower it to 512 so a bound in (512,1000] is rejected here with an actionable error; >1000 still fails closed via Parse. Specs pin the relationship so the guard can't silently revert. - PatternListEditor.jsx: clamp a directly-typed negative min_len to >=0 and force the DOM value back when clamping (min={0} only constrained the spinner, so a negative reached saved config and silently disabled the length filter). Tests: - piipattern_test.go: MaxQuantifier guard specs (must stay live, not dead). - model-config.spec.js: assert the min_len clamp, and that entity_actions collapses a duplicate group to a single row (map semantics; regression guard against emitting an array that drops a row on save). - tests/e2e-backends: token_classify capability driving the TokenClassify gRPC RPC against the backend image, asserting byte-correct, UTF-8 rune-aligned spans (entity.Text == text[start:end]) at threshold 0. Verified on CPU via `make test-extra-backend-privacy-filter` (3/3 specs). - Makefile: test-extra-backend-privacy-filter wrapper. - tests/e2e: e2e_pii_ner_test.go drives /api/pii/analyze + /api/pii/redact (mask + block) through the full HTTP -> detector -> redactor path; gated on PII_NER_MODEL_GGUF so the default suite is unaffected. - .github/workflows/tests-pii-ner-e2e.yml: path-filtered / nightly CI job running the container harness on CPU. Assisted-by: Claude:claude-opus-4-8 [Claude Code] Signed-off-by: Richard Palethorpe <io@richiejp.com> * feat(gallery): add privacy-filter-nemotron (f16 + q8) GGUF conversions of OpenMed/privacy-filter-nemotron — a fine-grained English PII token-classifier (55 categories / 221 BIOES classes), fine-tuned from openai/privacy-filter on NVIDIA's Nemotron-PII dataset. Sibling to the existing privacy-filter-multilingual entry, trading language breadth for category depth. - privacy-filter-nemotron: F16 reference artifact (~2.8 GB). - privacy-filter-nemotron-q8: Q8_0 quant (~1.64 GB) for RAM-constrained / edge use; description notes the size/speed tradeoff and to validate on your own data (a single dropped span is a PII leak). Both run on the privacy-filter backend with known_usecases [token_classify] and a default mask policy (min_score 0.5); operators add per-category entity_actions as needed. sha256s taken from the HF repo's LFS object ids. Assisted-by: Claude:claude-opus-4-8 [Claude Code] Signed-off-by: Richard Palethorpe <io@richiejp.com> --------- Signed-off-by: Richard Palethorpe <io@richiejp.com>
1 parent 95b058e commit 63bcbf6

13 files changed

Lines changed: 608 additions & 35 deletions

File tree

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
---
2+
name: 'PII NER tier E2E (live GGUF, CPU)'
3+
4+
# Runs the real privacy-filter GGUF NER tier end-to-end on CPU — the gap the
5+
# hermetic tests/e2e suite cannot cover (it only exercises the in-process
6+
# pattern tier). Heavy (builds the C++ backend image + downloads a ~2.7 GB
7+
# GGUF), so it is path-filtered on PRs and otherwise runs nightly / on demand.
8+
#
9+
# This drives the container-level harness (tests/e2e-backends) via
10+
# `make test-extra-backend-privacy-filter`: it builds the privacy-filter image,
11+
# downloads the model, loads it on CPU, and asserts byte-correct, UTF-8-aligned
12+
# TokenClassify spans. The complementary HTTP-path specs in tests/e2e
13+
# (e2e_pii_ner_test.go) Skip unless PII_NER_MODEL_GGUF is wired.
14+
15+
on:
16+
workflow_dispatch:
17+
schedule:
18+
- cron: '0 3 * * *'
19+
push:
20+
branches:
21+
- master
22+
paths:
23+
- 'backend/cpp/privacy-filter/**'
24+
- 'backend/Dockerfile.privacy-filter'
25+
- 'core/services/routing/pii/**'
26+
- 'core/services/routing/piidetector/**'
27+
- 'core/backend/token_classify.go'
28+
- 'core/http/endpoints/localai/pii.go'
29+
- 'core/schema/pii.go'
30+
- 'tests/e2e-backends/**'
31+
- 'tests/e2e/e2e_pii_ner_test.go'
32+
- 'tests/e2e/e2e_suite_test.go'
33+
- '.github/workflows/tests-pii-ner-e2e.yml'
34+
pull_request:
35+
paths:
36+
- 'backend/cpp/privacy-filter/**'
37+
- 'backend/Dockerfile.privacy-filter'
38+
- 'core/services/routing/pii/**'
39+
- 'core/services/routing/piidetector/**'
40+
- 'core/backend/token_classify.go'
41+
- 'core/http/endpoints/localai/pii.go'
42+
- 'core/schema/pii.go'
43+
- 'tests/e2e-backends/**'
44+
- 'tests/e2e/e2e_pii_ner_test.go'
45+
- 'tests/e2e/e2e_suite_test.go'
46+
- '.github/workflows/tests-pii-ner-e2e.yml'
47+
48+
concurrency:
49+
group: ci-tests-pii-ner-e2e-${{ github.event.pull_request.number || github.sha }}-${{ github.repository }}
50+
cancel-in-progress: ${{ github.event_name == 'pull_request' }}
51+
52+
jobs:
53+
tests-pii-ner-e2e:
54+
runs-on: ubuntu-latest
55+
strategy:
56+
matrix:
57+
go-version: ['1.25.x']
58+
steps:
59+
- name: Clone
60+
uses: actions/checkout@v6
61+
with:
62+
submodules: true
63+
- name: Free disk space
64+
run: |
65+
sudo rm -rf /usr/share/dotnet /usr/local/lib/android /opt/ghc /opt/hostedtoolcache/CodeQL || true
66+
sudo docker image prune --all --force || true
67+
df -h
68+
- name: Configure apt mirror on runner
69+
uses: ./.github/actions/configure-apt-mirror
70+
- name: Setup Go ${{ matrix.go-version }}
71+
uses: actions/setup-go@v5
72+
with:
73+
go-version: ${{ matrix.go-version }}
74+
cache: false
75+
- name: Proto Dependencies
76+
run: |
77+
curl -L -s https://github.com/protocolbuffers/protobuf/releases/download/v26.1/protoc-26.1-linux-x86_64.zip -o protoc.zip && \
78+
unzip -j -d /usr/local/bin protoc.zip bin/protoc && \
79+
rm protoc.zip
80+
go install google.golang.org/protobuf/cmd/protoc-gen-go@v1.34.2
81+
go install google.golang.org/grpc/cmd/protoc-gen-go-grpc@1958fcbe2ca8bd93af633f11e97d44e567e945af
82+
PATH="$PATH:$HOME/go/bin" make protogen-go
83+
- name: Dependencies
84+
run: |
85+
sudo apt-get update
86+
sudo apt-get install -y build-essential
87+
# Builds local-ai-backend:privacy-filter, downloads the GGUF, loads it on
88+
# CPU and runs the token_classify capability spec (byte-offset contract).
89+
- name: Run live PII NER backend E2E
90+
run: PATH="$PATH:$HOME/go/bin" make test-extra-backend-privacy-filter
91+
- name: Setup tmate session if tests fail
92+
if: ${{ failure() }}
93+
uses: mxschmitt/action-tmate@v3.23
94+
with:
95+
detached: true
96+
connect-timeout-seconds: 180
97+
limit-access-to-actor: true

Makefile

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -690,6 +690,16 @@ test-extra-backend-llama-cpp-transcription: docker-build-llama-cpp
690690
BACKEND_TEST_CTX_SIZE=2048 \
691691
$(MAKE) test-extra-backend
692692

693+
## privacy-filter: the PII/NER token-classification backend. Exercises the
694+
## TokenClassify RPC and asserts byte-correct, UTF-8-aligned span offsets
695+
## against the openai-privacy-filter multilingual GGUF (CPU-runnable, ~50M
696+
## active params). This is the live-backend coverage for the PII NER tier.
697+
test-extra-backend-privacy-filter: docker-build-privacy-filter
698+
BACKEND_IMAGE=local-ai-backend:privacy-filter \
699+
BACKEND_TEST_MODEL_URL=https://huggingface.co/LocalAI-io/privacy-filter-multilingual-GGUF/resolve/main/privacy-filter-multilingual-f16.gguf \
700+
BACKEND_TEST_CAPS=health,load,token_classify \
701+
$(MAKE) test-extra-backend
702+
693703
## vllm is resolved from a HuggingFace model id (no file download) and
694704
## exercises Predict + streaming + tool-call extraction via the hermes parser.
695705
## Requires a host CPU with the SIMD instructions the prebuilt vllm CPU

core/application/application.go

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -341,11 +341,9 @@ func (a *Application) ResolvePIIPolicy(cfg *config.ModelConfig) (enabled bool, d
341341
}
342342
appCfg := a.ApplicationConfig()
343343

344-
if cfg.PII.Enabled != nil {
345-
enabled = *cfg.PII.Enabled
346-
} else {
347-
enabled = cfg.PIIIsEnabled() // backend default (cloud-proxy)
348-
}
344+
// PIIIsEnabled already encodes "explicit pii.enabled wins, else backend
345+
// default (cloud-proxy)" — the single source of that rule.
346+
enabled = cfg.PIIIsEnabled()
349347
if !enabled {
350348
return false, nil
351349
}
@@ -354,7 +352,7 @@ func (a *Application) ResolvePIIPolicy(cfg *config.ModelConfig) (enabled bool, d
354352
if len(detectors) == 0 {
355353
detectors = append([]string(nil), appCfg.PIIDefaultDetectors...)
356354
}
357-
return enabled, detectors
355+
return true, detectors // enabled is necessarily true past the !enabled guard
358356
}
359357

360358
// PIIPolicyResolver adapts ResolvePIIPolicy to pii.PolicyResolver for

core/http/react-ui/e2e/model-config.spec.js

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,21 @@ test.describe('Model Editor - Interactive Tab', () => {
288288
await expect(page.locator('input[placeholder^="match,"]')).toBeVisible()
289289
})
290290

291+
test('pattern min_len clamps a directly-typed negative to 0', async ({ page }) => {
292+
const searchInput = page.locator('input[placeholder="Search fields to add..."]')
293+
await searchInput.fill('Custom Secret Patterns')
294+
const dropdown = searchInput.locator('..').locator('..')
295+
await dropdown.locator('div', { hasText: 'Custom Secret Patterns' }).first().click()
296+
297+
await page.locator('button', { hasText: 'Add pattern' }).click()
298+
// The number input's min={0} only limits the spinner arrows, not keyboard
299+
// entry; the editor must sanitise a typed negative so a meaningless
300+
// negative length floor never reaches the saved config.
301+
const minLen = page.locator('input[aria-label="Minimum length"]')
302+
await minLen.fill('-5')
303+
await expect(minLen).toHaveValue('0')
304+
})
305+
291306
// Regression: a map-typed field (entity_actions) present in the loaded YAML
292307
// must render WITH its values. flattenConfig used to recurse into the map,
293308
// scattering it across pii_detection.entity_actions.<GROUP> paths that match
@@ -329,4 +344,37 @@ test.describe('Model Editor - Interactive Tab', () => {
329344
await expect(page.getByText(/block /i).first()).toBeVisible()
330345
})
331346

347+
// A map cannot hold two values for one key, so renaming a row to an existing
348+
// group must collapse to a single row (Object.fromEntries, last write wins)
349+
// rather than rendering two conflicting rows that silently lose one on save.
350+
test('entity_actions collapses a duplicate group to a single row', async ({ page }) => {
351+
await page.route('**/api/models/edit/ner-model', (route) => {
352+
route.fulfill({
353+
contentType: 'application/json',
354+
body: JSON.stringify({
355+
name: 'ner-model',
356+
config: [
357+
'name: ner-model',
358+
'backend: llama-cpp',
359+
'pii_detection:',
360+
' entity_actions:',
361+
' SSN: block',
362+
' EMAIL: mask',
363+
'',
364+
].join('\n'),
365+
}),
366+
})
367+
})
368+
369+
await page.goto('/app/model-editor/ner-model')
370+
371+
const groupInputs = page.locator('input[aria-label="Entity group"]')
372+
await expect(groupInputs).toHaveCount(2)
373+
374+
// Rename the EMAIL row to duplicate SSN; the editor collapses to one SSN row.
375+
await groupInputs.nth(1).fill('SSN')
376+
await expect(groupInputs).toHaveCount(1)
377+
await expect(groupInputs.nth(0)).toHaveValue('SSN')
378+
})
379+
332380
})

core/http/react-ui/src/components/PatternListEditor.jsx

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,18 @@ export default function PatternListEditor({ value, onChange }) {
7474
min={0}
7575
value={r.min_len || 0}
7676
title="Minimum match length (0 = no floor)"
77-
onChange={e => update(i, { min_len: parseInt(e.target.value, 10) || 0 })}
77+
// min={0} only constrains the spinner, not keyboard entry. Clamp a
78+
// typed negative to 0 (a negative floor is meaningless and would
79+
// disable the length filter). When we clamp, force the DOM value
80+
// too: the resulting 0->0 state change is a no-op, so React's
81+
// controlled input would otherwise keep displaying the rejected
82+
// "-5" even though the saved value is 0.
83+
onChange={e => {
84+
const parsed = parseInt(e.target.value, 10)
85+
const n = Math.max(0, parsed || 0)
86+
if (parsed < 0) e.target.value = String(n)
87+
update(i, { min_len: n })
88+
}}
7889
style={{ width: 80, fontSize: '0.8125rem' }}
7990
aria-label="Minimum length"
8091
/>

core/services/routing/piiadapter/openai_completion.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ func applyAnyText(v any, elem int, text string) any {
4444
if elem < 0 {
4545
return text
4646
}
47-
if arr, ok := v.([]any); ok && elem >= 0 && elem < len(arr) {
47+
if arr, ok := v.([]any); ok && elem < len(arr) {
4848
arr[elem] = text
4949
}
5050
return v

core/services/routing/piidetector/pattern.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,9 @@ type patternDetector struct {
3939
// When tracing is enabled it records a pattern_pii BackendTrace so the matches
4040
// (group, byte range, text) show in the Traces UI alongside NER detections.
4141
func (d *patternDetector) Detect(_ context.Context, text string) ([]pii.NEREntity, error) {
42+
tracing := d.appConfig != nil && d.appConfig.EnableTracing
4243
var start time.Time
43-
if d.appConfig != nil && d.appConfig.EnableTracing {
44+
if tracing {
4445
trace.InitBackendTracingIfEnabled(d.appConfig.TracingMaxItems, d.appConfig.TracingMaxBodyBytes)
4546
start = time.Now()
4647
}
@@ -50,12 +51,12 @@ func (d *patternDetector) Detect(_ context.Context, text string) ([]pii.NEREntit
5051
var traceEnts []backend.TokenEntity
5152
for _, mt := range matches {
5253
out = append(out, pii.NEREntity{Group: mt.Group, Start: mt.Start, End: mt.End, Score: 1.0, Text: mt.Text})
53-
if d.appConfig != nil && d.appConfig.EnableTracing {
54+
if tracing {
5455
traceEnts = append(traceEnts, backend.TokenEntity{Group: mt.Group, Start: mt.Start, End: mt.End, Score: 1.0, Text: mt.Text})
5556
}
5657
}
5758

58-
if d.appConfig != nil && d.appConfig.EnableTracing {
59+
if tracing {
5960
trace.RecordBackendTrace(patternPIITrace(d.modelName, text, traceEnts, start))
6061
}
6162
return out, nil

core/services/routing/piipattern/grammar.go

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,16 @@ const (
2828
// credential shape, small enough that the compiled program stays tiny.
2929
MaxPatternLen = 256
3030
// MaxQuantifier caps an explicit {n,m} upper bound. RE2 expands a bounded
31-
// repeat into that many copies, so an uncapped {0,1000000} would blow up
32-
// the compiled program's memory. Unbounded {n,} (no upper) is a loop, not
33-
// an expansion, and is allowed.
34-
MaxQuantifier = 4096
31+
// repeat into that many copies, so a large bound inflates the compiled
32+
// program. Go's regexp/syntax independently rejects any bound above 1000
33+
// at Parse time, so this cap MUST stay strictly below 1000 to be a live
34+
// guard rather than dead code shadowed by the parser: a bound in
35+
// (MaxQuantifier, 1000] reaches walk and is rejected here with an
36+
// actionable error, while >1000 is caught earlier by Parse. 512 is far
37+
// larger than any real credential token yet keeps the guard meaningful and
38+
// is defence in depth should the stdlib cap ever rise. Unbounded {n,} (no
39+
// upper) is a loop, not an expansion, and is allowed.
40+
MaxQuantifier = 512
3541
// MaxAlternation caps the arms of a single `a|b|c` alternation.
3642
MaxAlternation = 64
3743
// MaxAST bounds recursion depth so a pathologically nested pattern can't

core/services/routing/piipattern/piipattern_test.go

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package piipattern
22

33
import (
4+
"fmt"
45
"strings"
56
"testing"
67

@@ -36,6 +37,45 @@ var _ = Describe("ValidatePattern", func() {
3637
)
3738
})
3839

40+
var _ = Describe("MaxQuantifier guard (must stay live, not dead code)", func() {
41+
// Go's regexp/syntax hard-caps repeat bounds at 1000 and rejects anything
42+
// larger at Parse time, before walk() runs. So the walk() {n,m} guard only
43+
// fires for bounds in (MaxQuantifier, 1000]; if MaxQuantifier ever creeps
44+
// to >= 1000 the guard becomes unreachable dead code. These specs pin the
45+
// relationship and prove the guard is the binding constraint in that band.
46+
const stdlibRepeatCap = 1000
47+
48+
It("is strictly below the stdlib repeat cap so the guard is reachable", func() {
49+
Expect(MaxQuantifier).To(BeNumerically("<", stdlibRepeatCap),
50+
"MaxQuantifier must be < %d or walk()'s {n,m} guard is dead code (Parse rejects larger bounds first)", stdlibRepeatCap)
51+
})
52+
53+
It("accepts a bound at exactly MaxQuantifier", func() {
54+
Expect(ValidatePattern(fmt.Sprintf(`sk-ant-[A-Za-z0-9]{%d}`, MaxQuantifier))).To(Succeed())
55+
})
56+
57+
It("rejects a bound just above MaxQuantifier with our actionable error (proves the guard runs)", func() {
58+
// MaxQuantifier+1 is still parseable (<= stdlib cap), so it reaches
59+
// walk(), where our guard — not the parser — rejects it.
60+
err := ValidatePattern(fmt.Sprintf(`sk-ant-[A-Za-z0-9]{%d}`, MaxQuantifier+1))
61+
Expect(err).To(HaveOccurred())
62+
Expect(err.Error()).To(ContainSubstring("bound is too large"),
63+
"a bound in (MaxQuantifier, stdlib cap] must be rejected by walk(), not the parser")
64+
})
65+
66+
It("rejects an unbounded {n,} whose lower bound exceeds MaxQuantifier", func() {
67+
err := ValidatePattern(fmt.Sprintf(`sk-ant-[A-Za-z0-9]{%d,}`, MaxQuantifier+1))
68+
Expect(err).To(HaveOccurred())
69+
Expect(err.Error()).To(ContainSubstring("bound is too large"))
70+
})
71+
72+
It("still fails closed above the stdlib cap (Parse rejects before walk)", func() {
73+
// >1000: caught by syntax.Parse; the message is the parser's, but it
74+
// still fails closed — defence in depth.
75+
Expect(ValidatePattern(fmt.Sprintf(`sk-ant-[A-Za-z0-9]{%d}`, stdlibRepeatCap+1))).NotTo(Succeed())
76+
})
77+
})
78+
3979
var _ = Describe("Compile", func() {
4080
It("compiles a valid pattern with leftmost-longest semantics", func() {
4181
re, err := Compile(`sk-ant-[A-Za-z0-9_-]{4,}`)

0 commit comments

Comments
 (0)