fix(connectors): validate credentials before persisting + populate Gmail URLs (#410)

src/openjarvis/connectors/gmail.py

@@ -497,6 +497,11 @@ class GmailConnector(BaseConnector):
                     timestamp=timestamp,
                     thread_id=thread_id,
                     channel=channel,
+                    # Deep-link straight to the message. ``msg_id`` is Gmail's
+                    # internal hex id, which the ``#all/<id>`` permalink
+                    # resolves directly — so citations have a working URL
+                    # without relying on _hit_url reconstruction at query time.
+                    url=f"https://mail.google.com/mail/u/0/#all/{msg_id}",
                     metadata={
                         "message_id": msg_id,
                         "rfc_message_id": rfc_message_id,

src/openjarvis/connectors/granola.py

@@ -76,6 +76,41 @@ def _granola_api_list_notes(
     return resp.json()
 
 
+class GranolaKeyError(ValueError):
+    """Raised when a Granola API key is missing or rejected by the API.
+
+    Surfaced through the ``/connect`` endpoint as an HTTP 400 so the user
+    sees why the key was refused instead of a silent failed sync later.
+    """
+
+
+def _granola_api_validate_key(api_key: str) -> None:
+    """Verify an API key with a minimal ``GET /v1/notes?limit=1`` probe.
+
+    Raises :class:`GranolaKeyError` when the key is empty or the API
+    responds 401/403, so an invalid key never overwrites a working
+    credential on disk. Other HTTP errors propagate via ``raise_for_status``.
+    """
+    if not api_key:
+        raise GranolaKeyError("Granola API key is empty.")
+    try:
+        resp = httpx.get(
+            f"{_GRANOLA_API_BASE}/v1/notes",
+            headers={"Authorization": f"Bearer {api_key}"},
+            params={"limit": 1},
+            timeout=30.0,
+        )
+    except httpx.HTTPError as exc:
+        raise GranolaKeyError(
+            f"Could not reach Granola to verify the key: {exc}"
+        ) from exc
+    if resp.status_code in (401, 403):
+        raise GranolaKeyError(
+            "Invalid API key. Check your key in Granola Settings → API."
+        )
+    resp.raise_for_status()
+
+
 def _granola_api_get_note(api_key: str, note_id: str) -> Dict[str, Any]:
     """Fetch a single Granola note by ID (includes transcript).
 
@@ -242,10 +277,15 @@ class GranolaConnector(BaseConnector):
         )
 
     def handle_callback(self, code: str) -> None:
-        """Persist the API key to the credentials file.
+        """Validate and persist the API key.
 
-        The *code* parameter holds the raw API key string provided by the user.
+        The *code* parameter holds the raw API key string provided by the
+        user. The key is verified with a live ``GET /v1/notes?limit=1``
+        probe *before* it is written, so an invalid key can never overwrite
+        a working credential on disk (raises :class:`GranolaKeyError` on a
+        401/403).
         """
+        _granola_api_validate_key(code)
         save_tokens(self._credentials_path, {"token": code})
 
     def sync(

src/openjarvis/connectors/slack_connector.py

@@ -205,17 +205,13 @@ def _validate_user_token(token: str) -> None:
         raise SlackTokenError("Slack token is empty.")
     if token.startswith(_BOT_TOKEN_PREFIX):
         raise SlackTokenError(
-            "Slack bot tokens (xoxb-) can't see user-to-user DMs. "
-            "Use a User OAuth Token (xoxp-) from "
-            "api.slack.com/apps → OAuth & Permissions, "
-            "with User Token Scopes channels:history, channels:read, "
-            "groups:history, groups:read, im:history, im:read, "
-            "mpim:history, mpim:read, users:read."
+            "Bot tokens (xoxb-) can't read DMs. "
+            "Use a User OAuth Token (xoxp-) instead."
         )
     if not token.startswith(_USER_TOKEN_PREFIX):
         raise SlackTokenError(
-            "Slack token must be a User OAuth Token (starts with 'xoxp-'). "
-            "Got a token with an unexpected prefix."
+            "Invalid token format. Expected a Slack User OAuth Token "
+            "starting with xoxp-"
         )
 
 
@@ -337,14 +333,35 @@ class SlackConnector(BaseConnector):
         return f"{_SLACK_AUTH_ENDPOINT}?{urlencode(params)}"
 
     def handle_callback(self, code: str) -> None:
-        """Persist the supplied User OAuth Token after validating its shape.
+        """Validate and persist a supplied User OAuth Token.
 
         The connector ``/connect`` endpoint funnels manually-pasted tokens
         through this method (the parameter is named ``code`` for OAuth-flow
-        compatibility). Bot tokens (``xoxb-``) are rejected here so the
-        invalid credential never lands on disk.
+        compatibility). The token is checked two ways before it is allowed
+        to touch disk, so an invalid credential never overwrites a working
+        one:
+
+        1. **Shape** — must start with ``xoxp-`` (``xoxb-`` bot tokens and
+           any other prefix are rejected via :func:`_validate_user_token`).
+        2. **Liveness** — a live ``auth.test`` call must return ``ok`` so an
+           expired or revoked token is caught at connect time.
         """
         _validate_user_token(code)
+
+        # Verify the token actually works against Slack before persisting.
+        try:
+            auth_resp = _slack_api_auth_test(code)
+        except Exception as exc:  # noqa: BLE001 — surface as a token error
+            raise SlackTokenError(
+                f"Could not verify the Slack token (auth.test failed: {exc})."
+            ) from exc
+        if not auth_resp.get("ok", False):
+            err = str(auth_resp.get("error", "auth_failed"))
+            raise SlackTokenError(
+                f"Slack rejected the token (auth.test: {err}). "
+                "Check that it is a current User OAuth Token."
+            )
+
         save_tokens(self._credentials_path, {"token": code})
         self._last_error = None
 

src/openjarvis/server/connectors_router.py

@@ -320,9 +320,20 @@ def create_connectors_router():
                 if req.code:
                     instance.handle_callback(req.code)
                 elif req.token:
-                    # Some OAuth connectors accept a pre-existing token.
+                    # A credential pasted into the ``token`` field. Connectors
+                    # that accept a pre-existing access token expose ``_token``
+                    # and set it directly (their real OAuth code-exchange runs
+                    # via /oauth/start → /oauth/callback). Connectors that
+                    # persist a manually-supplied credential — the Slack user
+                    # token and the Granola API key — validate inside
+                    # handle_callback, so route through it to guarantee the
+                    # credential is verified before anything touches disk. A
+                    # failed validation raises and is turned into HTTP 400
+                    # below, leaving any existing credential intact.
                     if hasattr(instance, "_token"):
                         instance._token = req.token
+                    else:
+                        instance.handle_callback(req.token)
 
             else:
                 # Generic: try to store token or credentials if the instance

tests/connectors/test_gmail.py

@@ -150,6 +150,9 @@ def test_sync_yields_documents(
     assert doc1.content == "Hello world"
     assert doc1.thread_id == "thread1"
     assert "alice@example.com" in doc1.participants
+    # Deep-link permalink to the message must be populated at ingest time
+    # (GH #408) — not left empty for _hit_url to reconstruct later.
+    assert doc1.url == "https://mail.google.com/mail/u/0/#all/msg1"
 
     # --- Message 2 ---
     doc2 = next(d for d in docs if d.doc_id == "gmail:msg2")
@@ -157,6 +160,7 @@ def test_sync_yields_documents(
     assert doc2.author == "bob@example.com"
     assert doc2.content == "Budget reply"
     assert doc2.thread_id == "thread2"
+    assert doc2.url == "https://mail.google.com/mail/u/0/#all/msg2"
 
     # Verify the API was called correctly
     mock_list.assert_called_once()

tests/connectors/test_granola.py

@@ -375,3 +375,88 @@ def test_end_to_end_ingest_and_search(
     # And the client-facing sources list does end up with the stored URL.
     client_sources = build_sources_for_client([target])
     assert client_sources[0]["url"] == _NOTE_1_WEB_URL
+
+
+# ---------------------------------------------------------------------------
+# Test — API-key validation happens BEFORE the key is written to disk, so an
+# invalid key is rejected at connect time and can never overwrite a working
+# credential (the data-loss bug this guards against, GH #409).
+# ---------------------------------------------------------------------------
+
+
+class _FakeResponse:
+    """Minimal httpx.Response stand-in for the validation probe."""
+
+    def __init__(self, status_code: int) -> None:
+        self.status_code = status_code
+
+    def raise_for_status(self) -> None:
+        if self.status_code >= 400:
+            import httpx  # noqa: PLC0415
+
+            raise httpx.HTTPStatusError(
+                "error", request=None, response=None  # type: ignore[arg-type]
+            )
+
+
+def test_validate_key_empty_raises() -> None:
+    """An empty key is rejected without any network call."""
+    from openjarvis.connectors.granola import (  # noqa: PLC0415
+        GranolaKeyError,
+        _granola_api_validate_key,
+    )
+
+    with pytest.raises(GranolaKeyError):
+        _granola_api_validate_key("")
+
+
+@pytest.mark.parametrize("status", [401, 403])
+def test_validate_key_rejects_unauthorized(status: int) -> None:
+    """A 401/403 from GET /v1/notes raises GranolaKeyError with guidance."""
+    from openjarvis.connectors.granola import (  # noqa: PLC0415
+        GranolaKeyError,
+        _granola_api_validate_key,
+    )
+
+    with patch(
+        "openjarvis.connectors.granola.httpx.get",
+        return_value=_FakeResponse(status),
+    ) as mock_get:
+        with pytest.raises(GranolaKeyError) as excinfo:
+            _granola_api_validate_key("grl_bad_key")
+
+    assert str(excinfo.value) == (
+        "Invalid API key. Check your key in Granola Settings → API."
+    )
+    # The probe must hit GET /v1/notes with limit=1 (cheap validation call).
+    _, kwargs = mock_get.call_args
+    assert kwargs["params"] == {"limit": 1}
+
+
+@patch("openjarvis.connectors.granola._granola_api_validate_key")
+def test_handle_callback_persists_after_validation(mock_validate, connector) -> None:
+    """A valid key is written only after the validation probe succeeds."""
+    connector.handle_callback("grl_good_key")
+
+    mock_validate.assert_called_once_with("grl_good_key")
+    stored = json.loads(Path(connector._credentials_path).read_text())
+    assert stored["token"] == "grl_good_key"
+
+
+def test_handle_callback_invalid_key_does_not_overwrite_existing(connector) -> None:
+    """A bad key must not clobber an existing, working credential on disk."""
+    from openjarvis.connectors.granola import GranolaKeyError  # noqa: PLC0415
+
+    creds_path = Path(connector._credentials_path)
+    creds_path.write_text(json.dumps({"token": "grl_real_existing_key"}))
+
+    with patch(
+        "openjarvis.connectors.granola.httpx.get",
+        return_value=_FakeResponse(401),
+    ):
+        with pytest.raises(GranolaKeyError):
+            connector.handle_callback("fake-key-12345")
+
+    # The pre-existing credential must be untouched.
+    stored = json.loads(creds_path.read_text())
+    assert stored["token"] == "grl_real_existing_key"

tests/connectors/test_slack_connector.py

@@ -560,3 +560,63 @@ def test_sync_logs_per_type_channel_counts(
     assert "1 private channels" in summary
     assert "2 DMs" in summary
     assert "1 group DMs" in summary
+
+
+# ---------------------------------------------------------------------------
+# Test — handle_callback verifies the token with a live auth.test BEFORE it
+# persists anything, so a syntactically-valid-but-dead token is rejected at
+# connect time instead of overwriting a working credential on disk.
+# ---------------------------------------------------------------------------
+
+
+@patch("openjarvis.connectors.slack_connector._slack_api_auth_test")
+def test_handle_callback_persists_after_auth_test_succeeds(
+    mock_auth, connector
+) -> None:
+    """A valid xoxp- token is persisted only after auth.test returns ok."""
+    mock_auth.return_value = _AUTH_TEST_RESPONSE
+
+    connector.handle_callback("xoxp-valid-user-token")
+
+    mock_auth.assert_called_once_with("xoxp-valid-user-token")
+    stored = json.loads(Path(connector._credentials_path).read_text())
+    assert stored["token"] == "xoxp-valid-user-token"
+
+
+@patch("openjarvis.connectors.slack_connector._slack_api_auth_test")
+def test_handle_callback_rejects_when_auth_test_fails(mock_auth, connector) -> None:
+    """A well-formed token Slack rejects (auth.test not ok) is never written."""
+    from openjarvis.connectors.slack_connector import SlackTokenError  # noqa: PLC0415
+
+    mock_auth.return_value = {"ok": False, "error": "invalid_auth"}
+
+    with pytest.raises(SlackTokenError) as excinfo:
+        connector.handle_callback("xoxp-revoked-token")
+
+    assert "invalid_auth" in str(excinfo.value)
+    assert not Path(connector._credentials_path).exists()
+
+
+@patch("openjarvis.connectors.slack_connector._slack_api_auth_test")
+def test_handle_callback_skips_auth_test_for_bad_shape(mock_auth, connector) -> None:
+    """Shape validation short-circuits before any network call is made."""
+    from openjarvis.connectors.slack_connector import SlackTokenError  # noqa: PLC0415
+
+    with pytest.raises(SlackTokenError):
+        connector.handle_callback("xoxb-bot-token")
+
+    mock_auth.assert_not_called()
+    assert not Path(connector._credentials_path).exists()
+
+
+def test_handle_callback_xoxb_message_wording(connector) -> None:
+    """The xoxb- rejection carries the user-facing 'can't read DMs' guidance."""
+    from openjarvis.connectors.slack_connector import SlackTokenError  # noqa: PLC0415
+
+    with pytest.raises(SlackTokenError) as excinfo:
+        connector.handle_callback("xoxb-bot-token")
+
+    assert str(excinfo.value) == (
+        "Bot tokens (xoxb-) can't read DMs. "
+        "Use a User OAuth Token (xoxp-) instead."
+    )

tests/server/test_connectors_router.py

@@ -102,3 +102,60 @@ def test_trigger_sync(app, tmp_path: Path) -> None:
     data = resp.json()
     assert data["connector_id"] == "obsidian"
     assert data["status"] in {"started", "already_syncing"}
+
+
+# ---------------------------------------------------------------------------
+# Connect-time credential validation (GH #409): the /connect endpoint must
+# reject invalid credentials with HTTP 400 and never persist (or overwrite)
+# anything on disk when validation fails.
+# ---------------------------------------------------------------------------
+
+
+def test_connect_slack_bot_token_returns_400(app, tmp_path: Path) -> None:
+    """POST connect with an xoxb- token is rejected 400 and writes nothing."""
+    from openjarvis.connectors.slack_connector import SlackConnector
+    from openjarvis.server.connectors_router import _instances
+
+    creds = tmp_path / "slack.json"
+    _instances["slack"] = SlackConnector(credentials_path=str(creds))
+    try:
+        resp = app.post(
+            "/v1/connectors/slack/connect", json={"token": "xoxb-fake-token"}
+        )
+        assert resp.status_code == 400
+        assert "xoxb" in resp.json()["detail"].lower()
+        assert not creds.exists()
+    finally:
+        _instances.pop("slack", None)
+
+
+def test_connect_granola_invalid_key_returns_400_keeps_existing(
+    app, tmp_path: Path
+) -> None:
+    """A bad Granola key is rejected 400 and the existing credential survives."""
+    import json
+    from unittest.mock import patch
+
+    from openjarvis.connectors.granola import GranolaConnector, GranolaKeyError
+    from openjarvis.server.connectors_router import _instances
+
+    creds = tmp_path / "granola.json"
+    creds.write_text(json.dumps({"token": "grl_real_existing_key"}))
+    _instances["granola"] = GranolaConnector(credentials_path=str(creds))
+    try:
+        with patch(
+            "openjarvis.connectors.granola._granola_api_validate_key",
+            side_effect=GranolaKeyError(
+                "Invalid API key. Check your key in Granola Settings → API."
+            ),
+        ):
+            resp = app.post(
+                "/v1/connectors/granola/connect",
+                json={"code": "fake-key-12345"},
+            )
+        assert resp.status_code == 400
+        assert "Invalid API key" in resp.json()["detail"]
+        # The previously-working credential must be untouched.
+        assert json.loads(creds.read_text())["token"] == "grl_real_existing_key"
+    finally:
+        _instances.pop("granola", None)