Repository navigation
PER-16680: log through an SDK logger that applies the log settings - #163
Merged
Merged
Conversation
Ported to main from #137 (Zeev Manilovich), which is stacked on the release-CI draft #136; this carries only #137's own changes. - The client no longer logs its config on creation; it logs the PDP and API URL. - permit.utils.sdk_logger applies log.level and log.label to the SDK's records without touching the application's loguru sinks, and replaces every client's API key with [REDACTED] in what the SDK logs and in PDP error bodies. - PermitConfig.token is repr=False. - ruff bans logging straight to loguru inside the package. - README: what each log option does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Dependency Security AuditScanned: pyproject.toml dependencies + dev group, resolved at Python 3.10 (the current resolution, and the lowest versions the published specs permit under each pydantic major) ❌ 6 fixable HIGH/CRITICAL advisories -- blocking this build.
Advisory detailsCRITICAL -- GHSA-hq85-5cpj-mr62 ( Found by: trivy:dev-ceiling HIGH -- CVE-2026-111064 ( Found by: trivy:runtime-floor, trivy:runtime-floor-pydantic-v2 HIGH -- CVE-2026-111066 ( Found by: trivy:runtime-floor, trivy:runtime-floor-pydantic-v2 HIGH -- GHSA-2g87-pp6c-29x5 ( Found by: pip-audit:runtime-floor, pip-audit:runtime-floor-pydantic-v2, trivy:runtime-floor, trivy:runtime-floor-pydantic-v2 HIGH -- CVE-2026-114605 ( Found by: trivy:dev-ceiling, trivy:runtime-ceiling, trivy:runtime-floor, trivy:runtime-floor-pydantic-v2 HIGH -- CVE-2026-112940 ( Found by: trivy:dev-ceiling MEDIUM -- CVE-2026-111063 ( Found by: trivy:runtime-floor, trivy:runtime-floor-pydantic-v2 MEDIUM -- CVE-2026-111065 ( Found by: trivy:runtime-floor, trivy:runtime-floor-pydantic-v2 MEDIUM -- CVE-2026-111748 ( Found by: trivy:runtime-floor, trivy:runtime-floor-pydantic-v2 MEDIUM -- CVE-2026-111750 ( Found by: trivy:runtime-floor, trivy:runtime-floor-pydantic-v2 MEDIUM -- CVE-2026-112906 ( Found by: trivy:dev-ceiling MEDIUM -- CVE-2026-112908 ( Found by: trivy:dev-ceiling MEDIUM -- CVE-2026-112909 ( Found by: trivy:dev-ceiling MEDIUM -- CVE-2026-112910 ( Found by: trivy:dev-ceiling MEDIUM -- CVE-2026-112911 ( Found by: trivy:dev-ceiling MEDIUM -- CVE-2026-112914 ( Found by: trivy:dev-ceiling MEDIUM -- CVE-2026-112915 ( Found by: trivy:dev-ceiling MEDIUM -- CVE-2026-112934 ( Found by: trivy:dev-ceiling MEDIUM -- CVE-2026-112935 ( Found by: trivy:dev-ceiling MEDIUM -- CVE-2026-112937 ( Found by: trivy:dev-ceiling MEDIUM -- CVE-2026-112938 ( Found by: trivy:dev-ceiling MEDIUM -- CVE-2026-112939 ( Found by: trivy:dev-ceiling MEDIUM -- CVE-2026-104843 ( Found by: trivy:dev-ceiling MEDIUM -- CVE-2026-102598 ( Found by: trivy:dev-ceiling LOW -- CVE-2026-111749 ( Found by: trivy:runtime-floor, trivy:runtime-floor-pydantic-v2 LOW -- CVE-2026-112907 ( Found by: trivy:dev-ceiling LOW -- CVE-2026-112912 ( Found by: trivy:dev-ceiling LOW -- CVE-2026-112913 ( Found by: trivy:dev-ceiling LOW -- CVE-2026-112941 ( Found by: trivy:dev-ceiling UNKNOWN -- CVE-2026-111063 (GHSA-494j-54xm-rwp3) ( Found by: pip-audit:runtime-floor, pip-audit:runtime-floor-pydantic-v2 UNKNOWN -- CVE-2026-111064 (GHSA-5wr8-gcqf-gqf3) ( Found by: pip-audit:runtime-floor, pip-audit:runtime-floor-pydantic-v2 UNKNOWN -- CVE-2026-111065 (GHSA-6cjc-53gc-5rqm) ( Found by: pip-audit:runtime-floor, pip-audit:runtime-floor-pydantic-v2 UNKNOWN -- CVE-2026-111066 (GHSA-9xw5-w5jj-pvcg) ( Found by: pip-audit:runtime-floor, pip-audit:runtime-floor-pydantic-v2 UNKNOWN -- CVE-2026-111748 (GHSA-f9xw-jh8m-3mwq) ( Found by: pip-audit:runtime-floor, pip-audit:runtime-floor-pydantic-v2 UNKNOWN -- CVE-2026-111749 (GHSA-gx76-c99x-2c96) ( Found by: pip-audit:runtime-floor, pip-audit:runtime-floor-pydantic-v2 UNKNOWN -- CVE-2026-111750 (GHSA-pqxq-w44r-73xw) ( Found by: pip-audit:runtime-floor, pip-audit:runtime-floor-pydantic-v2 UNKNOWN -- CVE-2026-114605 (GHSA-45c3-73f7-pv4m) ( Found by: pip-audit:dev-ceiling, pip-audit:runtime-ceiling, pip-audit:runtime-floor, pip-audit:runtime-floor-pydantic-v2 UNKNOWN -- CVE-2026-112906 (GHSA-2g95-wg33-c5x2) ( Found by: pip-audit:dev-ceiling UNKNOWN -- CVE-2026-112907 (GHSA-567q-5qvw-pf92) ( Found by: pip-audit:dev-ceiling UNKNOWN -- CVE-2026-112908 (GHSA-6627-m3xw-93mp) ( Found by: pip-audit:dev-ceiling UNKNOWN -- CVE-2026-112909 (GHSA-69hj-f68j-2879) ( Found by: pip-audit:dev-ceiling UNKNOWN -- CVE-2026-112910 (GHSA-8mx5-7m5m-r77g) ( Found by: pip-audit:dev-ceiling UNKNOWN -- CVE-2026-112911 (GHSA-9c4x-gm8v-wgc4) ( Found by: pip-audit:dev-ceiling UNKNOWN -- CVE-2026-112912 (GHSA-9j6q-wvj5-jgjg) ( Found by: pip-audit:dev-ceiling UNKNOWN -- CVE-2026-112913 (GHSA-9vhv-jxww-xfm5) ( Found by: pip-audit:dev-ceiling UNKNOWN -- CVE-2026-112914 (GHSA-crv4-6mhj-j5jh) ( Found by: pip-audit:dev-ceiling UNKNOWN -- CVE-2026-112915 (GHSA-cwqr-hh33-gh44) ( Found by: pip-audit:dev-ceiling UNKNOWN -- CVE-2026-112934 (GHSA-fj95-fr6c-h845) ( Found by: pip-audit:dev-ceiling UNKNOWN -- CVE-2026-112935 (GHSA-fq56-3cx3-fjxp) ( Found by: pip-audit:dev-ceiling UNKNOWN -- CVE-2026-112937 (GHSA-jqc4-5cvw-7w85) ( Found by: pip-audit:dev-ceiling UNKNOWN -- CVE-2026-112938 (GHSA-m462-7rxw-wvpg) ( Found by: pip-audit:dev-ceiling UNKNOWN -- CVE-2026-112939 (GHSA-rfpc-qcw5-3pc4) ( Found by: pip-audit:dev-ceiling UNKNOWN -- CVE-2026-112940 (GHSA-v4x8-3wr5-3wq8) ( Found by: pip-audit:dev-ceiling UNKNOWN -- CVE-2026-112941 (GHSA-wfhq-q4j2-c37m) ( Found by: pip-audit:dev-ceiling UNKNOWN -- PYSEC-2026-4305 (CVE-2026-104843, GHSA-2cv4-cqwr-gwf7) ( Found by: pip-audit:dev-ceiling UNKNOWN -- PYSEC-2026-4321 (GHSA-hq85-5cpj-mr62) ( Found by: pip-audit:dev-ceiling UNKNOWN -- PYSEC-2026-4312 (CVE-2026-102598, GHSA-g6x2-hccm-hh4m) ( Found by: pip-audit:dev-ceiling How to fixRaise the affected lower bound in This audit resolves without the 7-day publish-age cooldown ( An advisory with no fix available does not block the build -- it is reported here so it can be tracked, but no version bump can resolve it. Suppression files are deliberately not honoured: the scan runs with |
… info, wait_for_sync - handle_api_error scrubs the registered keys from the body it puts in an exception, text or JSON, so the REST API and the PDP's local API raise without them, as the enforcer already did. - The start-up line logs api_url and pdp without a user name or password. - wait_for_sync builds a client of the same class again, so a subclass's own setup runs, and keeps the process's log settings through the keep_log_settings context variable. - A secret shorter than 12 characters is not redacted, so a short test token doesn't mangle the SDK's text. - log.level takes a loguru severity number and surrounding whitespace; an unknown level is warned about once per process. - The SDK's enable/disable switch changes under a lock. - Drop sync.Permit.__init__, which only called super(). - README and docstrings match. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
EliMoshkovich
requested review from
zeevmoney
and
a balanced review from Copilot
October 27, 2026 23:27
…L helper - aiohttp keeps the request's headers, Authorization included, on the ClientResponse and ClientResponseError the SDK's exceptions carry (original_error, response, the cause), and their repr prints them. scrub_request_info replaces every registered key there before an exception holds them, including the response's cached request_info. - without_userinfo moves to permit.utils.sdk_logger. It never raises (a bad port no longer fails client creation, an unparseable URL shows as <invalid URL>) and keeps IPv6 brackets. - PDP error messages show the PDP URL without a user name or password. - README: wait_for_sync builds a new client of the same class; what the key is replaced in. - Tests: REST and PDP errors for a non-JSON answer (200 and 502) and for an echoed key carry no key in their cause, context, original_error or response; without_userinfo cases. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- handle_api_error's JSON scrubber replaces registered keys in object keys as well as values, so an error body that uses the key as a key doesn't expose it. - The SDK logger scrubs the message after the label is added, so a key placed in log.label is replaced too. - Tests: a key echoed as a JSON object key; a key in the label. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…d redirect history A ClientResponseError keeps its RequestInfo as args[0] as well as in request_info, and the redirect responses in its (or a response's) history keep their own. scrub_request_info replaces the key in all of them. Test: neither the carried objects nor their args print the key. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
zeevmoney
approved these changes
Oct 29, 2026
zeevmoney
left a comment
zeevmoney
left a comment
Member
There was a problem hiding this comment.
Approved — no CRITICAL or HIGH issues found.
Non-blocking:
- MEDIUM
permit/exceptions.py:309— an error body labelled JSON that isn't valid JSON skips the scrub and escapes as a rawJSONDecodeError - MEDIUM
permit/permit.py:37— new_connecthook changes how subclasses are built; the logging fix doesn't need it - MEDIUM
permit/exceptions.py:269— redirect-history and list-body scrub branches have no test - MEDIUM
tests/test_fix_logging.py:247— "no key at any level, for both clients" is only partly tested - LOW
permit/config.py:81— therepr=Falsecomment claims more than it delivers in tracebacks - LOW
permit/exceptions.py:352— the log line is scrubbed, the exception message built from the same text isn't - LOW
README.md:58— understates what"enable": Trueundoes - LOW
README.md:71— the redaction paragraph promises more than the code does - LOW
permit/utils/sdk_logger.py:111— dropped records are still labelled and scrubbed; cost grows with every key seen - LOW
pyproject.toml:248— the ban covers loguru but not stdliblogging - LOW
permit/logger.py:84— a level such as"²"makes client creation raise (isdigitvsisdecimal) - LOW
tests/test_fix_logging.py:334— thewarncase passes without the alias; four branches untested - LOW
tests/test_fix_logging.py:61— the fixture keeps the set of warned-about levels between tests - LOW
permit/exceptions.py:8—multidictimported directly but not declared
Details are in the inline comments on each line.
…http error text - An error response whose Content-Type says JSON but whose body isn't JSON (a proxy's error page, a truncated body, one that isn't UTF-8) now raises PermitApiError with the scrubbed body as its details, instead of letting a JSONDecodeError or UnicodeDecodeError, which keep the raw body, escape. The body is scrubbed before it is parsed, and the decode error isn't chained. - PermitConnectionError replaces the key in its message itself, so every raise site is covered, including the ones that build it from an aiohttp error's text. - The aiohttp error it carries has the key replaced in its message, the response headers it keeps and its string arguments, not only in the request headers. - Tests: a body labelled JSON that isn't JSON; an aiohttp error whose text echoes the key, for every enforcer call and the REST API; an echoed key after a followed redirect, and in a list of errors and an object key spelled with JSON escapes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nnect hook Permit.__init__ called self._connect(), a new override point: an application subclass with its own _connect method had it called by the SDK's constructor. The logging change doesn't need it, so the base constructor builds the four clients again and the blocking client's constructor builds its own after it, as before. A test checks that a subclass's own _connect is left alone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- SdkLogger logs with loguru's lazy option, so the label, the key replacement and the message itself are built only for a record loguru passes on. With the SDK's logging off (the default) a debug record is dropped without being built or scrubbed. An application that turns the permit records on itself still gets them in full. - The enforcer's debug lines, which pformat the request and the response, pass a function that builds the message, so the pformat runs only for a record a sink receives. - A log level such as "²", which str.isdigit accepts but int() doesn't, is now an unknown level that falls back to INFO with a warning, instead of failing client creation. - Tests: the echoed-key test runs on both clients, at trace, debug, info, error and critical, with logging off and with the application turning it on; the level sweep runs on both clients, checks the key and that no level falls back to INFO, and covers "fatal"; tests for the 12-character boundary, the label on the unknown-level path, an enabled client undoing the SDK's own disable only once, and a disable the application made for an SDK module before. The fixture resets the set of level names already warned about. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…s; lint and deps
- README: list where the key is replaced (log messages, exception messages and
error bodies, the aiohttp objects the exceptions carry) and where it isn't (the
response's own headers and raw body, frame values in tracebacks), and keep the
user-name-and-password sentence to the start-up line.
- README and docstrings: undoing the SDK's own disable with logger.enable("permit")
also drops every disable set for a permit module, before or since, and overrides
an application-wide logger.disable("") for the SDK's records. A test covers both.
- The repr=False comment on PermitConfig.token now says what it covers: repr() and
str() of the config, not every value a traceback prints.
- ruff bans the standard logging module in the package, as it bans loguru's logger.
- multidict, imported directly, is a declared dependency, in aiohttp's range.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
zeevmoney
approved these changes
Oct 29, 2026
zeevmoney
left a comment
zeevmoney
left a comment
Member
There was a problem hiding this comment.
Approved — no CRITICAL or HIGH issues found.
Non-blocking:
- MEDIUM
permit/enforcement/enforcer.py:64— a PDP error body that isn't UTF-8 escapes as a rawUnicodeDecodeError - MEDIUM
permit/exceptions.py:270— theargsscrub has no test - LOW
permit/exceptions.py:265— the aiohttp error is scrubbed, but not everything its traceback prints - LOW
permit/utils/sdk_logger.py:103— the key is matched case-sensitively, so a lower-cased copy isn't replaced - LOW
README.md:80— two items in the "replaced" list are broader than the code - LOW
permit/exceptions.py:60—PermitConnectionErrornow fails on a message that isn't a string - LOW
pyproject.toml:34— the floor lanes now test multidict 4.5.0, built as pure Python - LOW
tests/test_fix_logging.py:1162— the lazy-record test pins the exact call sequence and coverscheck()only - LOW
tests/test_fix_logging.py:1072— inline copy ofCLIENT_CLASSES
All 14 threads from the previous review were checked at bce2118 and replied to. One part of the PermitConnectionError thread is still open: the chained exceptions, covered by the exceptions.py:265 comment above.
Details are in the inline comments on each line.
…er-cased key - The aiohttp error an SDK exception carries is scrubbed together with every exception chained under it, such as the HTTP parser errors aiohttp raises it from: their string args, message, line, and the URL an InvalidURL error prints. Tested against the whole formatted traceback, for a bad header, a bad status line, a redirect to an invalid URL and one to a non-HTTP URL, on the PDP and the REST API, with both clients. The non-HTTP redirect keeps the key in args only. - Each key is also replaced in lower case: aiohttp lower-cases some server text, such as a Content-Type, in its error messages. The test key is now mixed case. - The enforcer reads a PDP error body with errors="replace", so a body that isn't valid UTF-8 raises PermitConnectionError instead of a UnicodeDecodeError that keeps the raw bytes. - PermitConnectionError keeps a message that isn't a string as it is, as before. - README: say what the redaction covers, now including the chained exceptions, and that URLs and a redirect response's own headers are not covered. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The test now runs bulk_check, authorized_users and get_user_permissions as well as check, and with the records on asserts that the message was built and scrubbed, without pinning how many pformat calls it makes. Two parametrizations use CLIENT_CLASSES instead of copying it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The multidict floor stays at aiohttp's own, 4.5, so declaring it narrows nothing users can install. The comment now says that the lowest-direct floor lanes therefore run 4.5.0, which has no C extension build for Python 3.10+. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
zeevmoney
requested changes
Oct 29, 2026
zeevmoney
left a comment
zeevmoney
left a comment
Member
There was a problem hiding this comment.
Changes requested — 1 HIGH, 2 MEDIUM, 4 LOW.
Blocking:
- HIGH
permit/exceptions.py:282— the chain walk rewrites the application's own exceptions, and can raise instead ofPermitConnectionError
Non-blocking:
- MEDIUM
permit/exceptions.py:298— messages and arguments that aren't strings are left as they are, though the README says they are covered - MEDIUM
tests/test_fix_logging.py:1253— four of the new scrub branches can be removed with every test still passing - LOW
permit/exceptions.py:274—_chain's docstring doesn't match what it walks, and its cycle guard has no test - LOW
permit/exceptions.py:270—_descriptionnever holds server text - LOW
tests/test_fix_logging.py:1294— the raw-server test doesn't check that its case still reaches a scrub - LOW
README.md:82— "an error's ... URL" reads as contradicting "not in URLs"
All 9 threads from the previous review were checked at 1eea9d8 and replied to; each is fixed.
Details are in the inline comments on each line.
…crub The walk over the exceptions chained under a carried aiohttp error reached, through __context__, an exception the application was handling when it called the SDK, and wrote to it. A frozen dataclass exception, read-only args or a headers dict with a value that isn't a string then made the scrub raise inside PermitConnectionError.__init__, so the caller got that error instead of a PermitConnectionError. - The walk keeps only exceptions raised and caught within the request: every frame of their traceback is in aiohttp, the libraries it raises errors from, asyncio or the SDK. It doesn't go past any other exception, so the application's exception and its own chain are never changed. - A value is written only when it holds a key, each write is guarded, and scrub_request_info never raises: it returns False when a value that holds a key can't be replaced. PermitConnectionError then attaches a plain ClientError that says so, and every raise site raises from original_error, so the unscrubbed error is neither attached nor the cause. handle_api_error doesn't chain to it either. - Every string and bytes argument and attribute of those exceptions is scrubbed, not a fixed list, and so is a yarl URL, which becomes a string when it holds a key. - Tests: the application's exception (frozen, headers with an int, read-only args) is left alone and PermitConnectionError is still raised, on both clients, for the PDP and the REST API; a scrub that fails is not attached or the cause, at every raise site; an error chained only as the context; a __cause__ cycle, in a subprocess; raw answers whose errors keep the key as bytes (a header line too long, a NUL in a header) and as a yarl URL; a padded key echoed in lower case. assert_holds_no_key also checks every attribute of every chained exception, and the raw-server test checks that each case still reaches a key to replace. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ceptions alone The redaction list says which chained exceptions are covered (those aiohttp and its libraries raised during the request), that string and bytes values and an invalid-URL error's URL are, the latter becoming a string, and that values that aren't text, such as a parsed HTTP message, are not. It says the SDK never changes an exception it didn't raise, and what it attaches when the key can't be removed. "Request URLs" now names the URLs that are not covered, so it no longer reads as contradicting the invalid-URL item. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
EliMoshkovich
requested review from
zeevmoney
and
a balanced review from Copilot
October 29, 2026 21:19
zeevmoney
requested changes
Oct 30, 2026
zeevmoney
left a comment
zeevmoney
left a comment
Member
There was a problem hiding this comment.
Changes requested: 1 HIGH, 3 MEDIUM, 3 LOW.
Blocking:
- HIGH
permit/exceptions.py:324: the frame check also applies to the error the SDK caught. When that error fails it, nothing is scrubbed and the scrub still reports success.
Non-blocking:
- MEDIUM
permit/exceptions.py:274: the walk now stops at errors raised in packages missing from the list, some of which 1eea9d8 scrubbed. - MEDIUM
tests/test_fix_logging.py:1466: only 2 of the application-exception test's 12 cases catch a regression. - MEDIUM
permit/exceptions.py:408: the fail-closed paths outsidePermitConnectionErrorhave no test. - LOW
permit/exceptions.py:538: when the scrub fails, the error that wasn't attached is still the raised exception's__context__. - LOW
README.md:89: the fail-closed sentence holds only forPermitConnectionError. - LOW
tests/test_fix_logging.py:1276: the raw-server cases run only on aiohttp's C parser, and the positive control can pass without reaching a chained error.
All 7 threads from the previous review were checked at d2d1f86 and replied to. The previous HIGH, which changed the application's own exceptions, is fixed.
Details are in the inline comments on each line.
…rub can't reach - The error the SDK caught is always scrubbed; the frame check applies only to what is chained under it. - The exception the application was handling when it called the SDK is passed in as `handled` and is left alone, with everything under it. - Any other exception left as it is (raised in a package off the list, or with a frame of the application's), or text the scrub can't rewrite (a UnicodeError's `object`), makes the scrub return False if it still holds a key. - PermitConnectionError and PermitApiError are raised after the `except` block, so an error that wasn't attached isn't their context. - Tests: the caught error with a foreign frame; an idna error in a redirect host; a yarl-raised error; all four PDP calls with the application's exception; a refused connection; the frame-check, fail-closed, bytearray and builtin-field paths; and the raw answers on aiohttp's pure-Python parser. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A traceback shows every member of an exception group and what is chained under each, so the scrub now walks them as it walks a cause and a context: members raised during the request are scrubbed, and a key in one left as it is fails the scrub. Works with the exceptiongroup backport on 3.10. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TEQWbhLbnWSwco3H5iNFis
The check for a key the scrub could not reach also catches most write failures, so several tests passed whether or not the scrub worked. They now check that the error is attached where the scrub should succeed, and new cases cover: a write that fails out of sight of that check, an error without the key that can't be written to, a key held only in an error's text or only in its args, a cycle among the exceptions the scrub leaves as they are, and the exception the application was handling, under one left as it is, with frames only in the request's packages, and while an error body that isn't JSON is read. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TEQWbhLbnWSwco3H5iNFis
zeevmoney
approved these changes
Oct 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Linear: PER-16680
This ports #137 (Zeev) to
main, with review follow-ups on top. #137 is stacked on the release-CI draft #136; this PR carries only #137's own changes, so the logging fix can ship in a patch release without that stack.Changes
api_urlandpdpat debug, without any user name or password in them.permit.utils.sdk_loggeris what the SDK logs through:log.levelandlog.labelto the SDK's own records, without adding a sink or changing the application's loguru setup.leveltakes a name in any case, or a loguru severity number. An unknown one is warned about once per process.opt(lazy=True)). The enforcer's debug lines pass a function, so theirpformatruns only for a record a sink receives.[REDACTED]in the records the SDK logs, in the messages and error bodies of the exceptions it raises (PermitConnectionErrorscrubs its own message when it is a string, and keeps any other value as it is), and in the aiohttp objects those exceptions carry. The aiohttp error the SDK caught is always scrubbed. So are the exceptions chained under it (as a cause or a context, or as a member of an exception group) that aiohttp and the libraries it uses raised during the request: their string and bytes arguments and attributes, and an invalid-URL error's URL. The README lists what is not covered.sys.exc_info()at each entry point, passed to the scrub ashandled). A value is written only when it holds a key, andscrub_request_infonever raises.PermitConnectionErrorattaches a plainaiohttp.ClientErrorin its place, and aPermitApiErrorfor an error body that isn't JSON has no cause. Every raise site raisesfrom original_error, after itsexceptblock, so an error that wasn't attached isn't the context either.PermitApiErrorwith the scrubbed body as its details.check,bulk_checkandauthorized_usersraisePermitConnectionErrorfor it.without_userinfo, which never raises: an unparseable URL shows as<invalid URL>, IPv6 brackets are kept).PermitConfig.tokenisrepr=False.wait_for_sync()builds a client of the same class again, so a subclass's own setup runs. Thekeep_log_settingscontext variable keeps the process's log settings unchanged while it does.loguru.loggerdirectly, and the standardloggingmodule, inside the package.multidict>=4.5,<7(aiohttp's own range) is declared, sincepermit/exceptions.pyimports it;uv.lockadds it to the package's dependency list (no new package). The lowest-direct floor lanes therefore run multidict 4.5.0, as pure Python.main:Permit.__init__builds the four clients, andsync.Permit.__init__callssuper().__init__()and then builds the blocking ones.Behaviour change
log.levelandlog.labelused to be ignored. Withlog={"enable": True}and nothing else, the defaults now apply:info).[Permit].Set
level="debug"orlabel=""to keep the old output.Left as is
Tests
tests/test_fix_logging.py: 233 tests, covering:PermitConnectionErrorfor every enforcer call and the REST API, checked against the whole formatted traceback and the lower-cased key__cause__cycle, and exception-group members; the raw-server cases run again on aiohttp's pure-Python parserargs,headerswith an int or holding the key, a refused connection) is left unchanged for all five calls, andPermitConnectionErrorstill carries the aiohttp errorPermitConnectionErrormessage that isn't a stringwait_for_sync, and a subclass's own_connectleft aloneOffline suite (
pytest -m "not e2e"): 537 passed, 4 skipped on Python 3.10 and 3.13 with pydantic 2, on Python 3.10 with pydantic 1, and on the Python 3.10 floor lane. The e2e tests need a livePDP_API_KEY. ruff, ruff format and mypy pass.The failing Dependency Audit check also fails on
main, with the same advisories (uv, aiohttp, filelock, dnspython). This PR'suv.lockchange is only the twomultidictlines.Description updated during review to match fbc59c0 (the client-construction bullet:
sync.Permit.__init__built the sync clients, not only calledsuper(); it is now a_connect()override; redaction wording made neutral).Description updated during review to match bce2118 (no
_connect()hook any more, lazy records,PermitConnectionErrorscrub, non-JSON error bodies,loggingban,multidictdependency anduv.lock, test counts).Description updated during review to match 1eea9d8 (lower-case key, chained exceptions, non-string
PermitConnectionErrormessages, PDP error bodies in another charset, multidict floor, test counts).Description updated during review to match d2d1f86 (which chained exceptions are scrubbed, application exceptions left alone, fail-closed
PermitConnectionError, tests and counts).Description updated during review to match 43454fd (the caught error always scrubbed, exception-group members,
handled, fail closed on a key left in place, raise after theexceptblock, tests and counts).🤖 Generated with Claude Code