mirror of
https://github.com/Sea-Haven-Industries/open-swe.git
synced 2026-10-07 16:19:09 +00:00
Port fork webhook security delta onto modular handlers
Re-apply the fork's security customizations that #1621 did not carry: Linear webhook replay protection (freshness window on the signed webhookTimestamp), per-repo token-cache binding threaded through the thread token resolvers, the INTERNAL_BOT_LOGINS self-check in the review-finding-reply path, and a user-mapping cache refresh before email resolution on the issue and PR-comment paths (multi-replica staleness). Existing fork security tests pass unchanged. Refs: #80
This commit is contained in:
parent
0cac1ad363
commit
3f78c3ab15
2 changed files with 85 additions and 17 deletions
|
|
@ -816,8 +816,32 @@ async def _post_account_link_prompt(
|
|||
logger.debug("Failed to post account-link prompt to Slack", exc_info=True)
|
||||
|
||||
|
||||
LINEAR_WEBHOOK_MAX_AGE_SECONDS = 60
|
||||
|
||||
|
||||
def _linear_timestamp_is_fresh(body: bytes) -> bool:
|
||||
"""Reject replays: the signed payload's ``webhookTimestamp`` must be recent.
|
||||
|
||||
Linear includes ``webhookTimestamp`` (Unix milliseconds) inside the signed
|
||||
body. Fail closed when it is missing or malformed.
|
||||
"""
|
||||
try:
|
||||
ts_ms = json.loads(body)["webhookTimestamp"]
|
||||
except (json.JSONDecodeError, KeyError, TypeError):
|
||||
logger.warning("Linear webhook missing/invalid webhookTimestamp — rejecting")
|
||||
return False
|
||||
if not isinstance(ts_ms, (int, float)) or isinstance(ts_ms, bool):
|
||||
logger.warning("Linear webhook webhookTimestamp is not numeric — rejecting")
|
||||
return False
|
||||
now_ms = datetime.now(UTC).timestamp() * 1000
|
||||
if abs(now_ms - ts_ms) > LINEAR_WEBHOOK_MAX_AGE_SECONDS * 1000:
|
||||
logger.warning("Linear webhook timestamp outside freshness window — rejecting")
|
||||
return False
|
||||
return True
|
||||
|
||||
|
||||
def verify_linear_signature(body: bytes, signature: str, secret: str) -> bool:
|
||||
"""Verify the Linear webhook signature.
|
||||
"""Verify the Linear webhook signature and replay-freshness window.
|
||||
|
||||
Args:
|
||||
body: Raw request body bytes
|
||||
|
|
@ -825,15 +849,17 @@ def verify_linear_signature(body: bytes, signature: str, secret: str) -> bool:
|
|||
secret: The webhook signing secret
|
||||
|
||||
Returns:
|
||||
True if signature is valid, False otherwise
|
||||
True if the signature is valid AND the signed timestamp is fresh.
|
||||
"""
|
||||
if not secret:
|
||||
logger.warning("LINEAR_WEBHOOK_SECRET is not configured — rejecting webhook request")
|
||||
return False
|
||||
|
||||
expected = hmac.new(secret.encode("utf-8"), body, hashlib.sha256).hexdigest()
|
||||
if not hmac.compare_digest(expected, signature):
|
||||
return False
|
||||
|
||||
return hmac.compare_digest(expected, signature)
|
||||
return _linear_timestamp_is_fresh(body)
|
||||
|
||||
|
||||
@app.post("/webhooks/linear")
|
||||
|
|
@ -1695,31 +1721,36 @@ async def update_agent_thread_pr_state(payload: dict[str, Any]) -> None:
|
|||
logger.debug("Failed to update pr_state for thread %s", thread_id, exc_info=True)
|
||||
|
||||
|
||||
async def _refresh_thread_github_token_after_401(thread_id: str, email: str) -> str | None:
|
||||
async def _refresh_thread_github_token_after_401(
|
||||
thread_id: str, email: str, *, repo: dict[str, str] | None = None
|
||||
) -> str | None:
|
||||
"""Invalidate the cached token after a 401 and try to resolve a fresh one."""
|
||||
logger.warning(
|
||||
"GitHub returned 401 for thread %s; invalidating cached token and re-resolving",
|
||||
thread_id,
|
||||
)
|
||||
await invalidate_cached_github_token(thread_id)
|
||||
return await _get_or_resolve_thread_github_token(thread_id, email)
|
||||
return await _get_or_resolve_thread_github_token(thread_id, email, repo=repo)
|
||||
|
||||
|
||||
async def _get_or_resolve_thread_github_token(thread_id: str, email: str) -> str | None:
|
||||
async def _get_or_resolve_thread_github_token(
|
||||
thread_id: str, email: str, *, repo: dict[str, str] | None = None
|
||||
) -> str | None:
|
||||
"""Resolve and cache a GitHub token for a thread when available.
|
||||
|
||||
In bot-token-only mode, returns a fresh GitHub App installation token
|
||||
instead of resolving per-user OAuth tokens.
|
||||
instead of resolving per-user OAuth tokens. ``repo`` (owner/name) binds the
|
||||
cached entry so a colliding thread_id from a different repo cannot reuse it.
|
||||
"""
|
||||
if is_bot_token_only_mode():
|
||||
bot_token, expires_at = await get_github_app_installation_token_with_expiry()
|
||||
if bot_token:
|
||||
cache_github_token_for_thread(thread_id, bot_token, expires_at=expires_at)
|
||||
cache_github_token_for_thread(thread_id, bot_token, expires_at=expires_at, repo=repo)
|
||||
return bot_token
|
||||
logger.warning("Bot-token-only mode but GitHub App token unavailable")
|
||||
return None
|
||||
|
||||
github_token, _expires_at = await get_github_token_from_thread(thread_id)
|
||||
github_token, _expires_at = await get_github_token_from_thread(thread_id, expected_repo=repo)
|
||||
if github_token:
|
||||
return github_token
|
||||
|
||||
|
|
@ -1730,7 +1761,10 @@ async def _get_or_resolve_thread_github_token(thread_id: str, email: str) -> str
|
|||
|
||||
expires_at = auth_result.get("expires_at")
|
||||
cache_github_token_for_thread(
|
||||
thread_id, github_token, expires_at=expires_at if isinstance(expires_at, str) else None
|
||||
thread_id,
|
||||
github_token,
|
||||
expires_at=expires_at if isinstance(expires_at, str) else None,
|
||||
repo=repo,
|
||||
)
|
||||
return github_token
|
||||
|
||||
|
|
|
|||
|
|
@ -679,9 +679,23 @@ async def process_github_pr_comment(payload: dict[str, Any], event_type: str) ->
|
|||
"Failed to persist branch_name metadata for thread %s", thread_id
|
||||
)
|
||||
|
||||
# Refresh the per-process user-mapping cache from the Store before
|
||||
# resolving the author's email. On a multi-replica managed deployment this
|
||||
# replica's cache may be stale (a mapping created on another replica is not
|
||||
# otherwise visible), which would drop a legitimately-mapped user. Mirrors
|
||||
# the Slack mention path (process_slack_mention).
|
||||
try:
|
||||
await webapp.refresh_user_mapping_cache()
|
||||
except Exception: # noqa: BLE001
|
||||
webapp.logger.debug(
|
||||
"Could not refresh user mapping cache for GitHub PR comment", exc_info=True
|
||||
)
|
||||
|
||||
email = await webapp.email_for_login(github_login) or ""
|
||||
if email:
|
||||
github_token = await webapp._get_or_resolve_thread_github_token(thread_id, email)
|
||||
github_token = await webapp._get_or_resolve_thread_github_token(
|
||||
thread_id, email, repo=repo_config
|
||||
)
|
||||
else:
|
||||
webapp.logger.warning("No email mapping for GitHub user '%s', skipping", github_login)
|
||||
return
|
||||
|
|
@ -701,7 +715,9 @@ async def process_github_pr_comment(payload: dict[str, Any], event_type: str) ->
|
|||
node_id=node_id,
|
||||
)
|
||||
except GitHubAuthError:
|
||||
github_token = await webapp._refresh_thread_github_token_after_401(thread_id, email)
|
||||
github_token = await webapp._refresh_thread_github_token_after_401(
|
||||
thread_id, email, repo=repo_config
|
||||
)
|
||||
if not github_token:
|
||||
webapp.logger.warning("Re-auth failed for thread %s after 401; skipping", thread_id)
|
||||
return
|
||||
|
|
@ -723,7 +739,9 @@ async def process_github_pr_comment(payload: dict[str, Any], event_type: str) ->
|
|||
repo_config, pr_number, token=github_token
|
||||
)
|
||||
except GitHubAuthError:
|
||||
github_token = await webapp._refresh_thread_github_token_after_401(thread_id, email)
|
||||
github_token = await webapp._refresh_thread_github_token_after_401(
|
||||
thread_id, email, repo=repo_config
|
||||
)
|
||||
if not github_token:
|
||||
webapp.logger.warning("Re-auth failed for thread %s after 401; skipping", thread_id)
|
||||
return
|
||||
|
|
@ -753,7 +771,7 @@ async def process_github_review_finding_reply(payload: dict[str, Any]) -> None:
|
|||
|
||||
sender = payload.get("sender", {})
|
||||
sender_login = sender.get("login") if isinstance(sender, dict) else None
|
||||
if sender_login == "open-swe[bot]":
|
||||
if sender_login in webapp.INTERNAL_BOT_LOGINS:
|
||||
return
|
||||
|
||||
repo = payload.get("repository", {})
|
||||
|
|
@ -893,6 +911,16 @@ async def process_github_issue(payload: dict[str, Any], event_type: str) -> None
|
|||
webapp.logger.warning("Missing GitHub issue id/number, skipping")
|
||||
return
|
||||
|
||||
# Refresh the per-process user-mapping cache from the Store before
|
||||
# resolving the author's email (multi-replica staleness; mirrors the Slack
|
||||
# mention path in process_slack_mention).
|
||||
try:
|
||||
await webapp.refresh_user_mapping_cache()
|
||||
except Exception: # noqa: BLE001
|
||||
webapp.logger.debug(
|
||||
"Could not refresh user mapping cache for GitHub issue", exc_info=True
|
||||
)
|
||||
|
||||
email = await webapp.email_for_login(github_login) or ""
|
||||
if not email:
|
||||
webapp.logger.warning("No email mapping for GitHub user '%s', skipping", github_login)
|
||||
|
|
@ -900,7 +928,9 @@ async def process_github_issue(payload: dict[str, Any], event_type: str) -> None
|
|||
|
||||
thread_id = webapp.generate_thread_id_from_github_issue(issue_id)
|
||||
existing_thread = await webapp._thread_exists(thread_id)
|
||||
github_token = await webapp._get_or_resolve_thread_github_token(thread_id, email)
|
||||
github_token = await webapp._get_or_resolve_thread_github_token(
|
||||
thread_id, email, repo=repo_config
|
||||
)
|
||||
app_token = await webapp.get_github_app_installation_token()
|
||||
reaction_token = github_token or app_token
|
||||
comment = payload.get("comment", {})
|
||||
|
|
@ -919,7 +949,9 @@ async def process_github_issue(payload: dict[str, Any], event_type: str) -> None
|
|||
token=reaction_token,
|
||||
)
|
||||
except GitHubAuthError:
|
||||
github_token = await webapp._refresh_thread_github_token_after_401(thread_id, email)
|
||||
github_token = await webapp._refresh_thread_github_token_after_401(
|
||||
thread_id, email, repo=repo_config
|
||||
)
|
||||
reaction_token = github_token or app_token
|
||||
reacted = False
|
||||
if reaction_token:
|
||||
|
|
@ -953,7 +985,9 @@ async def process_github_issue(payload: dict[str, Any], event_type: str) -> None
|
|||
repo_config, issue_number, token=github_token or app_token
|
||||
)
|
||||
except GitHubAuthError:
|
||||
github_token = await webapp._refresh_thread_github_token_after_401(thread_id, email)
|
||||
github_token = await webapp._refresh_thread_github_token_after_401(
|
||||
thread_id, email, repo=repo_config
|
||||
)
|
||||
comments = await webapp.fetch_issue_comments(
|
||||
repo_config, issue_number, token=github_token or app_token
|
||||
)
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue