Skip to content

fix: trim whitespace from API keys and host config#525

Merged
marandaneto merged 7 commits intomainfrom
fix/trim-whitespace-in-config-keys-and-host
Apr 20, 2026
Merged

fix: trim whitespace from API keys and host config#525
marandaneto merged 7 commits intomainfrom
fix/trim-whitespace-in-config-keys-and-host

Conversation

@marandaneto
Copy link
Copy Markdown
Member

@marandaneto marandaneto commented Apr 20, 2026

💡 Motivation and Context

This ports the whitespace-trimming fix from posthog-go to posthog-python. It trims surrounding spaces and line breaks from the project API key, personal API key, and host before using them so the SDK does not silently send invalid tokens or build broken endpoints from whitespace-polluted configuration.

💚 How did you test it?

  • uv run --extra test pytest posthog/test/test_client.py -k "trims_api_key_whitespace or trims_host_and_personal_api_key_whitespace"
  • uv run --extra test pytest posthog/test/test_request.py -k "routing_to_custom_host or defaults_blank_host_after_trimming_whitespace or trims_whitespace_sensitive_config"
  • uv run ruff format --check .

📝 Checklist

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • I updated the docs if needed.
  • No breaking change or entry added to the changelog.

If releasing new changes

  • Ran sampo add to generate a changeset file
  • Added the release label to the PR

@marandaneto marandaneto requested a review from a team as a code owner April 20, 2026 13:08
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 20, 2026

Prompt To Fix All With AI
This is a comment left during a code review.
Path: posthog/test/test_client.py
Line: 44-62

Comment:
**Missing "trim from a valid key" case**

The test only exercises the all-whitespace path (`" \n\t "`) where the result is always an empty/None value. The primary purpose of this fix is to strip surrounding whitespace from a *valid* key (e.g., `" phc_mykey "``"phc_mykey"`), but that case is never asserted. A parametrised test covering both a valid-but-padded key and the whitespace-only key would express every idea the feature needs to express and would catch a regression where `.strip()` is accidentally removed.

```python
@pytest.mark.parametrize(
    "raw_key,expected",
    [
        (" phc_validkey ", "phc_validkey"),  # trim from a valid key
        (" \n\t ", ""),                       # all-whitespace collapses to empty
    ],
)
def test_trims_api_key_whitespace(raw_key, expected):
    ...
    assert client.api_key == expected
```

How can I resolve this? If you propose a fix, please make it concise.

Reviews (1): Last reviewed commit: "fix: trim whitespace from API keys and h..." | Re-trigger Greptile

Comment thread posthog/test/test_client.py Outdated
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 20, 2026

posthog-python Compliance Report

Date: 2026-04-20 14:24:22 UTC
Duration: 160033ms

✅ All Tests Passed!

30/30 tests passed


Capture Tests

29/29 tests passed

View Details
Test Status Duration
Format Validation.Event Has Required Fields 518ms
Format Validation.Event Has Uuid 1507ms
Format Validation.Event Has Lib Properties 1508ms
Format Validation.Distinct Id Is String 1507ms
Format Validation.Token Is Present 1507ms
Format Validation.Custom Properties Preserved 1507ms
Format Validation.Event Has Timestamp 1507ms
Retry Behavior.Retries On 503 9520ms
Retry Behavior.Does Not Retry On 400 3506ms
Retry Behavior.Does Not Retry On 401 3508ms
Retry Behavior.Respects Retry After Header 9514ms
Retry Behavior.Implements Backoff 23519ms
Retry Behavior.Retries On 500 7513ms
Retry Behavior.Retries On 502 7517ms
Retry Behavior.Retries On 504 7513ms
Retry Behavior.Max Retries Respected 23529ms
Deduplication.Generates Unique Uuids 1498ms
Deduplication.Preserves Uuid On Retry 7514ms
Deduplication.Preserves Uuid And Timestamp On Retry 14522ms
Deduplication.Preserves Uuid And Timestamp On Batch Retry 7511ms
Deduplication.No Duplicate Events In Batch 1503ms
Deduplication.Different Events Have Different Uuids 1507ms
Compression.Sends Gzip When Enabled 1508ms
Batch Format.Uses Proper Batch Structure 1507ms
Batch Format.Flush With No Events Sends Nothing 1005ms
Batch Format.Multiple Events Batched Together 1506ms
Error Handling.Does Not Retry On 403 3509ms
Error Handling.Does Not Retry On 413 3508ms
Error Handling.Retries On 408 7513ms

Feature_Flags Tests

1/1 tests passed

View Details
Test Status Duration
Request Payload.Request With Person Properties Device Id 517ms

Copy link
Copy Markdown

@ioannisj ioannisj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lg, but format before merge

@marandaneto marandaneto merged commit 1b098e7 into main Apr 20, 2026
26 checks passed
@marandaneto marandaneto deleted the fix/trim-whitespace-in-config-keys-and-host branch April 20, 2026 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants