[PranavaKCode]

Description

• reads MANTIS_INTERNAL_SERVICE_SECRET into the SDK configuration
• sends X-Internal-Secret with the existing internal-service marker and user ID
• fails before any network request when internal-user mode lacks the required secret
• redacts the internal secret from debug transport logs
• preserves cookie auth when an ambient internal user ID has no service secret, with explicit mixed-auth precedence
• documents and tests the authenticated internal-service contract

Motivation and context

MantisAPI PR #2442 replaced the spoofable marker-only bypass with an authenticated X-Internal-Secret contract. SDK callers using MANTIS_INTERNAL_USER_ID must send that same provisioned secret or their requests fall through to browser authentication.

Fixes #11.

This is intentionally fail-closed: the SDK accepts a provisioned secret but never derives or logs it. Cookie authentication is unchanged.

How has this been tested?

• python -m ruff check .
• uv run --with pytest --with pytest-asyncio pytest -q (74 tests passed)
• focused coverage verifies the valid three-header set, missing-secret failure, mixed cookie/internal precedence, environment loading, and debug-log redaction
• git diff --check

Screenshots (if appropriate):

Not applicable; this is an SDK authentication contract change.

Types of changes

• ✅ Bug fix (non-breaking change which fixes an issue)
• ✅ Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• My code follows the code style of this project. [REQUIRED]
• Ran ruff check. There are no style warnings. [REQUIRED]
• Ran pytest. All new and existing tests passed. [REQUIRED]
• Recorded a video walkthrough showing the platform changes, testing used to verify the fix, and a brief review of the code changes made in this PR. [REQUIRED]
• My change requires a change to the documentation and I have updated it accordingly.
• I have added tests to cover my changes.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0e7fe789ee

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[FungousLand1941]

I read through the auth/header changes and tests. The secret redaction and fail-closed internal-service path look solid. Two edge cases seem worth tightening before merge:

1. MantisClient.__init__ still says that setting internal_user_id / MANTIS_INTERNAL_USER_ID is sufficient for backend-to-backend auth. With this PR that guidance is incomplete; the error should mention internal_service_secret / MANTIS_INTERNAL_SERVICE_SECRET too so the first configuration failure points to the full contract.

2. The PR says cookie authentication is unchanged, but HttpClient.auth_headers() now raises whenever internal_user_id is present without a secret, even if a valid cookie was supplied. That means a process using cookie auth with a stale/ambient MANTIS_INTERNAL_USER_ID will regress from working to raising ConfigurationError. If mixed cookie + internal configuration is intentionally fail-closed, I think it should be documented and covered by a test. If cookie should take precedence, the secret requirement probably needs to be gated to the internal-auth path.

The existing tests cover pure cookie and pure internal-service auth well; a mixed-auth regression test would make the intended precedence explicit.

[PranavaKCode]

Addressed the mixed-auth feedback in 74af9dc. A cookie now remains usable when an ambient internal user ID has no service secret; internal-only requests still fail closed. The README documents precedence when both complete methods are present, the constructor error names both required settings, and regression tests cover both mixed cases. Verified with Ruff, 74 passing tests, and git diff --check. Ready for a fresh review.

[PranavaKCode]

@MantisCartography Could you review the current head 74af9dcb when available? The constructor guidance and both mixed cookie/internal-auth cases raised in the prior review are addressed, and Python 3.10-3.12 CI plus build are green. Please flag any remaining contract or deployment prerequisite before merge.

[PranavaKCode]

@FungousLand1941 Your two auth edge cases are addressed in head 74af9dcb: the constructor error now names both required internal settings, and an ambient internal user ID no longer breaks cookie auth when the service secret is absent. README and mixed-auth regression tests cover the precedence, with all SDK CI checks green. If you have time for a fresh review, I would appreciate a look.

[PranavaKCode]

Deployment-contract note for review: current MantisAPI main derives settings.INTERNAL_SERVICE_SECRET from SECRET_KEY when backend env INTERNAL_SERVICE_SECRET is unset (the zero-config path from #2442). An external SDK process cannot obtain that derived value without sharing the Django SECRET_KEY, which it should not do. This SDK PR safely fails closed and its unit/CI coverage is green, but an actual external internal-user session needs an explicitly provisioned backend INTERNAL_SERVICE_SECRET and the matching SDK MANTIS_INTERNAL_SERVICE_SECRET deployed together (including blue/green backend instances), followed by an end-to-end auth check. Cookie-only callers do not need that credential. I have not verified production secret provisioning, so please do not treat the header tests alone as proof that external service mode works in production.

[FungousLand1941]

Rechecked the complete diff at 74af9dcb3efe6c9f38165875724900ee9aa8f5f3 following the replies to my original review.

Both concerns are addressed: the constructor error now names the internal user ID and service secret, and an ambient internal user ID without a secret no longer breaks cookie authentication. The new tests also cover the complete mixed-auth case, missing-secret fail-closed behavior, environment loading, and secret redaction in debug logs. The README states the intended precedence.

Local verification on Python 3.11: python -m pytest -q — 74 passed; ruff check . — all checks passed. The PR's Python 3.10/3.11/3.12 test jobs and build are green on this same head.

No remaining findings in the SDK changes from my earlier review. This verification uses mocked transport; it does not establish that external service credentials are provisioned or that production backend authentication has been exercised. The deployment-contract prerequisite described in the existing discussion remains a separate end-to-end check.