Original object is modified and not restored in any way

[meirkeller]

I want to be able the redact a key in multiple levels. So i wrote this code:

const fastRedact = require('fast-redact');
const paths = ['*.d', '*.*.d', '*.*.*.d'];

const redact = fastRedact({
  paths,
});

const obj = {
  x: { c: { d: 'hide me', e: 'leave me be' } },
  y: { c: { d: 'and me', f: 'I want to live' } },
  z: { c: { d: 'and also I', g: 'I want to run in a stream' } },
};

console.log(redact(obj));
console.log(obj);

The output is:

{"x":{"c":{"d":"[REDACTED]","e":"leave me be"}},"y":{"c":{"d":"[REDACTED]","f":"I want to live"}},"z":{"c":{"d":"[REDACTED]","g":"I want to run in a stream"}}}
{
  x: { c: { d: '[REDACTED]', e: 'leave me be' } },
  y: { c: { d: '[REDACTED]', f: 'I want to live' } },
  z: { c: { d: '[REDACTED]', g: 'I want to run in a stream' } }
}

You can see that the original object is modified.

Seems like something is wrong with the nestedRestore and the nestedRedact functions.

When i added multiple levels that do not exist in the object. Seems like that nestedRedact with the specialSet functions doesn't build the store properly.

[jsumners]

fast-redact/readme.md

Lines 180 to 188 in 41339e6

	It's important to note, that the original object is mutated – for performance reasons
	a copy is not made. See [rfdc](https://github.com/davidmarkclements/rfdc) (Really Fast
	Deep Clone) for the fastest known way to clone – it's not nearly close enough in speed
	to editing the original object, serializing and then restoring values.
	A `restore` function is also created and compiled to put the original state back on
	to the object after redaction. This means that in the default usage case, the operation
	is essentially atomic - the object is mutated, serialized and restored internally which
	avoids any state management issues.

[mcollina]

ouch, it seem we have a bug in the restore function.

[mcollina]

Thanks for reporting! Would you like to send a Pull Request to address this issue? Remember to add unit tests.

[meirkeller]

Will try to find a fix. Need to dig in the specialSet function.

[meirkl]

#51 this could be a fix. Needs some more investigation.

[meirkeller]

function specialSet (o, k, path, afterPath, censor, isCensorFct, censorFctTakesPath) {
  const afterPathLen = afterPath.length
  const lastPathIndex = afterPathLen - 1
  const originalKey = k
  var i = -1
  var n
  var nv
  var ov
  var oov = null
  var exists = true
  var wc = null
  ov = n = o[k]
  if (typeof n !== 'object') return { value: null, parent: null, exists }
  while (n != null && ++i < afterPathLen) {
    k = afterPath[i]
    oov = ov
    if (k !== '*' && !wc && !(typeof n === 'object' && k in n)) {
      exists = false
      break
    }
    if (k === '*') {
      wc = k
      if (i !== lastPathIndex) {
        continue
      }
    }
    if (wc) {
      const wcKeys = Object.keys(n)
      for (var j = 0; j < wcKeys.length; j++) {
        const wck = wcKeys[j]
        const wcov = n[wck]
        const kIsWc = k === '*'
        if (kIsWc || (typeof wcov === 'object' && wcov !== null && k in wcov)) {
          if (kIsWc) {
            ov = wcov
          } else {
            ov = wcov[k]
          }
          nv = (i !== lastPathIndex)
            ? ov
            : (isCensorFct
              ? (censorFctTakesPath ? censor(ov, [...path, originalKey, ...afterPath]) : censor(ov))
              : censor)
          if (kIsWc) {
            n[wck] = nv
          } else {
            if (wcov[k] === nv) {
              exists = false
            } else {
              wcov[k] = (nv === undefined && censor !== undefined) || (has(wcov, k) && nv === ov) ? wcov[k] : nv
            }
          }
        }
      }
      wc = null
    } else {
      ov = n[k]
      nv = (i !== lastPathIndex)
        ? ov
        : (isCensorFct
          ? (censorFctTakesPath ? censor(ov, [...path, originalKey, ...afterPath]) : censor(ov))
          : censor)
      n[k] = (has(n, k) && nv === ov) || (nv === undefined && censor !== undefined) ? n[k] : nv
      n = n[k]
    }
    if (typeof n !== 'object') break
    if (ov === oov) {
      exists = false
    }
  }
  return { value: ov, parent: oov, exists }
}

This can fix pino issue. but it's a Band Aid

if (ov === oov) {
  exists = false
}

[sgurnani99]

Hello @mrjwt , @mcollina
I was wondering if this issue is resolved as I face a similar problem where the original object is modified adding 'REDACTED' to the properties of the original object

Additionally I tried the restore function in an attempt to fix it and that appears to undo the redactions on object generated by redact function. Is that the intended behavior?

Thanks

[nagy135]

We should at least put warning "do not use multiple levels of wildcards!" to readme because this issue is actually breaking services, mutating data potentially causing damage

This mutating issue happens on:

└─┬ [email protected]
  └── [email protected]

[jsumners]

Pull requests to resolve issues are welcome.

[nagy135]

/*
 /test/index.js
*/
test('redact.restore function places original values back in place (deeply nested wildcards)', ({ end, is }) => {
  const censor = 'test'
  const value = 'hidden'
  const paths = ['*.c2.*.d']
  const redact = fastRedact({ paths, censor, serialize: false })
  const o = {
    x: { c1: [ { d: value, e: 'leave me be' } ] },
    y: { c2: [ { d: value, f: 'I want to live' } ] },
    z: { c3: [ { d: value, g: 'I want to run in a stream' } ] }
  }

  redact(o)
  is(o.x.c1[0].d, value)
  is(o.y.c2[0].d, censor)
  is(o.z.c3[0].d, value)
  redact.restore(o)
  is(o.x.c1[0].d, value)
  is(o.y.c2[0].d, value)
  is(o.z.c3[0].d, value)
  end()
})

It seems to be happening when combined with arrays ...testcase that started this issue works now, this one does not

[matt-clarson]

i've opened PR that should fix this issue, if someone can take a look #59