Hi - this has been outstanding for a while now, should still be good.

Hi - this has been outstanding for a while now, should still be good.

thanks i will check this out asap

@jasonsaayman I dont think there is anything outstanding on this for me? Didnt hear back from your last comment

Thanks for the patch. The code change is the right pattern and worth landing, but a couple of things need fixing before it can go in, and the framing around what it actually fixes needs to change too.

I traced and reproduced both the claimed and actual behaviour against the current v1.x HEAD.

The description says this prevents pollution via constructor.prototype chains. That isn't quite accurate — the existing code already handles that case, because target.constructor returns the function Object, which fails utils.isObject (functions don't pass the typeof === 'object' check), so the slot already gets reset to []. Demonstrated against unpatched HEAD:

input:  constructor.prototype.polluted = 'true'
result: {"constructor":{"prototype":{"polluted":"true"}},"safe":"data"}
{}.polluted === undefined
Object.prototype.polluted === undefined

The new test passes against unpatched code too, so it isn't covering the path your change affects.

The real benefit of the change is defence-in-depth against an already-polluted runtime. If something else has set Object.prototype.injected = { hijack: true }, the unpatched code traverses the prototype chain, calls isObject, and writes the user's value into Object.prototype:

without PR:
  result: {}
  Object.prototype.injected.hijack === [true, "STOLEN"]   // mutated through

with PR:
  result: {"injected":{"hijack":"STOLEN"}}
  Object.prototype.injected.hijack === true               // intact

That's the scenario these guards face, and that's what the test should cover.

A few more things to fix:

1. The test path is wrong. The PR adds tests at test/specs/helpers/formDataToJSON.spec.js. The repo runs Vitest from tests/unit/helpers/formDataToJSON.test.js; package.json only defines test:vitest. The new test won't run in CI.

2. The test duplicates existing coverage. tests/unit/helpers/formDataToJSON.test.js:78 already has a should resist prototype pollution CVE block that exercises constructor.prototype.y and asserts {}.y === undefined. The added test is essentially the same, with renamed keys.

3. Suggested replacement test that actually exercises the guard:

it('should not write through to inherited objects on Object.prototype', () => {
  Object.prototype.injected = { hijack: true };
  try {
    const formData = new FormData();
    formData.append('injected.hijack', 'STOLEN');
    const result = formDataToJSON(formData);

    expect(result.injected).toEqual({ hijack: 'STOLEN' });
    expect(Object.prototype.injected.hijack).toEqual(true);
  } finally {
    delete Object.prototype.injected;
  }
});

This would fail against the current HEAD and pass with your one-line change applied.

Could you fold the fix into tests/unit/helpers/formDataToJSON.test.js, drop the duplicate test, and add the inherited-object write-through case? Once that's in, the code change itself is a clean defence-in-depth improvement.

Hi - thanks for the detailed feedback. Please see below -

Updated:

Rebased onto current v1.x (was 164 commits behind). Branch is now up to date with the test/ → tests/ migration and Vitest.
Dropped the duplicate test that was essentially re-testing constructor.prototype (already covered by the existing CVE test).
Added the write-through regression test you suggested — pollutes Object.prototype.injected, sends injected.hijack through formDataToJSON, and asserts the inherited object isn't mutated. Confirmed it fails on unpatched HEAD and passes with the fix.
Corrected framing: this is defense-in-depth against an already-polluted Object.prototype, not a standalone constructor.prototype fix.
Test file is now in tests/unit/helpers/formDataToJSON.test.js (no leftover under old test/specs/).