From d57f657a32e75ad089bf8c7c4b1473dfe2aa992e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Agust=C3=ADn=20M=C3=A9ndez?= Date: Sun, 13 Sep 2026 20:27:44 -0500 Subject: [PATCH 1/2] fix(auth): write token file atomically at 0600; add coverage gate - pyproject: fail_under = 90 in [tool.coverage.report] so coverage cannot regress silently (currently 95.26%; Codecov runs with fail_ci_if_error: false). - save_oauth_token now creates ~/.quickup/auth.json through a 0600 temp file and publishes it with os.replace, removing the window where the OAuth token sat on disk with default-umask permissions (e.g. 0644) before the chmod. - tests: assert 0600 holds under umask 000, and that no temp file is left behind. --- pyproject.toml | 2 ++ quickup/cli/auth.py | 14 ++++++++++++-- tests/test_auth.py | 26 ++++++++++++++++++++++++++ 3 files changed, 40 insertions(+), 2 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 2452f34..45f6ac4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -143,6 +143,8 @@ source = ["quickup"] omit = ["*/tests/*", "*/__pycache__/*"] [tool.coverage.report] +# Hard gate: coverage below 90% fails the test run (CI included). +fail_under = 90 exclude_lines = [ "pragma: no cover", "if __name__ == .__main__.:", diff --git a/quickup/cli/auth.py b/quickup/cli/auth.py index f89d118..88cf98f 100644 --- a/quickup/cli/auth.py +++ b/quickup/cli/auth.py @@ -50,8 +50,18 @@ def save_oauth_token(token: str, user_info: dict | None = None) -> None: if user_info: data["user"] = user_info - AUTH_FILE.write_text(json.dumps(data, indent=2)) - os.chmod(AUTH_FILE, 0o600) + # Write via a 0600 temp file and publish atomically: the token is never + # world/group-readable, not even for the instant between create and chmod. + tmp_file = AUTH_FILE.with_suffix(AUTH_FILE.suffix + ".tmp") + fd = os.open(tmp_file, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600) + try: + with os.fdopen(fd, "w", encoding="utf-8") as fh: + json.dump(data, fh, indent=2) + os.chmod(tmp_file, 0o600) # O_CREAT mode is filtered by umask + os.replace(tmp_file, AUTH_FILE) + except BaseException: + tmp_file.unlink(missing_ok=True) + raise def delete_oauth_token() -> bool: diff --git a/tests/test_auth.py b/tests/test_auth.py index 0c95a83..ad08221 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -2,6 +2,7 @@ from io import BytesIO import json +import os import sys from typing import cast from unittest.mock import Mock, patch @@ -81,6 +82,31 @@ def test_save_token_file_permissions(self, tmp_path, monkeypatch): stat = auth_file.stat() assert stat.st_mode & 0o777 == 0o600 + @pytest.mark.skipif(sys.platform == "win32", reason="Windows does not support Unix file permissions") + def test_save_token_ignores_permissive_umask(self, tmp_path, monkeypatch): + auth_file = tmp_path / "auth.json" + monkeypatch.setattr("quickup.cli.auth.AUTH_FILE", auth_file) + monkeypatch.setattr("quickup.cli.auth.AUTH_DIR", tmp_path) + + previous_umask = os.umask(0o000) + try: + save_oauth_token("secret-token") + finally: + os.umask(previous_umask) + + assert auth_file.stat().st_mode & 0o777 == 0o600 + + def test_save_token_leaves_no_temp_file(self, tmp_path, monkeypatch): + auth_file = tmp_path / "auth.json" + monkeypatch.setattr("quickup.cli.auth.AUTH_FILE", auth_file) + monkeypatch.setattr("quickup.cli.auth.AUTH_DIR", tmp_path) + + save_oauth_token("token-1") + save_oauth_token("token-2") + + assert sorted(p.name for p in tmp_path.iterdir()) == ["auth.json"] + assert json.loads(auth_file.read_text())["access_token"] == "token-2" + class TestOAuthConfig: """Tests for OAuth client config resolution.""" From 6bdbcde13f36fc9ec065d6cb932f6fa37d38f42d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Agust=C3=ADn=20M=C3=A9ndez?= Date: Sun, 13 Sep 2026 20:44:48 -0500 Subject: [PATCH 2/2] fix(auth)!: require OAuth app credentials from env instead of bundling them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ClickUp OAuth client ID and secret were committed in plaintext at quickup/cli/auth.py:13-14, exposing a shared app credential to anyone with read access to the repo (and permanently to git history). They are now gone: get_oauth_config() resolves QUICKUP_CLIENT_ID / QUICKUP_CLIENT_SECRET from the environment or a local .env file and raises the new OAuthConfigError (exit code 6, with a setup hint) when either is missing or blank. - quickup login fails fast on that error before opening a browser tab, and no longer flattens ClickupyError into OAuthError, so the hint survives. - Docs updated: README, commands.rst, features.rst, Installation.rst, and .env.example now document creating your own app (redirect URI http://localhost:4242) and exporting the two variables. - Tests: 235 pass (was 226) covering missing/partial/blank credentials, .env loading, real env winning over .env, and browser-not-opened fail-fast. Coverage 95.05%; ruff, black and isort clean. BREAKING CHANGE: `quickup login` no longer works out of the box. Create a ClickUp OAuth app and set QUICKUP_CLIENT_ID and QUICKUP_CLIENT_SECRET first. NOTE: the leaked pair must still be revoked/rotated in ClickUp — deleting it from HEAD does not remove it from git history. --- .env.example | 6 +++ README.md | 10 ++++- docs/source/Installation.rst | 9 +++++ docs/source/commands.rst | 26 ++++++++++-- docs/source/features.rst | 11 +++++ quickup/cli/auth.py | 34 ++++++++++++---- quickup/cli/exceptions.py | 14 +++++++ quickup/cli/main.py | 8 +++- tests/test_auth.py | 78 +++++++++++++++++++++++++++++++----- tests/test_exceptions.py | 24 +++++++++++ 10 files changed, 197 insertions(+), 23 deletions(-) diff --git a/.env.example b/.env.example index bbf7682..771931d 100644 --- a/.env.example +++ b/.env.example @@ -1 +1,7 @@ TOKEN=pk_XXXXXXXX_YYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYY + +# ClickUp OAuth app credentials — required by `quickup login`. +# Create an app at https://app.clickup.com/settings/apps with the redirect URI +# http://localhost:4242, then paste its client ID and secret below. +QUICKUP_CLIENT_ID= +QUICKUP_CLIENT_SECRET= diff --git a/README.md b/README.md index 5bd5447..e11f9ec 100644 --- a/README.md +++ b/README.md @@ -33,9 +33,13 @@ pip install quickup ## Quick Start -Authenticate with ClickUp (recommended): +Authenticate with ClickUp (recommended). QuickUp! ships without bundled credentials, so +first point it at your own ClickUp OAuth app — create one at + with redirect URI `http://localhost:4242`: ```bash +export QUICKUP_CLIENT_ID=your_client_id +export QUICKUP_CLIENT_SECRET=your_client_secret quickup login ``` @@ -69,7 +73,9 @@ Authenticate with ClickUp via OAuth. Opens your default browser and waits for th quickup login ``` -Credentials are saved to `~/.quickup/auth.json` (permissions: `0o600`). +Requires `QUICKUP_CLIENT_ID` and `QUICKUP_CLIENT_SECRET` (environment or `.env`) pointing at +your own ClickUp OAuth app; without them the command exits with code `6` and prints a setup +hint. Credentials are saved to `~/.quickup/auth.json` (permissions: `0o600`). ### `quickup logout` - Remove Stored Credentials diff --git a/docs/source/Installation.rst b/docs/source/Installation.rst index 1c6e880..ba9de84 100644 --- a/docs/source/Installation.rst +++ b/docs/source/Installation.rst @@ -37,6 +37,15 @@ After installation, authenticate with ClickUp using one of the methods below. Option 1: OAuth Login (Recommended) ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +First, point QuickUp! at your own ClickUp OAuth app — no credentials ship with the +package. Create an app at https://app.clickup.com/settings/apps with the redirect URI +``http://localhost:4242``, then export its credentials (or add them to a ``.env`` file): + +.. code-block:: bash + + export QUICKUP_CLIENT_ID=your_client_id + export QUICKUP_CLIENT_SECRET=your_client_secret + Run the login command to authenticate via your browser: .. code-block:: bash diff --git a/docs/source/commands.rst b/docs/source/commands.rst index f076b1f..cea8384 100644 --- a/docs/source/commands.rst +++ b/docs/source/commands.rst @@ -23,16 +23,36 @@ Opens the ClickUp authorization page in your browser and starts a local HTTP ser the token is exchanged and saved securely to ``~/.quickup/auth.json`` (permissions: ``0o600``). The callback times out after 120 seconds if not completed. -To use a custom OAuth application, set ``QUICKUP_CLIENT_ID`` and ``QUICKUP_CLIENT_SECRET`` -environment variables before running ``quickup login``. +Prerequisites +~~~~~~~~~~~~~ + +QuickUp! ships without any bundled credentials, so you must point it at your own ClickUp +OAuth app. Create one at https://app.clickup.com/settings/apps with the redirect URI +``http://localhost:4242``, then export its credentials (or put them in a ``.env`` file): + +.. code-block:: bash + + export QUICKUP_CLIENT_ID=your_client_id + export QUICKUP_CLIENT_SECRET=your_client_secret + +If either variable is missing, ``quickup login`` exits with code ``6`` and prints the setup +hint instead of opening a browser. Examples ~~~~~~~~ -Log in with the default QuickUp! OAuth application: +Log in using the credentials exported above: + +.. code-block:: bash + + quickup login + +Or read them from a ``.env`` file in the current directory: .. code-block:: bash + echo 'QUICKUP_CLIENT_ID=your_client_id' >> .env + echo 'QUICKUP_CLIENT_SECRET=your_client_secret' >> .env quickup login ``quickup logout`` - Remove Stored Credentials diff --git a/docs/source/features.rst b/docs/source/features.rst index 6d22a47..562ca13 100644 --- a/docs/source/features.rst +++ b/docs/source/features.rst @@ -15,6 +15,17 @@ QuickUp! supports two authentication modes: quickup login # opens browser, saves token to ~/.quickup/auth.json quickup logout # removes the stored token +OAuth login uses your own ClickUp OAuth app — no credentials are bundled with QuickUp!. +Create an app at https://app.clickup.com/settings/apps with redirect URI +``http://localhost:4242`` and export its credentials first: + +.. code-block:: bash + + export QUICKUP_CLIENT_ID=your_client_id + export QUICKUP_CLIENT_SECRET=your_client_secret + +Both values may also live in a ``.env`` file in the current directory. + **API Token** — set a personal token via environment variable or ``.env`` file: .. code-block:: bash diff --git a/quickup/cli/auth.py b/quickup/cli/auth.py index 88cf98f..78d5075 100644 --- a/quickup/cli/auth.py +++ b/quickup/cli/auth.py @@ -9,9 +9,9 @@ from urllib.request import Request, urlopen import webbrowser -# Default OAuth app credentials — override via QUICKUP_CLIENT_ID / QUICKUP_CLIENT_SECRET env vars -_DEFAULT_CLIENT_ID = "G0F2EFTGBIKJD3YY3EOWGMPZZ4ENRYWK" -_DEFAULT_CLIENT_SECRET = "4K8KUVGU9CFQZ83TSGABMJM30KJ3BE5L8H8HAAPI6OZOPBJ54JE05DJL91VR575A" +import dotenv + +from .exceptions import OAuthConfigError AUTH_DIR = Path.home() / ".quickup" AUTH_FILE = AUTH_DIR / "auth.json" @@ -26,10 +26,29 @@ def get_oauth_config() -> tuple[str, str]: - """Return (client_id, client_secret) from env vars or defaults.""" - client_id = os.environ.get("QUICKUP_CLIENT_ID", _DEFAULT_CLIENT_ID) - client_secret = os.environ.get("QUICKUP_CLIENT_SECRET", _DEFAULT_CLIENT_SECRET) - return client_id, client_secret + """Return the (client_id, client_secret) of the ClickUp OAuth app to use. + + Credentials are never bundled with the package: they come from + ``QUICKUP_CLIENT_ID`` / ``QUICKUP_CLIENT_SECRET`` in the environment or in a + local ``.env`` file. Register your own app at + https://app.clickup.com/settings/apps with redirect URI + ``http://localhost:4242``. + + Raises: + OAuthConfigError: if either credential is missing or blank. + """ + # Pick up a local .env without overriding variables already in the environment. + dotenv.load_dotenv(".env") + + credentials = { + "QUICKUP_CLIENT_ID": os.environ.get("QUICKUP_CLIENT_ID", "").strip(), + "QUICKUP_CLIENT_SECRET": os.environ.get("QUICKUP_CLIENT_SECRET", "").strip(), + } + missing = [name for name, value in credentials.items() if not value] + if missing: + raise OAuthConfigError(missing) + + return credentials["QUICKUP_CLIENT_ID"], credentials["QUICKUP_CLIENT_SECRET"] def load_oauth_token() -> str | None: @@ -178,6 +197,7 @@ def perform_oauth_login() -> tuple[str, dict]: Tuple of (access_token, user_info dict). Raises: + OAuthConfigError: If the OAuth app credentials are not configured. RuntimeError: If the OAuth flow fails at any step. """ client_id, client_secret = get_oauth_config() diff --git a/quickup/cli/exceptions.py b/quickup/cli/exceptions.py index 014ff3d..49a34bc 100644 --- a/quickup/cli/exceptions.py +++ b/quickup/cli/exceptions.py @@ -127,6 +127,20 @@ def __init__(self, message: str = "OAuth authentication failed."): super().__init__(message, "Try running 'quickup login' again.") +class OAuthConfigError(ClickupyError): + """Raised when the ClickUp OAuth app credentials are not configured.""" + + exit_code = 6 + + def __init__(self, missing: list[str]): + super().__init__( + f"Missing OAuth app credential{'s' if len(missing) > 1 else ''}: {', '.join(missing)}.", + "Create an OAuth app at https://app.clickup.com/settings/apps (redirect URI " + "http://localhost:4242), then set QUICKUP_CLIENT_ID and QUICKUP_CLIENT_SECRET " + "in your environment or in a .env file.", + ) + + class NetworkError(ClickupyError): """Raised for HTTP/connection failures.""" diff --git a/quickup/cli/main.py b/quickup/cli/main.py index 7817e4a..d264b49 100644 --- a/quickup/cli/main.py +++ b/quickup/cli/main.py @@ -8,7 +8,7 @@ import requests from .api_client import get_current_sprint_list, get_list_for, get_project_for, get_space_for, get_team -from .auth import delete_oauth_token, perform_oauth_login, save_oauth_token +from .auth import delete_oauth_token, get_oauth_config, perform_oauth_login, save_oauth_token from .cache import get_task_data, maybe_warmup from .config import init_environ from .exceptions import ClickupyError, OAuthError, TokenError, handle_exception @@ -330,6 +330,9 @@ def comment_task( @app.command def login() -> None: """Authenticate with ClickUp via OAuth2 browser login.""" + # Fail fast with a setup hint if the OAuth app credentials are not configured. + get_oauth_config() + print("Opening browser for ClickUp authentication...") try: access_token, user_info = perform_oauth_login() @@ -337,6 +340,9 @@ def login() -> None: username = user_info.get("username", "unknown") email = user_info.get("email", "") print(f"Successfully logged in as {username} ({email})") + except ClickupyError: + # Keep the specific message and hint (e.g. OAuthConfigError) intact. + raise except Exception as e: raise OAuthError(str(e)) from e diff --git a/tests/test_auth.py b/tests/test_auth.py index ad08221..97dc22e 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -20,6 +20,7 @@ save_oauth_token, ) from quickup.cli.config import init_environ +from quickup.cli.exceptions import OAuthConfigError class TestTokenStorage: @@ -111,18 +112,55 @@ def test_save_token_leaves_no_temp_file(self, tmp_path, monkeypatch): class TestOAuthConfig: """Tests for OAuth client config resolution.""" - def test_default_config(self): - config = get_oauth_config() - assert len(config) == 2 - assert isinstance(config[0], str) - assert isinstance(config[1], str) - - def test_env_var_override(self, monkeypatch): + @pytest.fixture(autouse=True) + def _isolate_credentials(self, tmp_path, monkeypatch): + """Start from a clean slate: no inherited env vars, no repo .env file.""" + monkeypatch.delenv("QUICKUP_CLIENT_ID", raising=False) + monkeypatch.delenv("QUICKUP_CLIENT_SECRET", raising=False) + monkeypatch.chdir(tmp_path) + + def test_no_bundled_defaults(self): + """Credentials must come from the environment, never from the package.""" + with pytest.raises(OAuthConfigError) as exc_info: + get_oauth_config() + assert exc_info.value.message == "Missing OAuth app credentials: QUICKUP_CLIENT_ID, QUICKUP_CLIENT_SECRET." + assert exc_info.value.exit_code == 6 + + def test_partial_credentials_raise(self, monkeypatch): + """A client ID without its secret is not enough to log in.""" + monkeypatch.setenv("QUICKUP_CLIENT_ID", "my-id") + with pytest.raises(OAuthConfigError) as exc_info: + get_oauth_config() + assert exc_info.value.message == "Missing OAuth app credential: QUICKUP_CLIENT_SECRET." + + def test_blank_credentials_raise(self, monkeypatch): + """Empty or whitespace-only values count as missing.""" + monkeypatch.setenv("QUICKUP_CLIENT_ID", " ") + monkeypatch.setenv("QUICKUP_CLIENT_SECRET", "") + with pytest.raises(OAuthConfigError): + get_oauth_config() + + def test_env_var_credentials(self, monkeypatch): monkeypatch.setenv("QUICKUP_CLIENT_ID", "my-id") monkeypatch.setenv("QUICKUP_CLIENT_SECRET", "my-secret") - client_id, client_secret = get_oauth_config() - assert client_id == "my-id" - assert client_secret == "my-secret" + assert get_oauth_config() == ("my-id", "my-secret") + + def test_env_var_credentials_are_trimmed(self, monkeypatch): + monkeypatch.setenv("QUICKUP_CLIENT_ID", " my-id ") + monkeypatch.setenv("QUICKUP_CLIENT_SECRET", " my-secret\n") + assert get_oauth_config() == ("my-id", "my-secret") + + def test_reads_dotenv_file(self, tmp_path): + """A .env in the working directory supplies the credentials.""" + (tmp_path / ".env").write_text("QUICKUP_CLIENT_ID=dotenv-id\nQUICKUP_CLIENT_SECRET=dotenv-secret\n") + assert get_oauth_config() == ("dotenv-id", "dotenv-secret") + + def test_real_environment_wins_over_dotenv(self, tmp_path, monkeypatch): + """load_dotenv must not clobber variables already set in the environment.""" + (tmp_path / ".env").write_text("QUICKUP_CLIENT_ID=dotenv-id\nQUICKUP_CLIENT_SECRET=dotenv-secret\n") + monkeypatch.setenv("QUICKUP_CLIENT_ID", "env-id") + monkeypatch.setenv("QUICKUP_CLIENT_SECRET", "env-secret") + assert get_oauth_config() == ("env-id", "env-secret") class TestCallbackHandler: @@ -220,6 +258,26 @@ def test_successful_fetch(self, mock_urlopen): class TestPerformOAuthLogin: """Tests for the full OAuth login flow.""" + @pytest.fixture(autouse=True) + def _oauth_credentials(self, tmp_path, monkeypatch): + """Provide configured credentials so these tests focus on the flow itself.""" + monkeypatch.chdir(tmp_path) + monkeypatch.setenv("QUICKUP_CLIENT_ID", "test-client-id") + monkeypatch.setenv("QUICKUP_CLIENT_SECRET", "test-client-secret") + + @patch("quickup.cli.auth.webbrowser.open") + @patch("quickup.cli.auth.HTTPServer") + def test_missing_credentials_fail_before_opening_browser(self, mock_server_cls, mock_browser, monkeypatch): + """Fail fast with a setup hint instead of opening a doomed browser tab.""" + monkeypatch.delenv("QUICKUP_CLIENT_ID") + monkeypatch.delenv("QUICKUP_CLIENT_SECRET") + + with pytest.raises(OAuthConfigError): + perform_oauth_login() + + mock_browser.assert_not_called() + mock_server_cls.assert_not_called() + @patch("quickup.cli.auth._fetch_user_info") @patch("quickup.cli.auth._exchange_code_for_token") @patch("quickup.cli.auth.webbrowser.open") diff --git a/tests/test_exceptions.py b/tests/test_exceptions.py index 41ab675..b50efec 100644 --- a/tests/test_exceptions.py +++ b/tests/test_exceptions.py @@ -7,6 +7,7 @@ ClickupyError, ListNotFoundError, NetworkError, + OAuthConfigError, ProjectNotFoundError, SpaceNotFoundError, TeamAmbiguousError, @@ -114,6 +115,29 @@ def test_with_list_id(self): assert "List 'list-000' not found" in exc.message +class TestOAuthConfigError: + """Tests for OAuthConfigError.""" + + def test_single_missing_credential(self): + """Test message when only one credential is missing.""" + exc = OAuthConfigError(["QUICKUP_CLIENT_SECRET"]) + assert exc.message == "Missing OAuth app credential: QUICKUP_CLIENT_SECRET." + assert "QUICKUP_CLIENT_ID" not in exc.message + assert exc.exit_code == 6 + + def test_multiple_missing_credentials(self): + """Test message when both credentials are missing.""" + exc = OAuthConfigError(["QUICKUP_CLIENT_ID", "QUICKUP_CLIENT_SECRET"]) + assert exc.message == "Missing OAuth app credentials: QUICKUP_CLIENT_ID, QUICKUP_CLIENT_SECRET." + + def test_hint_explains_setup(self): + """Test the hint tells the user how to configure the OAuth app.""" + exc = OAuthConfigError(["QUICKUP_CLIENT_ID"]) + assert "app.clickup.com/settings/apps" in str(exc) + assert "http://localhost:4242" in str(exc) + assert "QUICKUP_CLIENT_SECRET" in str(exc) + + class TestNetworkError: """Tests for NetworkError."""