Skip to content

fix: add error logging to agentic provisioning endpoints#51418

Closed
MattBro wants to merge 2 commits intomasterfrom
matt/remove-api-version-check
Closed

fix: add error logging to agentic provisioning endpoints#51418
MattBro wants to merge 2 commits intomasterfrom
matt/remove-api-version-check

Conversation

@MattBro
Copy link
Copy Markdown
Contributor

@MattBro MattBro commented Mar 18, 2026

Problem

_error_response in the agentic provisioning views has no logging, making it impossible to diagnose 400/401 errors from production logs. We're seeing 401s on /api/agentic/provisioning/resources and can't tell why.

Changes

Added logger.warning to _error_response so all error responses from provisioning endpoints are logged with the error code and message.

How did you test this code

Verified in production logs that POST /api/agentic/provisioning/resources was returning 401 with no application-level log entry explaining the cause.

🤖 Generated with Claude Code

…ioning

Stripe does not consistently send the API-Version header. Remove the
verify_api_version check from all endpoints since HMAC signature is
the real auth layer. Also add logging to _error_response so we can
diagnose 400s from the provisioning endpoints.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Mar 18, 2026

Comments Outside Diff (1)

  1. ee/api/agentic_provisioning/signature.py, line 19-21 (link)

    P2 Unused constants after verify_api_version removal

    API_VERSION and API_VERSION_HEADER are now dead code — they were only referenced inside verify_api_version, which has been deleted. Neither constant is imported in views.py or anywhere else. Per the simplicity rule of "no superfluous parts", they should be removed.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: ee/api/agentic_provisioning/signature.py
    Line: 19-21
    
    Comment:
    **Unused constants after `verify_api_version` removal**
    
    `API_VERSION` and `API_VERSION_HEADER` are now dead code — they were only referenced inside `verify_api_version`, which has been deleted. Neither constant is imported in `views.py` or anywhere else. Per the simplicity rule of "no superfluous parts", they should be removed.
    
    
    
    How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
This is a comment left during a code review.
Path: ee/api/agentic_provisioning/views.py
Line: 164-165

Comment:
**`account_requests` now has no authentication**

After this PR, `account_requests` is the only provisioning endpoint with zero authentication. All other endpoints still call `verify_stripe_signature` as their auth layer, but `account_requests` never had it — it only had `verify_api_version`. Removing `verify_api_version` here leaves the endpoint completely open.

An unauthenticated caller can now:
- Trigger account creation for arbitrary email addresses
- Force existing users through an OAuth consent redirect

The PR description says "The HMAC signature is the real authentication layer", which is correct for the other endpoints — but `account_requests` does not call `verify_stripe_signature`. Should it be added here, or is there a specific reason this endpoint is intentionally unauthenticated? A comment explaining the decision would help clarify intent.

**Rule Used:** Add inline comments to clarify the purpose of sign... ([source](https://app.greptile.com/review/custom-context?memory=4d5b48c5-045d-4693-9cd9-4081bb19508b))

**Learnt From**
[PostHog/posthog#32083](https://github.com/PostHog/posthog/pull/32083)

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

---

This is a comment left during a code review.
Path: ee/api/agentic_provisioning/signature.py
Line: 19-21

Comment:
**Unused constants after `verify_api_version` removal**

`API_VERSION` and `API_VERSION_HEADER` are now dead code — they were only referenced inside `verify_api_version`, which has been deleted. Neither constant is imported in `views.py` or anywhere else. Per the simplicity rule of "no superfluous parts", they should be removed.

```suggestion
SIGNATURE_HEADER = "Stripe-Signature"
MAX_TIMESTAMP_DRIFT_SECONDS = 300
```

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

Last reviewed commit: "fix: remove API-Vers..."

Comment on lines 164 to 165
def account_requests(request: Request) -> Response:
if error := verify_api_version(request):
return error

data = request.data
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 account_requests now has no authentication

After this PR, account_requests is the only provisioning endpoint with zero authentication. All other endpoints still call verify_stripe_signature as their auth layer, but account_requests never had it — it only had verify_api_version. Removing verify_api_version here leaves the endpoint completely open.

An unauthenticated caller can now:

  • Trigger account creation for arbitrary email addresses
  • Force existing users through an OAuth consent redirect

The PR description says "The HMAC signature is the real authentication layer", which is correct for the other endpoints — but account_requests does not call verify_stripe_signature. Should it be added here, or is there a specific reason this endpoint is intentionally unauthenticated? A comment explaining the decision would help clarify intent.

Rule Used: Add inline comments to clarify the purpose of sign... (source)

Learnt From
PostHog/posthog#32083

Prompt To Fix With AI
This is a comment left during a code review.
Path: ee/api/agentic_provisioning/views.py
Line: 164-165

Comment:
**`account_requests` now has no authentication**

After this PR, `account_requests` is the only provisioning endpoint with zero authentication. All other endpoints still call `verify_stripe_signature` as their auth layer, but `account_requests` never had it — it only had `verify_api_version`. Removing `verify_api_version` here leaves the endpoint completely open.

An unauthenticated caller can now:
- Trigger account creation for arbitrary email addresses
- Force existing users through an OAuth consent redirect

The PR description says "The HMAC signature is the real authentication layer", which is correct for the other endpoints — but `account_requests` does not call `verify_stripe_signature`. Should it be added here, or is there a specific reason this endpoint is intentionally unauthenticated? A comment explaining the decision would help clarify intent.

**Rule Used:** Add inline comments to clarify the purpose of sign... ([source](https://app.greptile.com/review/custom-context?memory=4d5b48c5-045d-4693-9cd9-4081bb19508b))

**Learnt From**
[PostHog/posthog#32083](https://github.com/PostHog/posthog/pull/32083)

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

@MattBro
Copy link
Copy Markdown
Contributor Author

MattBro commented Mar 18, 2026

account_requests is authenticated via the @stripe_region_proxy(strategy="body_region") decorator, which calls verify_stripe_signature(request) before the view runs. See region_proxy.py line 130.

…n check

Add logging to _error_response so we can diagnose 400s. Restore
verify_api_version on all provisioning endpoints except oauth/token
(Stripe confirmed they send it on provisioning calls but not on
the standard OAuth token exchange).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@MattBro MattBro force-pushed the matt/remove-api-version-check branch from c30b592 to b2852e3 Compare March 18, 2026 01:16
@github-actions
Copy link
Copy Markdown
Contributor

🎭 Playwright report · View test results →

⚠️ 1 flaky test:

  • Creating a SQL insight with a variable and overriding it on a dashboard (chromium)

These issues are not necessarily caused by your changes.
Annoyed by this comment? Help fix flakies and failures and it'll disappear!

@MattBro MattBro changed the title fix: remove API-Version check and add error logging to agentic provisioning fix: add error logging to agentic provisioning endpoints Mar 18, 2026
@MattBro
Copy link
Copy Markdown
Contributor Author

MattBro commented Mar 18, 2026

Closing - will reopen with just the error logging change, without the version check removal.

@MattBro MattBro closed this Mar 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant