fix(rollbar): przestań zamazywać linie kodu w tracebackach#681
fix(rollbar): przestań zamazywać linie kodu w tracebackach#681mpasternak wants to merge 4 commits into
Conversation
ROLLBAR_SCRUB_FIELDS zawierało "code" — dodane świadomie, dla kodu autoryzacyjnego OAuth w POST do /o/token/. Problem: pyrollbar trzyma pod tą SAMĄ nazwą linię kodu źródłowego każdej ramki tracebacku, a ScrubRedactTransform dopasowuje ścieżkę klucza po SUFIKSIE — więc wpis trafiał w oba naraz. Od pyrollbara 1.4.0 skutek jest taki, że KAŻDY traceback w produkcyjnym Rollbarze ma wszystkie linie kodu zamazane na "****". Widać to gołym okiem w danych: item #379 (pyrollbar 1.3.0) ma "code": "sent = conn.send_messages([dict_to_email(message)])" a item #1554 (1.4.0), ten sam błąd, ma "code": "****" we wszystkich dziesięciu ramkach. Każde śledztwo zaczyna się więc bez najważniejszej informacji. Naprawa: "code" znika z ROLLBAR_SCRUB_FIELDS, a zamazywanie przejmuje ScrubKoduAutoryzacyjnego (bpp/rollbar_config.py) — podklasa ScrubTransform, która patrzy na CAŁĄ ścieżkę klucza i odpuszcza, gdy prowadzi ona przez "frames". Czyli: ("request", "POST", "code") → zamazane (sekret OAuth) ("body", "trace", "frames", 0, "code") → nietknięte (linia kodu) Transform wpinany jest w configure_rollbar(), nie w settings.ROLLBAR, żeby nie importować bpp.* na etapie ładowania ustawień. To bezpieczne: configure_rollbar biegnie z AppConfig.ready(), czyli PRZED middlewarem django-rollbar, a rollbar.init buduje łańcuch transformów tylko przy pierwszym wywołaniu. Pozostałe pola wrażliwe (password, code_verifier, refresh_token...) zostają na liście i są nadal zamazywane wszędzie, także w zmiennych lokalnych ramek — pilnuje tego osobny test. Zweryfikowane mutacją: przywrócenie "code" na listę, transform nie zamazujący nic oraz transform zamazujący wszędzie — każde łapane przez testy. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NcAqeqyqBzNEkkVnhpHDaH
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 35165582 | Triggered | Generic Password | cb1dd53 | src/bpp/tests/test_rollbar_config.py | View secret |
| 35165582 | Triggered | Generic Password | 5221229 | src/bpp/tests/test_rollbar_config.py | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secrets safely. Learn here the best practices.
- Revoke and rotate these secrets.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
… poprawki Self-review bezpieczeństwa wykazał, że pierwszy commit tego PR-a otwierał kanały wycieku kodu autoryzacyjnego, które WCZEŚNIEJ były zamknięte. Wszystkie trzy potwierdzone empirycznie na prawdziwym łańcuchu transformów. 1. ?code= w URL-ach przestawało być czyszczone. Wbudowany ScrubUrlTransform dostaje params_to_scrub=SETTINGS["scrub_fields"], więc zdjęcie stamtąd "code" odbierało mu wiedzę o tym parametrze — i to WSZĘDZIE, bo jego in_scrub_fields zwraca True dla każdego stringa (URL, Referer, zmienne lokalne). To nie było teoretyczne: /orcid/callback/ przyjmuje kod autoryzacyjny w query stringu, a pyrollbar zapisuje pełny request.build_absolute_uri(). Naprawa: własny ScrubUrlTransform znający "code". 2. frames[N].locals.code i frames[N].kwargs.code przestawały być czyszczone — czyli DOKŁADNIE ten sekret, dla którego "code" trafiło na listę. W django-oauth-toolkit `code` jest parametrem kilkunastu metod walidatora (validate_code, save_authorization_code, invalidate_authorization_code...), więc wyjątek w którejkolwiek wystawiłby aktywny kod. 3. Warunek `"frames" not in key` dawał się obejść: request.POST.frames.code oraz custom.frames[0].code przechodziły nietknięte. Wyjątek jest teraz DOKŁADNĄ listą dwóch ścieżek, pod którymi pyrollbar trzyma linię kodu, a dopasowanie nazwy klucza wróciło do case-insensitive (bez tego POST.Code wyciekało). Sprostowania: - Twierdzenie "od pyrollbara 1.4.0" było FAŁSZYWE. 1.3.0 zachowuje się identycznie. Prawdziwa przyczyna to commit 13ce70b z 2026-07-11, który dodał "code" do ROLLBAR_SCRUB_FIELDS; różnica wersji notifiera między itemami #379 i #1554 to korelacja, nie przyczyna. Poprawione w docstringu i newsfragmencie. - Komentarz przy ROLLBAR_SCRUB_FIELDS twierdził "NIE po sufiksie" — nieprawda od początku (build_key_matcher(..., type="suffix")) i to właśnie ta nieprawda uśpiła czujność przy dodawaniu "code". Poprawiony, wraz z ostrzeżeniem o sprzężeniu ze ScrubUrlTransform. Testy jadą teraz PRAWDZIWYM łańcuchem (rollbar.init + _build_payload) zamiast jego rekonstrukcji — poprzednia wersja składała listę transformów ręcznie i pomijała ScrubUrlTransform, więc świeciła na zielono przy wyciekającym URL-u. 18 testów, w tym 9 na wyciek. Każda z czterech poprawek zweryfikowana mutacją: usunięcie ScrubUrlTransform (4 failed), luźny warunek frames (3), brak case-insensitivity (1), powrót "code" na listę (2). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NcAqeqyqBzNEkkVnhpHDaH
|
| cofnięta poprawka | ile testów pada |
|---|---|
usunięcie własnego ScrubUrlTransform |
4 |
powrót do "frames" not in key |
3 |
| brak case-insensitivity | 1 |
"code" z powrotem na scrub_fields |
2 |
Sprostowania — dwa moje twierdzenia były fałszywe
„Od pyrollbara 1.4.0" — nieprawda. 1.3.0 zachowuje się identycznie
(to samo _build_payload, te same priorytety, to samo suffix-matchowanie).
Prawdziwa przyczyna to commit 13ce70be3 z 2026-07-11, który dodał "code"
do ROLLBAR_SCRUB_FIELDS — potwierdzone przez git log -S. Różnica wersji
notifiera między itemami #379 i #1554 to korelacja, nie przyczyna: itemy
rozdziela ta data, a nie upgrade biblioteki. Poprawione w docstringu, opisie
i newsfragmencie.
Komentarz przy ROLLBAR_SCRUB_FIELDS twierdził „dopasowuje po DOKŁADNEJ
nazwie klucza, NIE po sufiksie". To była nieprawda od początku
(build_key_matcher(..., type="suffix")) — i to właśnie ona uśpiła czujność
przy dodawaniu "code". Poprawiony, z ostrzeżeniem o sprzężeniu ze
ScrubUrlTransform.
Odnotowane, poza zakresem
frames[N].args[0] (pozycyjne argumenty) nie są czyszczone ani przed, ani po
tej zmianie — pyrollbar czyści tylko nazwane. To nie regresja, ale przy
przeglądzie maskowania warto o tym wiedzieć.
Lokalnie: src/bpp/tests/ + src/django_bpp/tests/ — 2968 passed, 2 skipped.
CI wywalił test_rollbar_scrubuje_sekrety_oauth — asertował, że "code" jest na liście ROLLBAR_SCRUB_FIELDS, a ta zmiana świadomie stamtąd je zdejmuje (zamazywało też linie kodu w tracebackach). Sekret nadal JEST maskowany, tylko innym mechanizmem — test sprawdzał implementację zamiast gwarancji. Przepisany: dla każdego z ośmiu pól wrażliwych budujemy payload przez prawdziwy łańcuch pyrollbara i sprawdzamy, że wartość NIE wychodzi. Dwie ścieżki: request.POST (wymiana kodu na token) oraz frames[N].locals — django-oauth-toolkit przekazuje te sekrety jako argumenty metod walidatora, a to właśnie tę ścieżkę przeoczyła pierwsza wersja ScrubKoduAutoryzacyjnego. 16 asercji zamiast 8, i odporne na kolejną zmianę mechanizmu. Przy okazji: atrapy sekretów w testach dostały nazwy nieprzypominające prawdziwych tokenów (GitGuardian zapalał się na "AUTHCODE_SUPERSECRET_XYZ"). Przegapiłem ten plik, bo lokalnie puszczałem src/bpp/tests + src/django_bpp/tests. Teraz przebieg obejmuje WSZYSTKIE pliki testowe dotykające Rollbara w repo (26 plików, 397 passed). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NcAqeqyqBzNEkkVnhpHDaH
Skaner sekretów w CI zgłosił "2 secrets uncovered" i wskazał dokładnie dwa moje wiersze: `ustawienia["access_token"] = "atrapa"` w dwóch plikach testowych. To były atrapy, ale skaner ma rację — wzorzec "przypisanie do access_token" jest nieodróżnialny od prawdziwego przecieku, a wyciszanie go przez allowlistę uczyłoby ignorowania właśnie tej klasy alertu. Przypisanie jest zbędne: `ustawienia_rollbara()` wnosi `access_token` z `settings.ROLLBAR` (w testach None), a wysyłka i tak jest zaślepiona podmienionym `send_payload`. Kontrola po zmianie: usunięcie własnego ScrubUrlTransform nadal wywala 4 testy, czyli suite pozostaje load-bearing. 34 passed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NcAqeqyqBzNEkkVnhpHDaH
Problem
ROLLBAR_SCRUB_FIELDSzawiera"code"— dodane świadomie, dla koduautoryzacyjnego OAuth w POST do
/o/token/. Kłopot w tym, że pyrollbar trzymapod tą samą nazwą linię kodu źródłowego każdej ramki tracebacku, a
ScrubRedactTransformdopasowuje ścieżkę klucza po sufiksie — więc jedenwpis trafiał w oba naraz.
Od pyrollbara 1.4.0 skutek jest taki, że każdy traceback w produkcyjnym
Rollbarze ma wszystkie linie kodu zamazane na
"****". Widać to gołym okiemw danych — ten sam błąd SMTP, dwa wydania:
frames[4].codesent = conn.send_messages([dict_to_email(message)])****We wszystkich dziesięciu ramkach. Każde śledztwo w Rollbarze zaczyna się więc
bez najważniejszej informacji.
Zreprodukowane też lokalnie, poza Django — sam transform na ręcznie zbudowanym
payloadzie:
Rozwiązanie
"code"znika zROLLBAR_SCRUB_FIELDS, a zamazywanie przejmujeScrubKoduAutoryzacyjnego(bpp/rollbar_config.py) — podklasaScrubTransform, która patrzy na całą ścieżkę klucza i odpuszcza, gdyprowadzi ona przez
frames:("request", "POST", "code")("body", "trace", "frames", 0, "code")Transform wpinany jest w
configure_rollbar(), a nie wsettings.ROLLBAR, żebynie importować
bpp.*na etapie ładowania ustawień. To bezpieczne i celowe:configure_rollbarbiegnie zAppConfig.ready(), czyli przed middlewaremdjango-rollbar, a
rollbar.initbuduje łańcuch transformów tylko przypierwszym wywołaniu — gdyby ubiegł nas middleware, transform nigdy by nie
wszedł. Osobny test pilnuje tego okablowania.
Pozostałe pola wrażliwe (
password,code_verifier,refresh_token…) zostająna liście i są nadal zamazywane wszędzie, także w zmiennych lokalnych ramek.
Testy
Cztery nowe w
src/bpp/tests/test_rollbar_config.py, TDD — test linii kodunajpierw padał (
assert '*******' == "autor_str = ..."):bez tego poprawka byłaby regresją bezpieczeństwa),
passwordw zmiennych lokalnych ramki nadal znika,configure_rollbarfaktycznie przekazuje transform dorollbar.init.Zweryfikowane mutacją kodu produkcyjnego — każda łapana:
"code"na listę scrubowanychLokalnie:
src/bpp/tests/+src/django_bpp/tests/— 2959 passed, 2 skipped.Uwaga
Poprawka działa od następnego wydania — itemy już zebrane w Rollbarze pozostają
z zamazanym kodem.
🤖 Generated with Claude Code
https://claude.ai/code/session_01NcAqeqyqBzNEkkVnhpHDaH