Repository navigation
Commit a7cd148
PER-16680: log through an SDK logger that applies the log settings (#163)
* PER-16680: log through an SDK logger that applies the log settings
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>
* PER-16680: review follow-ups: scrub every raised error body, URL user 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>
* PER-16680: second review pass: scrub carried request headers, safe URL 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>
* PER-16680: scrub JSON object keys and the labelled log line
- 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>
* PER-16680: scrub the request info aiohttp keeps in an error's args and 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>
* PER-16680: scrub error bodies labelled JSON that aren't JSON, and aiohttp 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>
* PER-16680: build the clients in the constructors again, without a _connect 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>
* PER-16680: build and scrub a log record only when a sink will receive it
- 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>
* PER-16680: say exactly what is redacted and what "enable": True undoes; 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>
* PER-16680: scrub the errors chained under an aiohttp error, and a lower-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>
* PER-16680: the lazy-record test covers all four enforcer debug lines
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>
* PER-16680: note that the floor lanes run multidict 4.5.0 as pure Python
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>
* PER-16680: change only what the request raised, and never fail on a scrub
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>
* PER-16680: README: what the scrub covers, and that it leaves other exceptions 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>
* PER-16680: always scrub the caught error; fail closed on a key the scrub 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>
* PER-16680: walk the members of an exception group under an aiohttp error
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
* PER-16680: test the scrub paths the final key check would otherwise hide
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
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Zeev Manilovich <zeevmanilovich@gmail.com>1 parent ef80ae2 commit a7cd148
12 files changed
Lines changed: 2796 additions & 87 deletions
File tree
- permit
- api
- enforcement
- utils
- tests
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
40 | 40 | | |
41 | 41 | | |
42 | 42 | | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
43 | 108 | | |
44 | 109 | | |
45 | 110 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2 | 2 | | |
3 | 3 | | |
4 | 4 | | |
5 | | - | |
6 | 5 | | |
7 | 6 | | |
8 | 7 | | |
| 8 | + | |
9 | 9 | | |
10 | 10 | | |
11 | 11 | | |
| |||
68 | 68 | | |
69 | 69 | | |
70 | 70 | | |
71 | | - | |
| 71 | + | |
72 | 72 | | |
73 | 73 | | |
74 | | - | |
| 74 | + | |
75 | 75 | | |
76 | 76 | | |
77 | 77 | | |
| |||
241 | 241 | | |
242 | 242 | | |
243 | 243 | | |
244 | | - | |
| 244 | + | |
245 | 245 | | |
246 | 246 | | |
247 | 247 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
3 | | - | |
4 | | - | |
5 | 3 | | |
| 4 | + | |
6 | 5 | | |
7 | 6 | | |
8 | 7 | | |
| |||
198 | 197 | | |
199 | 198 | | |
200 | 199 | | |
201 | | - | |
| 200 | + | |
202 | 201 | | |
203 | 202 | | |
204 | 203 | | |
| |||
212 | 211 | | |
213 | 212 | | |
214 | 213 | | |
215 | | - | |
| 214 | + | |
216 | 215 | | |
217 | 216 | | |
218 | 217 | | |
| |||
227 | 226 | | |
228 | 227 | | |
229 | 228 | | |
230 | | - | |
| 229 | + | |
231 | 230 | | |
232 | 231 | | |
233 | 232 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
13 | 13 | | |
14 | 14 | | |
15 | 15 | | |
16 | | - | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
17 | 30 | | |
18 | 31 | | |
19 | | - | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
20 | 39 | | |
21 | 40 | | |
22 | | - | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
23 | 47 | | |
24 | 48 | | |
25 | 49 | | |
26 | | - | |
| 50 | + | |
| 51 | + | |
27 | 52 | | |
28 | 53 | | |
29 | 54 | | |
30 | 55 | | |
31 | | - | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
32 | 59 | | |
33 | 60 | | |
34 | 61 | | |
| |||
54 | 81 | | |
55 | 82 | | |
56 | 83 | | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
57 | 87 | | |
58 | 88 | | |
| 89 | + | |
59 | 90 | | |
60 | 91 | | |
61 | 92 | | |
| |||
0 commit comments