diff --git a/INSTALLATION.md b/INSTALLATION.md index 4f175c1e..42897ee1 100644 --- a/INSTALLATION.md +++ b/INSTALLATION.md @@ -223,7 +223,7 @@ ALLOWED_GITHUB_ORGS="langchain-ai,anthropics" ALLOWED_GITHUB_REPOS="some-user/their-repo,another-org/specific-repo" ``` -A webhook is accepted if the repo's org is in `ALLOWED_GITHUB_ORGS` **or** the `owner/repo` is in `ALLOWED_GITHUB_REPOS`. If both are empty, all repos are allowed. +A GitHub or Linear webhook is accepted if the resolved repo's org is in `ALLOWED_GITHUB_ORGS` **or** the `owner/repo` is in `ALLOWED_GITHUB_REPOS`. If both are empty, all repos are allowed. Slack mentions are not rejected from regex-inferred repository text; repository access is bounded by the GitHub App installation permissions. ### Linear (optional) @@ -386,7 +386,8 @@ GITHUB_OAUTH_PROVIDER_ID="" # The provider ID from steps 3a / 4b # Leave empty to allow all orgs. ALLOWED_GITHUB_ORGS="" # e.g. "my-org,my-other-org" # Comma-separated list of specific owner/repo pairs the agent is allowed to operate on. -# A repo is allowed if its org is in ALLOWED_GITHUB_ORGS OR its owner/repo is in ALLOWED_GITHUB_REPOS. +# For GitHub/Linear webhooks, a repo is allowed if its org is in ALLOWED_GITHUB_ORGS OR its owner/repo is in ALLOWED_GITHUB_REPOS. +# Slack mentions are not rejected from regex-inferred repository text; repository access is bounded by GitHub App installation permissions. # Leave both empty to allow all repos. ALLOWED_GITHUB_REPOS="" # e.g. "some-user/their-repo,another-org/specific-repo" diff --git a/agent/webapp.py b/agent/webapp.py index 3e775670..eb97c517 100644 --- a/agent/webapp.py +++ b/agent/webapp.py @@ -486,7 +486,6 @@ async def _upsert_slack_thread_repo_metadata( async def get_slack_repo_config( - message: str, channel_id: str, thread_ts: str, slack_user_id: str | None = None, @@ -494,31 +493,29 @@ async def get_slack_repo_config( """Resolve repository configuration for Slack-triggered runs. Priority: - 1. Explicit ``owner/repo`` mention in the message body. - 2. Repo carried over from the existing Slack thread's metadata. - 3. The triggering user's dashboard ``default_repo`` (if they have a + 1. Repo carried over from the existing Slack thread's metadata. + 2. The triggering user's dashboard ``default_repo`` (if they have a profile and their Slack email maps to a known GitHub login). - 4. ``SLACK_REPO_*`` env defaults. + 3. ``SLACK_REPO_*`` env defaults. """ default_owner = SLACK_REPO_OWNER.strip() or DEFAULT_REPO_OWNER default_name = SLACK_REPO_NAME.strip() or DEFAULT_REPO_NAME thread_id = generate_thread_id_from_slack_thread(channel_id, thread_ts) langgraph_client = get_client(url=LANGGRAPH_URL) - repo_config = extract_repo_from_text(message, default_owner=default_owner) + repo_config: dict[str, str] | None = None - if not repo_config: - try: - thread = await langgraph_client.threads.get(thread_id) - thread_repo_config = _extract_repo_config_from_thread(thread) - if thread_repo_config: - repo_config = thread_repo_config - except Exception as exc: # noqa: BLE001 - if not _is_not_found_error(exc): - logger.exception( - "Failed to fetch Slack thread %s for repo resolution", - thread_id, - ) + try: + thread = await langgraph_client.threads.get(thread_id) + thread_repo_config = _extract_repo_config_from_thread(thread) + if thread_repo_config: + repo_config = thread_repo_config + except Exception as exc: # noqa: BLE001 + if not _is_not_found_error(exc): + logger.exception( + "Failed to fetch Slack thread %s for repo resolution", + thread_id, + ) if not repo_config and slack_user_id: try: @@ -947,7 +944,9 @@ async def process_slack_mention(event_data: dict[str, Any], repo_config: dict[st prompt = ( "You were mentioned in Slack.\n\n" - f"## Repository\n{repo_config.get('owner')}/{repo_config.get('name')}\n\n" + "## Default Repository Hint\n" + f"{repo_config.get('owner')}/{repo_config.get('name')}\n" + "Use this only if the Slack conversation does not identify a different repository.\n\n" f"## Triggered by\n{trigger_user}\n\n" f"## Slack Thread\n- Channel: {channel_id}\n- Thread TS: {thread_ts}\n" f"- Context starts at: {context_source}\n\n" @@ -1370,15 +1369,7 @@ async def slack_webhook(request: Request, background_tasks: BackgroundTasks) -> "text": text, "bot_user_id": bot_user_id, } - repo_config = await get_slack_repo_config(text, channel_id, thread_ts, slack_user_id=user_id) - - if not _is_repo_allowed(repo_config): - logger.warning( - "Rejecting Slack webhook: repo '%s/%s' not in allowlist", - repo_config.get("owner"), - repo_config.get("name"), - ) - return {"status": "ignored", "reason": "Repository not in allowlist"} + repo_config = await get_slack_repo_config(channel_id, thread_ts, slack_user_id=user_id) background_tasks.add_task(process_slack_mention, event_data, repo_config) diff --git a/tests/test_github_issue_webhook.py b/tests/test_github_issue_webhook.py index 2dd6d5e9..0ed01a95 100644 --- a/tests/test_github_issue_webhook.py +++ b/tests/test_github_issue_webhook.py @@ -485,10 +485,9 @@ def test_slack_webhook_non_pr_review_request_starts_agent(monkeypatch) -> None: captured: dict[str, object] = {} async def fake_get_slack_repo_config( - text: str, channel_id: str, thread_ts: str, slack_user_id: str | None = None + channel_id: str, thread_ts: str, slack_user_id: str | None = None ) -> dict[str, str]: captured["repo_config_request"] = { - "text": text, "channel_id": channel_id, "thread_ts": thread_ts, "slack_user_id": slack_user_id, @@ -507,6 +506,13 @@ def test_slack_webhook_non_pr_review_request_starts_agent(monkeypatch) -> None: monkeypatch.setattr(slack_utils.time, "time", lambda: 1700000000) monkeypatch.setattr(webapp, "get_slack_repo_config", fake_get_slack_repo_config) monkeypatch.setattr(webapp, "process_slack_mention", fake_process_slack_mention) + monkeypatch.setattr( + webapp, + "_is_repo_allowed", + lambda repo_config: (_ for _ in ()).throw( + AssertionError("Slack webhook should not gate inferred repos with allowlists") + ), + ) client = TestClient(webapp.app) response = _post_slack_webhook( @@ -535,10 +541,9 @@ def test_slack_webhook_threaded_followup_uses_parent_thread_ts(monkeypatch) -> N captured: dict[str, object] = {} async def fake_get_slack_repo_config( - text: str, channel_id: str, thread_ts: str, slack_user_id: str | None = None + channel_id: str, thread_ts: str, slack_user_id: str | None = None ) -> dict[str, str]: captured["repo_config_request"] = { - "text": text, "channel_id": channel_id, "thread_ts": thread_ts, "slack_user_id": slack_user_id, @@ -577,7 +582,6 @@ def test_slack_webhook_threaded_followup_uses_parent_thread_ts(monkeypatch) -> N assert response.status_code == 200 assert response.json()["message"] == "Slack mention queued" assert captured["repo_config_request"] == { - "text": "<@UBOT> continue on the branch", "channel_id": "C123", "thread_ts": "1700000000.000100", "slack_user_id": "U123", diff --git a/tests/test_slack_context.py b/tests/test_slack_context.py index ca60f97c..a077cc16 100644 --- a/tests/test_slack_context.py +++ b/tests/test_slack_context.py @@ -301,7 +301,7 @@ def test_select_slack_context_messages_detects_username_mention() -> None: assert [item["ts"] for item in selected] == ["1.0", "2.0", "3.0"] -def test_get_slack_repo_config_message_repo_overrides_existing_thread_repo( +def test_get_slack_repo_config_uses_existing_thread_repo( monkeypatch: pytest.MonkeyPatch, ) -> None: threads_client = _FakeThreadsClient( @@ -320,27 +320,27 @@ def test_get_slack_repo_config_message_repo_overrides_existing_thread_repo( webapp, "post_slack_thread_reply", fake_post_slack_thread_reply, raising=False ) - repo = asyncio.run( - webapp.get_slack_repo_config("please use repo:new-owner/new-repo", "C123", "1.234") - ) + repo = asyncio.run(webapp.get_slack_repo_config("C123", "1.234")) - assert repo == {"owner": "new-owner", "name": "new-repo"} - assert threads_client.requested_thread_id is None + assert repo == {"owner": "saved-owner", "name": "saved-repo"} + assert threads_client.requested_thread_id == generate_thread_id_from_slack_thread( + "C123", "1.234" + ) assert not posted -def test_get_slack_repo_config_parses_message_for_new_thread( +def test_get_slack_repo_config_new_thread_uses_default( monkeypatch: pytest.MonkeyPatch, ) -> None: threads_client = _FakeThreadsClient(raise_not_found=True) + monkeypatch.setattr(webapp, "SLACK_REPO_OWNER", "default-owner") + monkeypatch.setattr(webapp, "SLACK_REPO_NAME", "default-repo") monkeypatch.setattr(webapp, "get_client", lambda url: _FakeClient(threads_client)) - repo = asyncio.run( - webapp.get_slack_repo_config("please use repo:new-owner/new-repo", "C123", "1.234") - ) + repo = asyncio.run(webapp.get_slack_repo_config("C123", "1.234")) - assert repo == {"owner": "new-owner", "name": "new-repo"} + assert repo == {"owner": "default-owner", "name": "default-repo"} def test_get_slack_repo_config_existing_thread_without_repo_uses_default( @@ -352,7 +352,7 @@ def test_get_slack_repo_config_existing_thread_without_repo_uses_default( monkeypatch.setattr(webapp, "get_client", lambda url: _FakeClient(threads_client)) - repo = asyncio.run(webapp.get_slack_repo_config("please help", "C123", "1.234")) + repo = asyncio.run(webapp.get_slack_repo_config("C123", "1.234")) assert repo == {"owner": "default-owner", "name": "default-repo"} assert threads_client.requested_thread_id == generate_thread_id_from_slack_thread( @@ -360,127 +360,43 @@ def test_get_slack_repo_config_existing_thread_without_repo_uses_default( ) -def test_get_slack_repo_config_space_syntax_detected( +def test_get_slack_repo_config_ignores_repo_syntax_in_message( monkeypatch: pytest.MonkeyPatch, ) -> None: - """repo owner/name (space instead of colon) should be detected correctly.""" - threads_client = _FakeThreadsClient(raise_not_found=True) - - monkeypatch.setattr(webapp, "get_client", lambda url: _FakeClient(threads_client)) - - repo = asyncio.run( - webapp.get_slack_repo_config( - "please fix the bug in repo langchain-ai/langchainjs", "C123", "1.234" - ) - ) - - assert repo == {"owner": "langchain-ai", "name": "langchainjs"} - - -def test_get_slack_repo_config_github_url_extracted( - monkeypatch: pytest.MonkeyPatch, -) -> None: - """GitHub URL in message should be used to detect the repo.""" - threads_client = _FakeThreadsClient(raise_not_found=True) - - monkeypatch.setattr(webapp, "get_client", lambda url: _FakeClient(threads_client)) - - repo = asyncio.run( - webapp.get_slack_repo_config( - "I found a bug in https://github.com/langchain-ai/langgraph-api please fix it", - "C123", - "1.234", - ) - ) - - assert repo == {"owner": "langchain-ai", "name": "langgraph-api"} - - -def test_get_slack_repo_config_explicit_repo_beats_github_url( - monkeypatch: pytest.MonkeyPatch, -) -> None: - """Explicit repo: syntax takes priority over a GitHub URL also present in the message.""" - threads_client = _FakeThreadsClient(raise_not_found=True) - - monkeypatch.setattr(webapp, "get_client", lambda url: _FakeClient(threads_client)) - - repo = asyncio.run( - webapp.get_slack_repo_config( - "see https://github.com/langchain-ai/langgraph-api but use repo:my-org/my-repo", - "C123", - "1.234", - ) - ) - - assert repo == {"owner": "my-org", "name": "my-repo"} - - -def test_get_slack_repo_config_explicit_space_syntax_beats_thread_metadata( - monkeypatch: pytest.MonkeyPatch, -) -> None: - """Explicit repo owner/name (space syntax) takes priority over saved thread metadata.""" threads_client = _FakeThreadsClient( thread={"metadata": {"repo": {"owner": "saved-owner", "name": "saved-repo"}}} ) monkeypatch.setattr(webapp, "get_client", lambda url: _FakeClient(threads_client)) - repo = asyncio.run( - webapp.get_slack_repo_config( - "actually use repo langchain-ai/langchainjs today", "C123", "1.234" - ) - ) + repo = asyncio.run(webapp.get_slack_repo_config("C123", "1.234")) - assert repo == {"owner": "langchain-ai", "name": "langchainjs"} + assert repo == {"owner": "saved-owner", "name": "saved-repo"} -def test_get_slack_repo_config_github_url_beats_thread_metadata( +def test_get_slack_repo_config_applies_profile_default_repo( monkeypatch: pytest.MonkeyPatch, ) -> None: - """A GitHub URL in the message takes priority over saved thread metadata.""" - threads_client = _FakeThreadsClient( - thread={"metadata": {"repo": {"owner": "saved-owner", "name": "saved-repo"}}} - ) + threads_client = _FakeThreadsClient(thread={"metadata": {}}) + + async def fake_get_slack_user_info(user_id: str) -> dict: + return {"profile": {"email": "mason@example.com"}} + + def fake_resolve_login_from_email(email: str | None) -> str | None: + return "mason" + + async def fake_get_profile_default_repo(login: str | None) -> dict[str, str] | None: + assert login == "mason" + return {"owner": "profile-owner", "name": "profile-repo"} monkeypatch.setattr(webapp, "get_client", lambda url: _FakeClient(threads_client)) + monkeypatch.setattr(webapp, "get_slack_user_info", fake_get_slack_user_info) + monkeypatch.setattr(webapp, "resolve_login_from_email", fake_resolve_login_from_email) + monkeypatch.setattr(webapp, "get_profile_default_repo", fake_get_profile_default_repo) - repo = asyncio.run( - webapp.get_slack_repo_config( - "I found a bug in https://github.com/langchain-ai/langgraph-api", - "C123", - "1.234", - ) - ) + repo = asyncio.run(webapp.get_slack_repo_config("C123", "1.234", slack_user_id="U123")) - assert repo == {"owner": "langchain-ai", "name": "langgraph-api"} - - -def test_get_slack_repo_config_repo_name_only_defaults_org( - monkeypatch: pytest.MonkeyPatch, -) -> None: - """repo:name without org should default owner to langchain-ai.""" - threads_client = _FakeThreadsClient(raise_not_found=True) - - monkeypatch.setattr(webapp, "get_client", lambda url: _FakeClient(threads_client)) - - repo = asyncio.run( - webapp.get_slack_repo_config("fix bug in repo:langchainplus", "C123", "1.234") - ) - - assert repo == {"owner": "langchain-ai", "name": "langchainplus"} - - -def test_get_slack_repo_config_repo_name_only_space_syntax( - monkeypatch: pytest.MonkeyPatch, -) -> None: - """repo name (space syntax, no org) should default owner to langchain-ai.""" - threads_client = _FakeThreadsClient(raise_not_found=True) - - monkeypatch.setattr(webapp, "get_client", lambda url: _FakeClient(threads_client)) - - repo = asyncio.run(webapp.get_slack_repo_config("fix bug in repo open-swe", "C123", "1.234")) - - assert repo == {"owner": "langchain-ai", "name": "open-swe"} + assert repo == {"owner": "profile-owner", "name": "profile-repo"} def _setup_slack_mention_fakes( @@ -609,6 +525,11 @@ def test_process_slack_mention_creates_thread_first_run_with_trace_reply( assert "multitask_strategy" not in kwargs assert kwargs["config"]["configurable"]["slack_thread"]["thread_ts"] == thread_ts prompt_block = kwargs["input"]["messages"][0]["content"][0] + assert "## Default Repository Hint\nlangchain-ai/open-swe" in prompt_block["text"] + assert ( + "Use this only if the Slack conversation does not identify a different repository." + in (prompt_block["text"]) + ) assert prompt_block["text"].count("## Slack Thread") == 1 assert f"Thread TS: {thread_ts}" in prompt_block["text"] assert "## Latest Mention Request\ncontinue on the branch" in prompt_block["text"]