[EliMoshkovich]

Linear: PER-16493

Changes

• Init line. The constructor's debug line (Permit.io SDK initialized) now passes pino an object with:

  • pdp and apiUrl through redactUrl: scheme, host, port and path, without user, password, query or fragment (a value without a scheme, such as localhost:7766, is logged without everything up to its last @, its query or its fragment)
  • the settings that change the SDK's behaviour: timeout, throwOnError, multiTenancy, proxyFactsViaPdp, factsSyncTimeout(Policy), retry, pdpRetry

  As fields, they can be filtered in JSON log tooling.

• log.json. LoggerFactory no longer passes prettyPrint, which pino 8 rejects. log.json: true and PERMIT_LOG_JSON=true no longer stop the client from starting. The output stays JSON either way, as it already was by default, so no existing log pipeline sees a different format. A real pretty mode is left to 3.0.

• pino redact. It covers a token field and a headers.Authorization / headers.authorization field, at the top level of a logged object or one level down, as a backstop for an object a log call passes. Errors go through the err serializer instead.

This is a patch for the 2.x line. The 3.0.0 refactor in #134 reworks the same code.

Tests

src/tests/unit/init-log.spec.ts starts a client in a child process (without PERMIT_* variables, so the shell can't change the config) and reads what it prints, because the logger writes to the process's stdout file descriptor. It checks:

• the init record's exact fields and redacted URLs, including a PDP value without a scheme
• the init output at debug and trace
• that log.json and PERMIT_LOG_JSON=true start

Ran locally: yarn lint, yarn build, yarn test:unit (87 passed).

Second review pass (3223122)

• Logged axios errors: the logger's err serializer turns an axios error into its type, message, code, status, method and URL. The redact paths stay as a second layer.
• URLs in errors and debug lines: redactUrl drops a URL's user, password, query and fragment. PDP connection errors and the HTTP debug lines show URLs through it.
• PERMIT_LOG_JSON: read leniently (true or 1, any case), so no value stops the client from starting. log.json is documented as having no effect.
• Init-log test: its child process has a 20s timeout.
• Tests:
  • the fields an axios error serializes to
  • PERMIT_LOG_JSON values that JSON.parse used to reject
  • yarn test:unit: 89 passed; yarn lint: 0 errors

Third pass (8913b83)

• checkAllTenants: it now posts the query (user, action, resource, merged context) as the body, with the SDK's request headers, and reads allowed_tenants, the field the PDP returns. The resource keeps only the tenant it names: the answer lists every tenant, so the default tenant isn't filled in.
• PermitApiError: originalError is no longer enumerable, and request headers in the axios error it carries are redacted. originalError, response and request are still readable; the redacted header in originalError.config stays redacted if that config is replayed.
• Tests: the all-tenants request's body and headers; an API error inspected at depth. yarn test:unit: 91 passed.

Fourth pass (7aacac9, d8ad051, 9801dc1)

• checkAllTenants errors: an axios error is rethrown as it is (same type for callers), with the same header redaction as PermitApiError, and logged through the err serializer. (Permit.checkAllTenants logged it again until the fifth pass; now only the enforcer does.)
• PermitApiError: request is non-enumerable on the axios error and its response, as axios's toJSON() already leaves it out (an accessor since the fifth pass). originalError is writable and configurable again. When the API response carries no message (no response at all, or a body without that field), error.message is axios's message, such as Request failed with status code 422 or connect ECONNREFUSED ..., instead of an empty string.
• LoggerFactory.createLogger: takes an optional destination stream (stdout stays the default), so tests can read the SDK logger's output.
• redactUrl: removes the user info before cutting the query and fragment. The init line logs pdp and apiUrl through it and keeps the path.
• Tests: real errors from 127.0.0.1 servers (500, refused port, dropped connection) for the REST client and checkAllTenants; the connection-error messages of check, bulkCheck and getUserPermissions; the SDK logger's err serializer and redact paths; the HTTP debug lines; redact-url.spec.ts. yarn test:unit: 114 passed; yarn lint: 0 errors.

Fifth pass (e9d7882, 943d546, 08494f3)

• checkAllTenants errors: logged once, by the enforcer. Permit.checkAllTenants returns the enforcer's result and doesn't log the failure again.
• Axios errors: request on the error and its response is a non-enumerable get/set accessor instead of a data property. It stays readable and writable.
• err serializer: for an error that isn't an axios error, a string input field (Node's ERR_INVALID_URL carries the URL there) goes through redactUrl.
• redactUrl: trims the input and accepts one or more slashes after the scheme. For a URL that new URL() parses, the query and fragment are cut before the user info is taken; for a URL with a scheme that doesn't parse, everything up to the last @ is removed.
• Tests: the checkAllTenants failure tests capture the SDK logger, assert one record with the error's code (and status), and check that the next call on the same client still sends the key; a server that drops each connection replaces the released-port helper; one test per redact path; an ERR_INVALID_URL logged through the SDK logger; new redact-url.spec.ts inputs. yarn test:unit: 126 passed; yarn lint: 0 errors.

