From fd698a8150470d2e82f7b127336dabb1bcbda60a Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 23 Jul 2026 16:51:17 +0000 Subject: [PATCH] safePushData: restrict placeholders to empty values, prefer null for unions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Only fabricate the four unambiguously-empty placeholder values — '' (string), [] (array), {} (object), and null — when healing a required field. Stop placeholdering enum, format, minLength/maxLength, numeric bounds, and integer/number/boolean types: a made-up email, first-enum-value, or fabricated number silently poisons the customer's dataset with plausible-looking junk, so those items are now dropped instead. When a field allows multiple types (e.g. [string, null]), always pick null — the cleanest placeholder, committing to no concrete value. Also add a NOTE that we could parse dataset_schema.json and one-shot the fix instead of recursively healing, at the cost of heavier code. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01BBjhQUKX8hoFERqqpoAiQZ --- README.md | 39 +++++++------ src/safePushData.ts | 114 +++++++++++++++----------------------- test/safePushData.test.ts | 67 ++++++++++++++++++---- 3 files changed, 123 insertions(+), 97 deletions(-) diff --git a/README.md b/README.md index be53d7e..92c3a79 100644 --- a/README.md +++ b/README.md @@ -47,25 +47,30 @@ every AJV error per item: ### Placeholder defaults When a constraint fires on a path we placeholder'd ourselves, the wrapper -picks a value that should satisfy it: - -| AJV keyword | Placeholder value | -| ------------------------------------------------------- | ------------------------------- | -| `type: string` | `''` | -| `type: integer` / `number` | `0` | -| `type: boolean` | `false` | -| `type: array` | `[]` | -| `type: object` | `{}` | -| `type: null` | `null` | -| `minLength: N` / `maxLength` | `'_'.repeat(N)` / `''` | -| `minimum: N` / `maximum` | `N` | -| `exclusiveMinimum: N` / `exclusiveMaximum` | `N + 1` / `N - 1` | -| `enum` | First allowed value | -| `format: email` / `uri` / `date` / `date-time` / `uuid` | a static valid example for each | -| Anything else (`pattern`, custom formats…) | Item is dropped. | +picks a value that should satisfy it. We deliberately only fill in the four +**empty** values below — they're unambiguously empty and can't be mistaken +for real data: + +| AJV keyword | Placeholder value | +| -------------- | ----------------- | +| `type: string` | `''` | +| `type: array` | `[]` | +| `type: object` | `{}` | +| `type: null` | `null` | +| Anything else | Item is dropped. | + +When a field allows **multiple types** (e.g. `['string', 'null']`), the +wrapper always picks `null` — it's the cleanest placeholder because it +commits to no concrete value at all. + +Everything else (`enum`, `format`, `minLength`, numeric bounds, `type: +integer` / `number` / `boolean`, …) is **not** placeholdered: a made-up +email, a first-enum-value, or a fabricated number would silently poison the +customer's dataset with plausible-looking junk, so the item is dropped +instead. The retry loop chases one layer of errors per round -(`required` → `type` → `minLength` → push) until either the push succeeds +(`required` → `type` → push) until either the push succeeds or `maxAttempts` (default 5) is hit. ## Options diff --git a/src/safePushData.ts b/src/safePushData.ts index 54210d0..188494b 100644 --- a/src/safePushData.ts +++ b/src/safePushData.ts @@ -1,6 +1,13 @@ // safePushData: parse the Apify dataset schema-validation error, repair the // offending items (strip bad fields, placeholder missing required ones), and // retry the push. +// +// NOTE: instead of recursively healing the data one error-round at a time, we +// could parse the Actor's `dataset_schema.json` up front and fix every item in +// a single pass (we'd know each field's expected type / constraints without +// waiting for the API to report them). That would avoid the multi-round retry +// loop, but requires heavier code (locating + loading the schema, resolving +// $refs, walking the schema tree). Something to consider in the future. const SCHEMA_ERROR_TYPE = 'schema-validation-error'; @@ -234,77 +241,48 @@ function cleanItemFields(item: T, validationErrors: ValidationError[], placeh } // Pick a value that will satisfy `err.keyword` on a placeholder field. -// Returns ok:false when we don't have a sensible default (e.g. `pattern`, -// custom formats); the caller drops the item in that case. +// Returns ok:false when we don't have a sensible default; the caller drops +// the item in that case. +// +// We deliberately only placeholder the four "empty" values — `''`, `[]`, `{}`, +// and `null`. These are unambiguously empty and can't be mistaken for real +// data. We do NOT fabricate values for `enum`, `format`, `minLength`, numeric +// bounds, etc.: a made-up email, a first-enum-value, or a `'_'.repeat(N)` +// string all silently poison the customer's dataset with plausible-looking +// junk. Better to drop the item than to lie about its contents. As a result +// the only keyword we handle is `type` (only for those four target types) — +// everything else falls through to ok:false and the item is dropped. function placeholderFor(err: ValidationError): { ok: true; value: unknown } | { ok: false } { const params = err.params ?? {}; - switch (err.keyword) { - case 'type': { - // params.type is the expected type as a string, or array of strings. - const t = Array.isArray(params.type) ? params.type[0] : params.type; - switch (t) { - case 'string': - return { ok: true, value: '' }; - case 'integer': - case 'number': - return { ok: true, value: 0 }; - case 'boolean': - return { ok: true, value: false }; - case 'array': - return { ok: true, value: [] }; - case 'object': - return { ok: true, value: {} }; - case 'null': - return { ok: true, value: null }; - default: - break; - } - return { ok: false }; - } - case 'minLength': { - const limit = Number(params.limit) || 1; - return { ok: true, value: '_'.repeat(limit) }; - } - case 'maxLength': - return { ok: true, value: '' }; - case 'minimum': - case 'exclusiveMinimum': { - const limit = Number(params.limit); - if (!Number.isFinite(limit)) return { ok: false }; - return { ok: true, value: err.keyword === 'exclusiveMinimum' ? limit + 1 : limit }; - } - case 'maximum': - case 'exclusiveMaximum': { - const limit = Number(params.limit); - if (!Number.isFinite(limit)) return { ok: false }; - return { ok: true, value: err.keyword === 'exclusiveMaximum' ? limit - 1 : limit }; - } - case 'enum': { - const allowed = params.allowedValues; - if (Array.isArray(allowed) && allowed.length > 0) return { ok: true, value: allowed[0] }; - return { ok: false }; - } - case 'format': { - switch (params.format) { - case 'email': - return { ok: true, value: 'placeholder@example.com' }; - case 'uri': - case 'uri-reference': - case 'url': - return { ok: true, value: 'about:blank' }; - case 'date': - return { ok: true, value: '1970-01-01' }; - case 'date-time': - return { ok: true, value: '1970-01-01T00:00:00Z' }; - case 'uuid': - return { ok: true, value: '00000000-0000-0000-0000-000000000000' }; - default: - break; - } - return { ok: false }; + if (err.keyword !== 'type') return { ok: false }; + + // params.type is the expected type as a string, or an array of strings + // when the field allows multiple types (e.g. `['string', 'null']`). + const types = Array.isArray(params.type) ? params.type : [params.type]; + + // Union type that permits null: prefer null. It's the cleanest possible + // placeholder — it commits to no concrete value at all — so whenever the + // schema allows it, that's what we use. + if (types.length > 1 && types.includes('null')) { + return { ok: true, value: null }; + } + + // Otherwise take the first allowed type we have an "empty" default for. + // integer / number / boolean are intentionally absent: 0 / false read as + // real data, so a field of only those types is dropped instead. + for (const t of types) { + switch (t) { + case 'null': + return { ok: true, value: null }; + case 'string': + return { ok: true, value: '' }; + case 'array': + return { ok: true, value: [] }; + case 'object': + return { ok: true, value: {} }; + default: + break; } - default: - break; } return { ok: false }; } diff --git a/test/safePushData.test.ts b/test/safePushData.test.ts index aa8c689..29629ac 100644 --- a/test/safePushData.test.ts +++ b/test/safePushData.test.ts @@ -125,8 +125,11 @@ test('placeholders a missing required field, then satisfies the type', async () assert.deepEqual(calls[calls.length - 1][0], { age: 30, name: '' }); }); -test('chases required -> type -> minLength on the same placeholder field', async () => { +test('drops item when a placeholder field carries a minLength it cannot satisfy', async () => { // Schema: { required: ['name'], properties: { name: { type: 'string', minLength: 3 } } } + // We placeholder name to '' (type: string), but we no longer fabricate a + // `'_'.repeat(N)` string for minLength — a made-up string is customer-data + // poison — so the item is dropped instead. const validate = (item: Item): ValidationError[] | null => { const errors: ValidationError[] = []; if (item?.name === undefined) { @@ -158,14 +161,17 @@ test('chases required -> type -> minLength on the same placeholder field', async } return null; }; - const { pushFn, calls } = makeMockPush(validate); + const { pushFn } = makeMockPush(validate); const res = await safePushData(pushFn, { age: 30 }, { maxAttempts: 10 }); - assert.equal(res.pushed, 1); - assert.equal(res.attempts, 4); // required, type, minLength, success - assert.equal((calls[calls.length - 1][0].name as string).length, 3); + assert.equal(res.pushed, 0); + assert.equal(res.dropped.length, 1); + assert.deepEqual(res.dropped[0].item, { age: 30 }); }); -test('placeholder for enum picks the first allowed value', async () => { +test('drops item on enum constraint instead of fabricating the first allowed value', async () => { + // We used to placeholder an enum field with its first allowed value; that + // silently injects a plausible-but-wrong value into the dataset, so we now + // drop the item instead. const validate = (item: Item): ValidationError[] | null => { if (item?.role === undefined) { return [ @@ -200,13 +206,15 @@ test('placeholder for enum picks the first allowed value', async () => { } return null; }; - const { pushFn, calls } = makeMockPush(validate); - const res = await safePushData(pushFn, { name: 'x' }); - assert.equal(res.pushed, 1); - assert.equal(calls[calls.length - 1][0].role, 'admin'); + const { pushFn } = makeMockPush(validate); + const res = await safePushData(pushFn, { name: 'x' }, { maxAttempts: 10 }); + assert.equal(res.pushed, 0); + assert.equal(res.dropped.length, 1); }); -test('placeholder for format=email', async () => { +test('drops item on format=email instead of fabricating a fake address', async () => { + // A made-up `placeholder@example.com` is exactly the kind of junk we no + // longer inject; the item is dropped instead. const validate = (item: Item): ValidationError[] | null => { if (item?.email === undefined) { return [ @@ -240,10 +248,45 @@ test('placeholder for format=email', async () => { } return null; }; + const { pushFn } = makeMockPush(validate); + const res = await safePushData(pushFn, { name: 'x' }, { maxAttempts: 10 }); + assert.equal(res.pushed, 0); + assert.equal(res.dropped.length, 1); +}); + +test('required field with a union type that allows null is placeholder-filled with null', async () => { + // Schema: { required: ['note'], properties: { note: { type: ['string', 'null'] } } } + // The initial `required` placeholder sets note = null; because the field + // allows null, the follow-up type error reports both allowed types and we + // keep null (the cleanest placeholder) rather than coercing to ''. + const validate = (item: Item): ValidationError[] | null => { + if (!('note' in (item ?? {}))) { + return [ + { + instancePath: '', + keyword: 'required', + params: { missingProperty: 'note' }, + message: "must have required property 'note'", + }, + ]; + } + if (item.note !== null && typeof item.note !== 'string') { + return [ + { + instancePath: '/note', + keyword: 'type', + params: { type: ['string', 'null'] }, + message: 'must be string,null', + }, + ]; + } + return null; + }; const { pushFn, calls } = makeMockPush(validate); const res = await safePushData(pushFn, { name: 'x' }); assert.equal(res.pushed, 1); - assert.equal(calls[calls.length - 1][0].email, 'placeholder@example.com'); + assert.equal(res.dropped.length, 0); + assert.deepEqual(calls[calls.length - 1][0], { name: 'x', note: null }); }); test('drops item when a placeholder constraint has no known fix (pattern)', async () => {