[P2] fix(sinks): Log only the webhook host on WebhookAuditSink POST failures

Open Beginner friendly
  • Is something wrong?
#767 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 3 days

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
80/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
python
Domain
security

Research direction

In src/doberman/storage/sinks.py, inspect WebhookAuditSink._post and its four failure log paths. Read the existing _stub_urlopen tests and add cases for the listed exception types, checking that the host appears and SYNTH-SECRET does not. Add a changelog.d fragment; the issue is done when all four paths log only the webhook host and the tests pass.

Written by the indexing model from the issue text.

Description

posthog

Operators who put a token in their audit webhook URL get that token written into the Doberman log every time a webhook POST fails.

Problem

WebhookAuditSink._post in src/doberman/storage/sinks.py logs the raw self._url on four failure paths: HTTPError, URLError, TimeoutError, and the general OSError fallback. The config loader (_load_webhook_config) checks only the scheme and the HTTPS-for-non-loopback rule. It never rejects userinfo or query strings, so a URL such as https://user:token@host/hook?token=... passes validation and goes into the log in full. The documented path (auth_env) is already protected, but nothing protects the URL itself. Tracked in #758.

Impact

This breaks the repo's never-expose-secrets directive, and that is a real problem for an authorization product. Exposure needs three things: a credential in the URL, a failed POST, and someone who can read the logs. PostHog telemetry has no event for webhook failures or log content, so I can't measure how often it happens.

Solution

Add a small helper that formats urlparse(url).hostname plus the port when one is set, with brackets for IPv6. Use it at all four log sites and keep the rest of each message as it is. Add unit tests that reuse the _stub_urlopen pattern. Each test sets up the sink with https://u:[email protected]:1/x?t=SYNTH-SECRET, raises each exception type, and asserts that SYNTH-SECRET is not in caplog.text and the host is. The change also needs a changelog.d fragment.

Expected impact

I have no usable baseline because these failures produce no telemetry. After the fix, the new tests should show zero credential leaks across all four failure paths. That is the only verifiable signal.

  • Actionability: immediately_actionable
  • Inbox status: ready
  • Signals behind it: 1

Open the report in PostHog

Filed automatically from the PostHog Self-driving inbox. It mirrors the finding; the fix is a maintainer's call.

Dominant language
Python
Stars
277
Forks
42
Avg merge
1d 19h
Merged PRs (30d)
16

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from DobermanCore/Doberman-Core

All issues in DobermanCore/Doberman-Core

Similar issues

More Python issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.