Sixth pass (1b9cc5a, 1e527da)

• redactUrl: an input that Node's URL parser accepts as http: or https: is rebuilt from the parsed URL (scheme, host, port, path), so the parser's own clean-up applies first. Anything else loses everything up to its last @, then its query and fragment; a scheme in front is kept. The scheme and host now come back lowercased, http:/host as http://host, and a /-only path is dropped (the init line's apiUrl for https://api.example.com/ is https://api.example.com).
• err serializer: the redacted input goes on a copy, so the logged value is never written to (a frozen one no longer makes the log call throw).
• Tests: new redact-url.spec.ts inputs; a frozen, non-error-like value logged through the SDK logger. yarn test:unit: 133 passed; yarn lint: 0 errors.

Seventh pass (f215370)

• HTTP debug lines: both lines log redactUrl(axiosInstance.getUri(config)), the URL the request goes to with baseURL joined, so a relative path such as /users/a@example.com/x keeps its @ and goes through the parse branch. If getUri throws, the line falls back to config.url.
• redactUrl: comments only; the header and the spec describe the two branches as they work.
• Tests: two fallback inputs with more than one @ (they pin the cut at the last one, and the query cut); an HTTP debug-line test with a baseURL that has user info, a path with @ and a query param. yarn test:unit: 136 passed; yarn lint: 0 errors.

Eighth pass (f1f375c)

