[SILO-964] chore: route all /auth/o/token calls to internal api base url

[Prashant-Surya]

Description

• Looks like requests from MCP server for tokens are being blocked by cloudflare WAF
• By moving requests to internal url we bypass these rate limits.
• Use uv.lock as main source for building plane-mcp-server image

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• Improvement (change that would cause existing functionality to not work as expected)

Test Scenarios

• Verified in local that if internal base url is provided it's used for /o/token endpoints
• For all public use cases like authorization page, public base url is used.
• Nothing breaks, even if internal base url is not provided.

References

[SILO-964]

Summary by CodeRabbit

• New Features

  • Added configurable internal base URL to enable separate server-to-server token exchanges while keeping external authorization flows unchanged.

• Refactor

  • Simplified build steps to copy dependency manifests in one step and use locked-version synchronization for installs.

• Chores

  • Improved runtime logging around token storage and authentication configuration to make Redis vs in-memory usage clearer.

[makeplane]

Linked to Plane Work Item(s)

• [SILO-964] MCP Server seems to fail sometimes with OAuth Error

This comment was auto-generated by Plane

[coderabbitai]

📝 Walkthrough

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main objective of the changeset: routing token endpoint calls to an internal API base URL to bypass Cloudflare WAF blocks.
Merge Conflict Detection	✅ Passed	✅ No merge conflicts detected when merging into canary

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/mcp-auth-token-errors

---

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments

plane_mcp/auth/plane_oauth_provider.py (4)

126-135: Token prefix logged at info level — consider the security implication.

Line 129 logs the first 20 characters of the bearer token at info level. While this is pre-existing code, it's worth noting that token prefixes in logs can aid credential-guessing or replay attacks if logs are exfiltrated. Consider redacting or reducing this to debug level.

---

148-153: Good: structured logging placeholders here, but style is inconsistent with the rest of the file.

Lines 150–152 now use %s placeholders (lazy evaluation, better for structured logging), which is the preferred pattern. However, the rest of the file (lines 129, 135, 147, 162, 201, 204) still uses f-strings in log calls. Consider aligning all log statements to one style for consistency.

---

252-252: Missing docstring entry for plane_internal_base_url.

The plane_base_url parameter is documented in the Args docstring (lines 288–289), but the new plane_internal_base_url parameter is not. Add a brief description for discoverability.

📝 Suggested docstring addition

             plane_base_url: Base URL for Plane API
                 (defaults to https://api.plane.so or PLANE_BASE_URL env var)
+            plane_internal_base_url: Internal base URL for server-to-server API calls
+                (e.g., token exchange, verification). Falls back to PLANE_INTERNAL_BASE_URL
+                env var, then to plane_base_url if not set.
         """

Also applies to: 288-290

---

327-334: Note: two different env vars can set the internal URL.

PLANE_OAUTH_PROVIDER_PLANE_INTERNAL_BASE_URL (via Pydantic SettingsConfigDict.env_prefix on line 78) and PLANE_INTERNAL_BASE_URL (via the explicit os.getenv on line 332) both feed into the same value. This is fine as a belt-and-suspenders approach, but it may confuse operators. Consider documenting the canonical env var in a README or deployment guide.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@Dockerfile`:
- Around line 19-20: The Dockerfile currently runs "uv sync --frozen --no-cache
--no-dev" which creates /app/.venv but the ENTRYPOINT ["python", "-m",
"plane_mcp"] invokes the base image Python, causing missing deps; fix by either
(A) exporting PATH to prioritize the venv bin directory before the ENTRYPOINT so
the venv's python is used, or (B) change the ENTRYPOINT to invoke "uv run" so
the environment is auto-activated, or (C) run uv sync with the --system flag to
install into the system Python; update the Dockerfile accordingly and keep the
existing ENTRYPOINT symbol name ("ENTRYPOINT" invoking python -m plane_mcp) or
replace it with the uv-run variant.

🧹 Nitpick comments (4)

plane_mcp/auth/plane_oauth_provider.py (1)

126-129: Pre-existing: token prefix is logged at INFO level.

Line 129 logs the first 20 characters of the bearer token. Even partial tokens can be exploited if logs are compromised. Consider removing this or gating it behind DEBUG level in a follow-up.

plane_mcp/server.py (2)

15-17: Move import logging to the module level.

Function-scoped imports of standard library modules are unconventional and add slight overhead on every call. logging is a standard import that belongs at the top of the file alongside os.

Proposed fix

At the top of the file:

import logging
import os

Then inside the function, just:

logger = logging.getLogger(__name__)

Or even better, use the same get_logger utility imported in the auth module for consistency.

---

41-42: Empty-string defaults bypass the NotSet sentinel in the provider.

When the env var is unset, os.getenv("PLANE_INTERNAL_BASE_URL", "") passes "" to the provider, which then has to rely on the falsy-ness of "" to fall through to its own env-var lookup. This works today but is fragile — if the provider ever tightens validation (e.g., URL format check), an empty string would fail differently than NotSet.

Prefer passing None (or omitting the kwarg) so the provider's NotSet / env-var fallback logic is exercised cleanly:

Suggested change

-            plane_base_url=os.getenv("PLANE_BASE_URL", ""),
-            plane_internal_base_url=os.getenv("PLANE_INTERNAL_BASE_URL", ""),
+            plane_base_url=os.getenv("PLANE_BASE_URL") or NotSet,
+            plane_internal_base_url=os.getenv("PLANE_INTERNAL_BASE_URL") or NotSet,

This requires importing NotSet from fastmcp.utilities.types. Alternatively, simply omit the kwargs when the env vars aren't set, or let the provider handle the env vars entirely (it already does via os.getenv fallback in lines 327–334 of the provider).

Dockerfile (1)

13-13: Consider pinning the uv version for reproducible builds.

Using :latest for the uv image means builds are not fully reproducible — a future uv release with breaking changes could silently break the build. Since this PR is already improving reproducibility by adding uv.lock, pinning the uv version would complete that story. The current latest version is 0.10.0.

Suggested change

-COPY --from=ghcr.io/astral-sh/uv:latest /uv /usr/local/bin/uv
+COPY --from=ghcr.io/astral-sh/uv:0.10.0 /uv /usr/local/bin/uv