• HTTP debug lines: the URL is built with getUri({ ...config, params: null }), so it carries no query and an app's paramsSerializer doesn't run for it (null, because undefined falls back to the instance's params). A request with neither url nor baseURL logs an empty URL.
• Tests: instance params with a counting paramsSerializer that is never called; a request with neither url nor baseURL; a getUri that throws, where both lines show the redacted config.url. yarn test:unit: 139 passed; yarn lint: 0 errors.

Ninth pass (7441f37)

• HTTP debug lines: whatever getUri (or the config.url fallback) returns is turned into a string ('' for null or undefined), so a url given as a URL object is sent, and logged without its user info or query, instead of making the request fail. Both lines are built only when the logger's level includes debug; at other levels getUri and redactUrl don't run.
• Tests: a URL object with user info as url, at warn and debug; getUri not called at info. yarn test:unit: 142 passed; yarn lint: 0 errors.

Worth a line in the release notes: originalError and the axios error's request no longer show up in Object.keys or a spread of the error (request is now an accessor), error.message is no longer empty when the API gives no message, and a checkAllTenants failure is logged once instead of twice.

Description updated during review to match d17b3c5 (wording made neutral for a public repo).
Description updated during review to match 9801dc1 (init-line URLs keep the path; redact scope; fourth-pass changes).
Description updated during review to match 08494f3 (fifth-pass changes; checkAllTenants logs once).
Description updated during review to match 1e527da (sixth-pass changes).
Description updated during review to match f215370 (seventh-pass changes).
Description updated during review to match f1f375c (eighth-pass changes).
Description updated during review to match 7441f37 (ninth-pass changes).

🤖 Generated with Claude Code

[linear-code]

PER-16493

[zeevmoney]

Changes requested: 1 HIGH, 3 MEDIUM, 6 LOW.

Blocking:

• HIGH src/enforcement/enforcer.ts:377: the checkAllTenants error path needs the same redaction as PermitApiError

Non-blocking:

• MEDIUM src/api/base.ts:18: the redaction covers less than its comment says, and the test uses a hand-made request
• MEDIUM src/api/base.ts:52: a network failure surfaces as a PermitApiError with an empty message and no visible cause
• MEDIUM src/tests/unit/key-exposure.spec.ts:79: five of the new protections can be removed with every unit test still passing
• LOW src/index.ts:95: the Permit class's TSDoc block now documents urlOrigin
• LOW src/index.ts:109: urlOrigin logs "null" for a PDP value without a scheme
• LOW src/utils/redact-url.ts:6: the query and fragment are cut before the user info is removed
• LOW src/logger.ts:39: the redact backstop covers one nesting level and two header spellings
• LOW src/api/base.ts:54: originalError is now read-only at runtime
• LOW src/tests/unit/init-log.spec.ts:71: the expected key list is filtered by the actual record

The earlier code-scanning thread on src/api/base.ts (polynomial regex) is fixed at this head.

Details are in the inline comments on each line.

[zeevmoney]

Approved — no CRITICAL or HIGH issues found.

All ten threads from the previous review are verified at 9801dc1, including the blocking checkAllTenants one; replies are on each thread.

Non-blocking:

• MEDIUM src/enforcement/enforcer.ts:391 — no test checks the checkAllTenants failure log lines
• LOW src/api/base.ts:47 — a non-enumerable data property still shows in some inspection modes
• LOW src/utils/redact-url.ts:11 — some inputs still come back with the wrong part removed or kept
• LOW src/logger.ts:27 — the err serializer fallback keeps a URL field unredacted
• LOW src/logger.ts:48 — two of the six redact paths aren't pinned by a test
• LOW src/tests/unit/key-exposure.spec.ts:43 — refusedUrl() connects to a port it has just released

Details are in the inline comments on each line.

[zeevmoney]

Approved — no CRITICAL or HIGH issues found.

Non-blocking:

• LOW src/utils/redact-url.ts:10 — a few malformed inputs still come back with their user info
• LOW src/logger.ts:31 — the input redaction can rewrite the caller's own object

Still open from the previous review: none. All six items are verified at 08494f3 (replies on each thread).

Details are in the inline comments on each line.

[zeevmoney]

Approved — no CRITICAL or HIGH issues found.

Both items from the previous review are verified fixed at 1e527da (replies on each thread).

Non-blocking:

• MEDIUM src/tests/unit/redact-url.spec.ts:34 — no test pins the cut at the last @ in the fallback
• LOW src/utils/redact-url.ts:23 — HTTP debug lines now cut a relative path at its last @
• LOW src/utils/redact-url.ts:3 — the header comment and two spec comments describe the old fallback

Details are in the inline comments on each line.

[zeevmoney]

Approved — no CRITICAL or HIGH issues found.

All three items from the previous review are verified fixed at f215370 (replies on each thread).

Non-blocking:

• LOW src/utils/http-logger.ts:16 — the logged URL carries params, and getUri can return undefined
• LOW src/utils/http-logger.ts:18 — no test covers the fallback when getUri throws

Details are in the inline comments on each line.

[zeevmoney]

Approved — no CRITICAL or HIGH issues found.

Non-blocking:

• LOW src/utils/http-logger.ts:19 — a url that isn't a string makes the interceptor throw

Details are in the inline comments on each line.

[zeevmoney]

Approved — no CRITICAL or HIGH issues found.

Non-blocking:

• LOW src/logger.ts:24 — the err serializer still assumes config.url is a string

Details are in the inline comments on each line.