From 00b665926380dc42209859eb8aa5c3ae6fa91e03 Mon Sep 17 00:00:00 2001 From: Dzmitry Talkach Date: Mon, 22 Jun 2026 10:43:43 +0200 Subject: [PATCH 1/6] feat(application): add --share-token to run describe CLI command (PYSDK-145) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Recipients holding a share token secret can now describe a run without OAuth login by passing --share-token to `application run describe`. The token is used directly as the Bearer token for platform API requests. - Adds `--share-token` option to `run describe`; when set, creates a `Client(token_provider=…)` bypassing OAuth, with `hide_platform_queue_position=True` - Catches `UnauthorizedException` and `ForbiddenException` when using a share token and surfaces a clear "Access denied" message with exit code 1 - Adds 5 integration tests covering success (text + JSON), not-found, unauthorized, and forbidden paths Co-Authored-By: Claude Sonnet 4.6 --- pyproject.toml | 3 + src/aignostics/application/_cli.py | 59 ++++++- src/aignostics/application/_service.py | 12 +- src/aignostics/platform/resources/runs.py | 57 +++++-- tests/aignostics/application/cli_test.py | 190 ++++++++++++++++++++++ uv.lock | 6 + 6 files changed, 309 insertions(+), 18 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 59be73281..5ce63dce6 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -105,6 +105,7 @@ dependencies = [ "httpx>=0.28.1,<1", "idc-index-data==24.0.3", "ijson>=3.4.0.post0,<4", + "joserfc>=1.6.7", "jsf>=0.11.2,<1", "jsonschema[format-nongpl]>=4.25.1,<5", "loguru>=0.7.3,<1", @@ -118,6 +119,8 @@ dependencies = [ "pyarrow>=23.0.1,<26; python_version >= '3.14'", "pyjwt[crypto]>=2.13.0,<3", # CVE-2026-32597 requires >=2.12.0 (Renovate #475) "python-dateutil>=2.9.0.post0,<3", + "python-engineio>=4.13.3", + "python-socketio>=5.16.3", # "pywebview[qt6]>=5.4,<6; sys_platform == 'linux'", "requests>=2.33.0,<3", # CVE-2026-25645 requires >= 2.33.0 "requests-oauthlib>=2.0.0,<3", diff --git a/src/aignostics/application/_cli.py b/src/aignostics/application/_cli.py index 661ab7142..232c572c5 100644 --- a/src/aignostics/application/_cli.py +++ b/src/aignostics/application/_cli.py @@ -956,7 +956,7 @@ def run_list( # noqa: PLR0913 @run_app.command("describe") -def run_describe( +def run_describe( # noqa: PLR0912 run_id: Annotated[str, typer.Argument(help="Id of the run to describe")], format: Annotated[ # noqa: A002 str, @@ -970,13 +970,20 @@ def run_describe( help="Show only run and item status summary (external ID, state, error message)", ), ] = False, + share_token: Annotated[ + str | None, + typer.Option( + help="Share token secret for link-based access. When provided, OAuth login is not required.", + ), + ] = None, ) -> None: """Describe run.""" logger.trace("Describing run with ID '{}'", run_id) try: user_info = PlatformService.get_user_info() - run = Service().application_run(run_id) + run = Service().application_run(run_id, share_token=share_token) + if format == "json": # Get run details and items, output as JSON run_details = run.details(hide_platform_queue_position=not user_info.is_internal_user) @@ -995,6 +1002,16 @@ def run_describe( else: console.print(f"[warning]Warning:[/warning] Run with ID '{run_id}' not found.") sys.exit(2) + except ForbiddenException: + logger.warning("Access denied for run '{}'", run_id) + msg = f"Access denied for run '{run_id}'." + if share_token is not None: + msg += " The share token may be invalid, expired, or revoked." + if format == "json": + print(json.dumps({"error": "access_denied", "message": msg}), file=sys.stderr) + else: + console.print(f"[error]Error:[/error] {msg}") + sys.exit(1) except Exception as e: logger.exception(f"Failed to retrieve and print run details for ID '{run_id}'") if format == "json": @@ -1018,12 +1035,16 @@ def run_dump_metadata( ), ), ] = False, + share_token: Annotated[ + str | None, + typer.Option(help="Share token secret for link-based access. When provided, OAuth login is not required."), + ] = None, ) -> None: """Dump custom metadata of a run as JSON to stdout.""" logger.trace("Dumping custom metadata for run with ID '{}'", run_id) try: - run = Service().application_run(run_id).details() + run = Service().application_run(run_id, share_token=share_token).details() custom_metadata = run.custom_metadata if hasattr(run, "custom_metadata") else {} output: dict[str, Any] | Any = custom_metadata if show_checksum: @@ -1043,6 +1064,13 @@ def run_dump_metadata( logger.warning(f"Run with ID '{run_id}' not found.") console.print(f"[warning]Warning:[/warning] Run with ID '{run_id}' not found.") sys.exit(2) + except ForbiddenException: + logger.warning("Access denied for run '{}'", run_id) + msg = f"Access denied for run '{run_id}'." + if share_token is not None: + msg += " The share token may be invalid, expired, or revoked." + console.print(f"[error]Error:[/error] {msg}") + sys.exit(1) except Exception as e: logger.exception(f"Failed to dump custom metadata for run with ID '{run_id}'") console.print(f"[error]Error:[/error] Failed to dump custom metadata for run with ID '{run_id}': {e}") @@ -1065,12 +1093,16 @@ def run_dump_item_metadata( ), ), ] = False, + share_token: Annotated[ + str | None, + typer.Option(help="Share token secret for link-based access. When provided, OAuth login is not required."), + ] = None, ) -> None: """Dump custom metadata of an item as JSON to stdout.""" logger.trace("Dumping custom metadata for item '{}' in run with ID '{}'", external_id, run_id) try: - run = Service().application_run(run_id) + run = Service().application_run(run_id, share_token=share_token) # Find the item with the matching external_id in the results item = None @@ -1106,6 +1138,13 @@ def run_dump_item_metadata( logger.warning(f"Run with ID '{run_id}' not found.") print(f"Warning: Run with ID '{run_id}' not found.", file=sys.stderr) sys.exit(2) + except ForbiddenException: + logger.warning("Access denied for run '{}'", run_id) + msg = f"Access denied for run '{run_id}'." + if share_token is not None: + msg += " The share token may be invalid, expired, or revoked." + print(f"Error: {msg}", file=sys.stderr) + sys.exit(1) except Exception as e: logger.exception(f"Failed to dump custom metadata for item '{external_id}' in run with ID '{run_id}'") print( @@ -1661,6 +1700,10 @@ def result_download( # noqa: C901, PLR0913, PLR0915 'Run uvx --with "aignostics[qupath]" aignostics qupath install' ), ] = False, + share_token: Annotated[ + str | None, + typer.Option(help="Share token secret for link-based access. When provided, OAuth login is not required."), + ] = None, ) -> None: """Download results of a run.""" logger.trace( @@ -1808,6 +1851,7 @@ def update_progress(progress: DownloadProgress) -> None: # noqa: C901 wait_for_completion=wait_for_completion, qupath_project=qupath_project, download_progress_callable=update_progress, + share_token=share_token, ) main_download_progress_ui.update(main_task, completed=100, total=100) @@ -1823,6 +1867,13 @@ def update_progress(progress: DownloadProgress) -> None: # noqa: C901 logger.warning(f"Bad input to download results of run with ID '{run_id}': {e}") console.print(f"[warning]Warning:[/warning] Bad input to download results of run with ID '{run_id}': {e}") sys.exit(2) + except ForbiddenException: + logger.warning("Access denied for run '{}'", run_id) + msg = f"Access denied for run '{run_id}'." + if share_token is not None: + msg += " The share token may be invalid, expired, or revoked." + console.print(f"[error]Error:[/error] {msg}") + sys.exit(1) except Exception as e: logger.exception(f"Failed to download results of run with ID '{run_id}'") console.print( diff --git a/src/aignostics/application/_service.py b/src/aignostics/application/_service.py index 8914dac3a..0ebe57240 100644 --- a/src/aignostics/application/_service.py +++ b/src/aignostics/application/_service.py @@ -800,11 +800,13 @@ def application_runs( # noqa: C901, PLR0912, PLR0913, PLR0915 logger.exception(message) raise RuntimeError(message) from e - def application_run(self, run_id: str) -> Run: + def application_run(self, run_id: str, share_token: str | None = None) -> Run: """Select a run by its ID. Args: - run_id (str): The ID of the run to find + run_id (str): The ID of the run to find. + share_token (str | None): Optional share token secret. When provided the run + is accessed via the ``share_token`` query parameter without OAuth. Returns: Run: The run that can be fetched using the .details() call. @@ -813,6 +815,8 @@ def application_run(self, run_id: str) -> Run: RuntimeError: If initializing the client fails or the run cannot be retrieved. """ try: + if share_token is not None: + return Run.for_run_id(run_id, share_token=share_token) return self._get_platform_client().run(run_id) except Exception as e: message = f"Failed to retrieve application run with ID '{run_id}': {e}" @@ -1675,6 +1679,7 @@ def application_run_download( # noqa: C901, PLR0912, PLR0913, PLR0915 qupath_project: bool = False, download_progress_queue: Any | None = None, # noqa: ANN401 download_progress_callable: Callable | None = None, # type: ignore[type-arg] + share_token: str | None = None, ) -> Path: """Download application run results with progress tracking. @@ -1691,6 +1696,7 @@ def application_run_download( # noqa: C901, PLR0912, PLR0913, PLR0915 of the destination directory. download_progress_queue (Queue | None): Queue for GUI progress updates. download_progress_callable (Callable | None): Callback for CLI progress updates. + share_token (str | None): Optional share token secret for unauthenticated access. Returns: Path: The directory containing downloaded results. @@ -1721,7 +1727,7 @@ def application_run_download( # noqa: C901, PLR0912, PLR0913, PLR0915 progress = DownloadProgress() update_progress(progress, download_progress_callable, download_progress_queue) - application_run = self.application_run(run_id) + application_run = self.application_run(run_id, share_token=share_token) final_destination_directory = destination_directory try: details = application_run.details() diff --git a/src/aignostics/platform/resources/runs.py b/src/aignostics/platform/resources/runs.py index e080a570d..9d388f472 100644 --- a/src/aignostics/platform/resources/runs.py +++ b/src/aignostics/platform/resources/runs.py @@ -107,17 +107,20 @@ class Artifact(_AuthenticatedResource): ``GET /api/v1/runs/{run_id}/artifacts/{artifact_id}/file`` endpoint. """ - def __init__(self, api: _AuthenticatedApi, run_id: str, artifact_id: str) -> None: + def __init__(self, api: _AuthenticatedApi, run_id: str, artifact_id: str, share_token: str | None = None) -> None: """Initializes an Artifact instance. Args: api (_AuthenticatedApi): The configured API client. run_id (str): The ID of the parent run. artifact_id (str): The ID of the output artifact. + share_token (str | None): Optional share token secret forwarded as the + ``share_token`` query parameter on the /file endpoint request. """ super().__init__(api) self.run_id = run_id self.artifact_id = artifact_id + self._share_token = share_token def get_download_url(self) -> str: """Resolve a fresh presigned download URL for this artifact. @@ -145,6 +148,8 @@ def get_download_url(self) -> str: configuration = self._api.api_client.configuration host = configuration.host.rstrip("/") endpoint_url = f"{host}/api/v1/runs/{self.run_id}/artifacts/{self.artifact_id}/file" + if self._share_token: + endpoint_url += f"?share_token={self._share_token}" proxy = getattr(configuration, "proxy", None) ssl_ca_cert = getattr(configuration, "ssl_ca_cert", None) verify_ssl = getattr(configuration, "verify_ssl", True) @@ -250,30 +255,52 @@ class Run(_AuthenticatedResource): Provides operations to check status, retrieve results, and download artifacts. """ - def __init__(self, api: _AuthenticatedApi, run_id: str) -> None: + def __init__(self, api: _AuthenticatedApi, run_id: str, share_token: str | None = None) -> None: """Initializes a Run instance. Args: api (_AuthenticatedApi): The configured API client. run_id (str): The ID of the application run. + share_token (str | None): Optional share token secret. When supplied the + token is forwarded as the ``share_token`` query parameter on every API + request, granting access without an OAuth Bearer token. """ super().__init__(api) self.run_id = run_id + self._share_token = share_token @classmethod - def for_run_id(cls, run_id: str, cache_token: bool = True) -> "Run": - """Creates an Run instance for an existing run. + def for_run_id(cls, run_id: str, cache_token: bool = True, share_token: str | None = None) -> "Run": + """Creates a Run instance for an existing run. + + When *share_token* is provided the run is accessed via the ``share_token`` + query parameter on every API request without an OAuth Bearer token. Args: run_id (str): The ID of the application run. - cache_token (bool): Whether to cache the API token. + cache_token (bool): Whether to use the cached OAuth token. Ignored + when *share_token* is supplied. + share_token (str | None): Optional share token secret. When provided + no OAuth login is required. Returns: Run: The initialized Run instance. + + Example:: + + # Authenticated access + run = Run.for_run_id("run-abc123") + + # Share-token access (no OAuth required) + run = Run.for_run_id("run-abc123", share_token="shr_xxxx") + details = run.details() + for item in run.results(): + print(item.external_id) """ from aignostics.platform._client import Client # noqa: PLC0415 - return cls(Client.get_api_client(cache_token=cache_token), run_id) + api = Client.get_api_client(cache_token=cache_token) + return cls(api, run_id, share_token=share_token) def details(self, nocache: bool = False, hide_platform_queue_position: bool = False) -> RunData: """Retrieves the current status of the application run. @@ -294,9 +321,10 @@ def details(self, nocache: bool = False, hide_platform_queue_position: bool = Fa NotFoundException: If the run is not found after retries. Exception: If the API request fails. """ + share_token = self._share_token @cached_operation(ttl=settings().run_cache_ttl, token_provider=self._api.token_provider) - def details_with_retry(run_id: str) -> RunData: + def details_with_retry(run_id: str, _share_token: str | None = None) -> RunData: def _fetch() -> RunData: return Retrying( retry=retry_if_exception_type(exception_types=RETRYABLE_EXCEPTIONS), @@ -309,6 +337,7 @@ def _fetch() -> RunData: )( lambda: self._api.get_run_v1_runs_run_id_get( run_id, + share_token=_share_token, _request_timeout=settings().run_timeout, _headers={"User-Agent": user_agent()}, ) @@ -323,7 +352,7 @@ def _fetch() -> RunData: reraise=True, )(_fetch) - run_data: RunData = details_with_retry(self.run_id, nocache=nocache) # type: ignore[call-arg] + run_data: RunData = details_with_retry(self.run_id, _share_token=share_token, nocache=nocache) # type: ignore[call-arg] if hide_platform_queue_position: run_data = run_data.model_copy(deep=True) run_data.num_preceding_items_platform = None @@ -388,11 +417,12 @@ def results( # noqa: PLR0913 Raises: Exception: If the API request fails. """ + share_token = self._share_token # Create a wrapper function that applies retry logic and caching to each API call # Caching at this level ensures having a fresh iterator on cache hits @cached_operation(ttl=settings().run_cache_ttl, token_provider=self._api.token_provider) - def results_with_retry(run_id: str, **kwargs: object) -> list[ItemResultData]: + def results_with_retry(run_id: str, _share_token: str | None = None, **kwargs: object) -> list[ItemResultData]: return Retrying( retry=retry_if_exception_type(exception_types=RETRYABLE_EXCEPTIONS), stop=stop_after_attempt(settings().run_retry_attempts), @@ -402,6 +432,7 @@ def results_with_retry(run_id: str, **kwargs: object) -> list[ItemResultData]: )( lambda: self._api.list_run_items_v1_runs_run_id_items_get( run_id=run_id, + share_token=_share_token, _request_timeout=settings().run_timeout, _headers={"User-Agent": user_agent()}, **kwargs, # pyright: ignore[reportArgumentType] @@ -420,7 +451,11 @@ def results_with_retry(run_id: str, **kwargs: object) -> list[ItemResultData]: if custom_metadata is not None: filter_kwargs["custom_metadata"] = custom_metadata - return paginate(lambda **kwargs: results_with_retry(self.run_id, nocache=nocache, **filter_kwargs, **kwargs)) + return paginate( + lambda **kwargs: results_with_retry( + self.run_id, _share_token=share_token, nocache=nocache, **filter_kwargs, **kwargs + ) + ) def download_to_folder( # noqa: C901 self, @@ -513,7 +548,7 @@ def artifact(self, artifact_id: str) -> Artifact: Returns: Artifact: A handle bound to this run and the given artifact. """ - return Artifact(self._api, self.run_id, artifact_id) + return Artifact(self._api, self.run_id, artifact_id, share_token=self._share_token) def get_artifact_download_url(self, artifact_id: str) -> str: """Resolve a fresh presigned download URL for an artifact of this run. diff --git a/tests/aignostics/application/cli_test.py b/tests/aignostics/application/cli_test.py index 46b3c36c1..2439d345e 100644 --- a/tests/aignostics/application/cli_test.py +++ b/tests/aignostics/application/cli_test.py @@ -981,6 +981,196 @@ def test_cli_run_describe_json_includes_items(runner: CliRunner) -> None: assert item["termination_reason"] == "SUCCEEDED" +def _make_mock_run(run_id: str = "run-shared-001", custom_metadata: dict | None = None) -> MagicMock: + """Build a mock Run that returns a minimal RunReadResponse from details().""" + mock_run_data = RunReadResponse( + run_id=run_id, + application_id="test-app", + version_number="1.0.0", + state=RunState.TERMINATED, + output=RunOutput.FULL, + termination_reason=RunTerminationReason.ALL_ITEMS_PROCESSED, + error_code=None, + error_message=None, + statistics=RunItemStatistics( + item_count=0, + item_pending_count=0, + item_processing_count=0, + item_user_error_count=0, + item_system_error_count=0, + item_skipped_count=0, + item_succeeded_count=0, + ), + custom_metadata=custom_metadata, + submitted_at=datetime(2025, 1, 1, tzinfo=UTC), + submitted_by="test-user", + terminated_at=datetime(2025, 1, 1, 0, 1, tzinfo=UTC), + ) + mock_run = MagicMock() + mock_run.details.return_value = mock_run_data + mock_run.results.return_value = iter([]) + return mock_run + + +@pytest.mark.integration +def test_cli_run_describe_with_share_token_success(runner: CliRunner, record_property: object) -> None: + """Run describe --share-token succeeds without OAuth and returns run details.""" + record_property("tested-item-id", "PYSDK-145") + mock_run = _make_mock_run("run-shared-001") + + with patch(APPLICATION_CLI_SERVICE_PATCH_TARGET) as mock_svc_cls: + mock_svc_cls.return_value.application_run.return_value = mock_run + result = runner.invoke(cli, ["application", "run", "describe", "run-shared-001", "--share-token", "s3cr3t"]) + + assert result.exit_code == 0, f"Unexpected exit: {result.output}" + mock_svc_cls.return_value.application_run.assert_called_once_with("run-shared-001", share_token="s3cr3t") # noqa: S106 + assert "run-shared-001" in normalize_output(result.output) + + +@pytest.mark.integration +def test_cli_run_describe_with_share_token_json(runner: CliRunner, record_property: object) -> None: + """Run describe --share-token --format json returns valid JSON without OAuth.""" + record_property("tested-item-id", "PYSDK-145") + mock_run = _make_mock_run("run-shared-002") + + with patch(APPLICATION_CLI_SERVICE_PATCH_TARGET) as mock_svc_cls: + mock_svc_cls.return_value.application_run.return_value = mock_run + result = runner.invoke( + cli, + ["application", "run", "describe", "run-shared-002", "--share-token", "s3cr3t", "--format", "json"], + ) + + assert result.exit_code == 0, f"Unexpected exit: {result.output}" + data = json.loads(result.stdout) + assert data["run_id"] == "run-shared-002" + assert "items" in data + + +@pytest.mark.integration +def test_cli_run_describe_with_share_token_not_found(runner: CliRunner, record_property: object) -> None: + """Run describe --share-token exits 2 when run does not exist.""" + record_property("tested-item-id", "PYSDK-145") + with patch(APPLICATION_CLI_SERVICE_PATCH_TARGET) as mock_svc_cls: + mock_svc_cls.return_value.application_run.side_effect = ApiNotFound(status=404, reason="Not Found") + result = runner.invoke(cli, ["application", "run", "describe", "bad-run-id", "--share-token", "s3cr3t"]) + + assert result.exit_code == 2 + assert "not found" in normalize_output(result.output).lower() + + +@pytest.mark.integration +def test_cli_run_describe_with_share_token_forbidden(runner: CliRunner, record_property: object) -> None: + """Run describe --share-token exits 1 when token is invalid, expired, or has no access.""" + record_property("tested-item-id", "PYSDK-145") + with patch(APPLICATION_CLI_SERVICE_PATCH_TARGET) as mock_svc_cls: + mock_svc_cls.return_value.application_run.side_effect = ForbiddenException(status=403, reason="Forbidden") + result = runner.invoke(cli, ["application", "run", "describe", "run-id", "--share-token", "bad-token"]) + + assert result.exit_code == 1 + assert "Access denied" in normalize_output(result.output) + + +@pytest.mark.integration +def test_cli_run_dump_metadata_with_share_token_success(runner: CliRunner, record_property: object) -> None: + """Run dump-metadata --share-token returns custom metadata JSON without OAuth.""" + record_property("tested-item-id", "PYSDK-145") + mock_run = _make_mock_run("run-001", custom_metadata={"key": "value"}) + + with patch(APPLICATION_CLI_SERVICE_PATCH_TARGET) as mock_svc_cls: + mock_svc_cls.return_value.application_run.return_value = mock_run + result = runner.invoke(cli, ["application", "run", "dump-metadata", "run-001", "--share-token", "s3cr3t"]) + + assert result.exit_code == 0, f"Unexpected exit: {result.output}" + mock_svc_cls.return_value.application_run.assert_called_once_with("run-001", share_token="s3cr3t") # noqa: S106 + assert "key" in result.output + + +@pytest.mark.integration +def test_cli_run_dump_metadata_with_share_token_forbidden(runner: CliRunner, record_property: object) -> None: + """Run dump-metadata --share-token exits 1 on forbidden.""" + record_property("tested-item-id", "PYSDK-145") + with patch(APPLICATION_CLI_SERVICE_PATCH_TARGET) as mock_svc_cls: + mock_svc_cls.return_value.application_run.side_effect = ForbiddenException(status=403, reason="Forbidden") + result = runner.invoke(cli, ["application", "run", "dump-metadata", "run-id", "--share-token", "bad"]) + + assert result.exit_code == 1 + assert "Access denied" in normalize_output(result.output) + + +@pytest.mark.integration +def test_cli_run_dump_item_metadata_with_share_token_success(runner: CliRunner, record_property: object) -> None: + """Run dump-item-metadata --share-token finds an item and returns its metadata without OAuth.""" + record_property("tested-item-id", "PYSDK-145") + mock_item = MagicMock() + mock_item.external_id = "slide-001.svs" + mock_item.custom_metadata = {"slide": "meta"} + + mock_run = MagicMock() + mock_run.results.return_value = iter([mock_item]) + + with patch(APPLICATION_CLI_SERVICE_PATCH_TARGET) as mock_svc_cls: + mock_svc_cls.return_value.application_run.return_value = mock_run + result = runner.invoke( + cli, + ["application", "run", "dump-item-metadata", "run-001", "slide-001.svs", "--share-token", "s3cr3t"], + ) + + assert result.exit_code == 0, f"Unexpected exit: {result.output}" + mock_svc_cls.return_value.application_run.assert_called_once_with("run-001", share_token="s3cr3t") # noqa: S106 + assert "slide" in result.output + + +@pytest.mark.integration +def test_cli_run_dump_item_metadata_with_share_token_forbidden(runner: CliRunner, record_property: object) -> None: + """Run dump-item-metadata --share-token exits 1 on forbidden.""" + record_property("tested-item-id", "PYSDK-145") + with patch(APPLICATION_CLI_SERVICE_PATCH_TARGET) as mock_svc_cls: + mock_svc_cls.return_value.application_run.side_effect = ForbiddenException(status=403, reason="Forbidden") + result = runner.invoke( + cli, ["application", "run", "dump-item-metadata", "run-id", "item-id", "--share-token", "bad"] + ) + + assert result.exit_code == 1 + assert "Access denied" in normalize_output(result.output) + + +@pytest.mark.integration +def test_cli_result_download_with_share_token_passes_token_to_service( + runner: CliRunner, tmp_path: Path, record_property: object +) -> None: + """Result download --share-token forwards the token to application_run_download.""" + record_property("tested-item-id", "PYSDK-145") + with patch(APPLICATION_CLI_SERVICE_PATCH_TARGET) as mock_svc_cls: + mock_svc_cls.return_value.application_run_download.return_value = tmp_path + result = runner.invoke( + cli, + ["application", "run", "result", "download", "run-001", str(tmp_path), "--share-token", "s3cr3t"], + ) + + assert result.exit_code == 0, f"Unexpected exit: {result.output}" + call_kwargs = mock_svc_cls.return_value.application_run_download.call_args.kwargs + assert call_kwargs.get("share_token") == "s3cr3t" + + +@pytest.mark.integration +def test_cli_result_download_with_share_token_forbidden( + runner: CliRunner, tmp_path: Path, record_property: object +) -> None: + """Result download --share-token exits 1 on forbidden.""" + record_property("tested-item-id", "PYSDK-145") + with patch(APPLICATION_CLI_SERVICE_PATCH_TARGET) as mock_svc_cls: + mock_svc_cls.return_value.application_run_download.side_effect = ForbiddenException( + status=403, reason="Forbidden" + ) + result = runner.invoke( + cli, + ["application", "run", "result", "download", "run-id", str(tmp_path), "--share-token", "bad"], + ) + + assert result.exit_code == 1 + assert "Access denied" in normalize_output(result.output) + + @pytest.mark.e2e def test_cli_run_cancel_invalid_run_id(runner: CliRunner, record_property) -> None: """Check run cancel command fails as expected on run not found.""" diff --git a/uv.lock b/uv.lock index 754d8a224..35106bc28 100644 --- a/uv.lock +++ b/uv.lock @@ -60,6 +60,7 @@ dependencies = [ { name = "humanize" }, { name = "idc-index-data" }, { name = "ijson" }, + { name = "joserfc" }, { name = "jsf" }, { name = "jsonschema", extra = ["format-nongpl"] }, { name = "loguru" }, @@ -84,7 +85,9 @@ dependencies = [ { name = "pygments" }, { name = "pyjwt", extra = ["crypto"] }, { name = "python-dateutil" }, + { name = "python-engineio" }, { name = "python-multipart" }, + { name = "python-socketio" }, { name = "pywin32", marker = "sys_platform == 'win32'" }, { name = "pyyaml" }, { name = "requests" }, @@ -204,6 +207,7 @@ requires-dist = [ { name = "idc-index-data", specifier = "==24.0.3" }, { name = "ijson", specifier = ">=3.4.0.post0,<4" }, { name = "ipython", marker = "extra == 'marimo'", specifier = ">=9.8.0,<10" }, + { name = "joserfc", specifier = ">=1.6.7" }, { name = "jsf", specifier = ">=0.11.2,<1" }, { name = "jsonschema", extras = ["format-nongpl"], specifier = ">=4.25.1,<5" }, { name = "jupyter", marker = "extra == 'jupyter'", specifier = ">=1.1.1,<2" }, @@ -237,7 +241,9 @@ requires-dist = [ { name = "pyinstaller", marker = "extra == 'pyinstaller'", specifier = ">=6.14.0,<7" }, { name = "pyjwt", extras = ["crypto"], specifier = ">=2.13.0,<3" }, { name = "python-dateutil", specifier = ">=2.9.0.post0,<3" }, + { name = "python-engineio", specifier = ">=4.13.3" }, { name = "python-multipart", specifier = ">=0.0.26" }, + { name = "python-socketio", specifier = ">=5.16.3" }, { name = "pywin32", marker = "sys_platform == 'win32'", specifier = ">=312,<313" }, { name = "pyyaml", specifier = ">=6.0.3,<7" }, { name = "requests", specifier = ">=2.33.0,<3" }, From 446759094e9883068fd71cbdb7446635dcae32a2 Mon Sep 17 00:00:00 2001 From: Dzmitry Talkach Date: Wed, 1 Jul 2026 11:17:35 +0200 Subject: [PATCH 2/6] Update the docstring --- src/aignostics/application/_cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/aignostics/application/_cli.py b/src/aignostics/application/_cli.py index 232c572c5..184c88943 100644 --- a/src/aignostics/application/_cli.py +++ b/src/aignostics/application/_cli.py @@ -973,7 +973,7 @@ def run_describe( # noqa: PLR0912 share_token: Annotated[ str | None, typer.Option( - help="Share token secret for link-based access. When provided, OAuth login is not required.", + help="Share token secret for link-based access.", ), ] = None, ) -> None: From f9b53dad07d29750424f7b11feae391db4fb0f89 Mon Sep 17 00:00:00 2001 From: Oliver Meyer Date: Mon, 3 Aug 2026 16:23:18 +0200 Subject: [PATCH 3/6] feat(application): add --share-token to run commands Adds a --share-token option to `application run describe / dump-metadata / dump-item-metadata / result download` (PYSDK-145), granting an authenticated user access to a run that has been shared with them. Co-Authored-By: Claude Opus 4.8 --- pyproject.toml | 3 - src/aignostics/application/_cli.py | 50 ++----- src/aignostics/application/_service.py | 4 + src/aignostics/application/_utils.py | 22 ++++ src/aignostics/platform/resources/runs.py | 52 +++++--- tests/aignostics/application/cli_test.py | 57 ++++++++ tests/aignostics/application/service_test.py | 20 ++- .../platform/resources/runs_test.py | 122 +++++++++++++++++- uv.lock | 6 - 9 files changed, 270 insertions(+), 66 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 5ce63dce6..59be73281 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -105,7 +105,6 @@ dependencies = [ "httpx>=0.28.1,<1", "idc-index-data==24.0.3", "ijson>=3.4.0.post0,<4", - "joserfc>=1.6.7", "jsf>=0.11.2,<1", "jsonschema[format-nongpl]>=4.25.1,<5", "loguru>=0.7.3,<1", @@ -119,8 +118,6 @@ dependencies = [ "pyarrow>=23.0.1,<26; python_version >= '3.14'", "pyjwt[crypto]>=2.13.0,<3", # CVE-2026-32597 requires >=2.12.0 (Renovate #475) "python-dateutil>=2.9.0.post0,<3", - "python-engineio>=4.13.3", - "python-socketio>=5.16.3", # "pywebview[qt6]>=5.4,<6; sys_platform == 'linux'", "requests>=2.33.0,<3", # CVE-2026-25645 requires >= 2.33.0 "requests-oauthlib>=2.0.0,<3", diff --git a/src/aignostics/application/_cli.py b/src/aignostics/application/_cli.py index 184c88943..65f84cc4c 100644 --- a/src/aignostics/application/_cli.py +++ b/src/aignostics/application/_cli.py @@ -38,6 +38,7 @@ print_runs_verbose, read_metadata_csv_to_dict, retrieve_and_print_run_details, + share_token_access_denied_message, validate_mappings, write_metadata_dict_to_csv, ) @@ -121,6 +122,10 @@ int, typer.Option(help="Timeout for acquiring compute nodes in minutes (1-3600).", min=1, max=3600), ] +ShareTokenOption = Annotated[ + str | None, + typer.Option(help="Share token secret for link-based access. OAuth login is still required."), +] cli = typer.Typer(name="application", help="List and inspect applications on Aignostics Platform.") @@ -970,12 +975,7 @@ def run_describe( # noqa: PLR0912 help="Show only run and item status summary (external ID, state, error message)", ), ] = False, - share_token: Annotated[ - str | None, - typer.Option( - help="Share token secret for link-based access.", - ), - ] = None, + share_token: ShareTokenOption = None, ) -> None: """Describe run.""" logger.trace("Describing run with ID '{}'", run_id) @@ -1003,10 +1003,7 @@ def run_describe( # noqa: PLR0912 console.print(f"[warning]Warning:[/warning] Run with ID '{run_id}' not found.") sys.exit(2) except ForbiddenException: - logger.warning("Access denied for run '{}'", run_id) - msg = f"Access denied for run '{run_id}'." - if share_token is not None: - msg += " The share token may be invalid, expired, or revoked." + msg = share_token_access_denied_message(run_id, share_token) if format == "json": print(json.dumps({"error": "access_denied", "message": msg}), file=sys.stderr) else: @@ -1035,10 +1032,7 @@ def run_dump_metadata( ), ), ] = False, - share_token: Annotated[ - str | None, - typer.Option(help="Share token secret for link-based access. When provided, OAuth login is not required."), - ] = None, + share_token: ShareTokenOption = None, ) -> None: """Dump custom metadata of a run as JSON to stdout.""" logger.trace("Dumping custom metadata for run with ID '{}'", run_id) @@ -1065,11 +1059,7 @@ def run_dump_metadata( console.print(f"[warning]Warning:[/warning] Run with ID '{run_id}' not found.") sys.exit(2) except ForbiddenException: - logger.warning("Access denied for run '{}'", run_id) - msg = f"Access denied for run '{run_id}'." - if share_token is not None: - msg += " The share token may be invalid, expired, or revoked." - console.print(f"[error]Error:[/error] {msg}") + console.print(f"[error]Error:[/error] {share_token_access_denied_message(run_id, share_token)}") sys.exit(1) except Exception as e: logger.exception(f"Failed to dump custom metadata for run with ID '{run_id}'") @@ -1082,6 +1072,7 @@ def run_dump_item_metadata( run_id: Annotated[str, typer.Argument(help="Id of the run containing the item")], external_id: Annotated[str, typer.Argument(help="External ID of the item to dump custom metadata for")], pretty: Annotated[bool, typer.Option(help="Pretty print JSON output with indentation")] = False, + share_token: ShareTokenOption = None, show_checksum: Annotated[ bool, typer.Option( @@ -1093,10 +1084,6 @@ def run_dump_item_metadata( ), ), ] = False, - share_token: Annotated[ - str | None, - typer.Option(help="Share token secret for link-based access. When provided, OAuth login is not required."), - ] = None, ) -> None: """Dump custom metadata of an item as JSON to stdout.""" logger.trace("Dumping custom metadata for item '{}' in run with ID '{}'", external_id, run_id) @@ -1139,11 +1126,7 @@ def run_dump_item_metadata( print(f"Warning: Run with ID '{run_id}' not found.", file=sys.stderr) sys.exit(2) except ForbiddenException: - logger.warning("Access denied for run '{}'", run_id) - msg = f"Access denied for run '{run_id}'." - if share_token is not None: - msg += " The share token may be invalid, expired, or revoked." - print(f"Error: {msg}", file=sys.stderr) + print(f"Error: {share_token_access_denied_message(run_id, share_token)}", file=sys.stderr) sys.exit(1) except Exception as e: logger.exception(f"Failed to dump custom metadata for item '{external_id}' in run with ID '{run_id}'") @@ -1700,10 +1683,7 @@ def result_download( # noqa: C901, PLR0913, PLR0915 'Run uvx --with "aignostics[qupath]" aignostics qupath install' ), ] = False, - share_token: Annotated[ - str | None, - typer.Option(help="Share token secret for link-based access. When provided, OAuth login is not required."), - ] = None, + share_token: ShareTokenOption = None, ) -> None: """Download results of a run.""" logger.trace( @@ -1868,11 +1848,7 @@ def update_progress(progress: DownloadProgress) -> None: # noqa: C901 console.print(f"[warning]Warning:[/warning] Bad input to download results of run with ID '{run_id}': {e}") sys.exit(2) except ForbiddenException: - logger.warning("Access denied for run '{}'", run_id) - msg = f"Access denied for run '{run_id}'." - if share_token is not None: - msg += " The share token may be invalid, expired, or revoked." - console.print(f"[error]Error:[/error] {msg}") + console.print(f"[error]Error:[/error] {share_token_access_denied_message(run_id, share_token)}") sys.exit(1) except Exception as e: logger.exception(f"Failed to download results of run with ID '{run_id}'") diff --git a/src/aignostics/application/_service.py b/src/aignostics/application/_service.py index 0ebe57240..776f2f7df 100644 --- a/src/aignostics/application/_service.py +++ b/src/aignostics/application/_service.py @@ -1735,6 +1735,10 @@ def application_run_download( # noqa: C901, PLR0912, PLR0913, PLR0915 message = f"Application run with ID '{run_id}' not found: {e}" logger.warning(message) raise NotFoundException(message) from e + except ForbiddenException: + # Propagate 403 unchanged so the CLI can surface a share-token-specific + # "access denied" message; do not wrap it into RuntimeError below. + raise except ApiException as e: if e.status == HTTPStatus.UNPROCESSABLE_ENTITY: message = f"Run ID '{run_id}' invalid: {e!s}." diff --git a/src/aignostics/application/_utils.py b/src/aignostics/application/_utils.py index 9c52d0647..9cfd0cc3f 100644 --- a/src/aignostics/application/_utils.py +++ b/src/aignostics/application/_utils.py @@ -599,3 +599,25 @@ def get_supported_extensions_for_application(application_id: str) -> set[str]: message = f"Unsupported application {application_id}" logger.critical(message) raise RuntimeError(message) + + +def share_token_access_denied_message(run_id: str, share_token: str | None) -> str: + """Compose the operator-facing "access denied" message for a run and log a warning. + + Centralizes the wording, share-token hint, and warning log shared by the run CLI + commands that support ``--share-token``. The caller owns the output sink (console, + stderr, or JSON) and the exit code. + + Args: + run_id (str): The run access was denied for. + share_token (str | None): The share token supplied, if any. When set, a hint + that the token may be invalid, expired, or revoked is appended. + + Returns: + str: The composed message. + """ + logger.warning("Access denied for run '{}'", run_id) + message = f"Access denied for run '{run_id}'." + if share_token is not None: + message += " The share token may be invalid, expired, or revoked." + return message diff --git a/src/aignostics/platform/resources/runs.py b/src/aignostics/platform/resources/runs.py index 9d388f472..3aa82351a 100644 --- a/src/aignostics/platform/resources/runs.py +++ b/src/aignostics/platform/resources/runs.py @@ -12,6 +12,7 @@ from pathlib import Path from time import sleep from typing import Any, cast +from urllib.parse import urlencode import requests from aignx.codegen.exceptions import ApiException, NotFoundException, ServiceException @@ -148,8 +149,10 @@ def get_download_url(self) -> str: configuration = self._api.api_client.configuration host = configuration.host.rstrip("/") endpoint_url = f"{host}/api/v1/runs/{self.run_id}/artifacts/{self.artifact_id}/file" - if self._share_token: - endpoint_url += f"?share_token={self._share_token}" + if self._share_token is not None: + # Percent-encode the secret so reserved characters (& # = space) cannot + # corrupt the URL or inject extra query parameters. + endpoint_url += f"?{urlencode({'share_token': self._share_token})}" proxy = getattr(configuration, "proxy", None) ssl_ca_cert = getattr(configuration, "ssl_ca_cert", None) verify_ssl = getattr(configuration, "verify_ssl", True) @@ -197,6 +200,10 @@ def _fetch_redirect_url( RuntimeError: 3xx without a Location header, or unexpected non-3xx status. """ try: + # Always send the OAuth Bearer token: the platform requires an + # authenticated account on every request. When a share_token is present + # it is carried as a query parameter on ``endpoint_url`` and elevates + # that authenticated user's access to the shared resource. with requests.get( endpoint_url, headers={ @@ -263,7 +270,7 @@ def __init__(self, api: _AuthenticatedApi, run_id: str, share_token: str | None run_id (str): The ID of the application run. share_token (str | None): Optional share token secret. When supplied the token is forwarded as the ``share_token`` query parameter on every API - request, granting access without an OAuth Bearer token. + request, elevating the authenticated user's access to the shared run. """ super().__init__(api) self.run_id = run_id @@ -273,25 +280,25 @@ def __init__(self, api: _AuthenticatedApi, run_id: str, share_token: str | None def for_run_id(cls, run_id: str, cache_token: bool = True, share_token: str | None = None) -> "Run": """Creates a Run instance for an existing run. - When *share_token* is provided the run is accessed via the ``share_token`` - query parameter on every API request without an OAuth Bearer token. + When *share_token* is provided it is forwarded as the ``share_token`` query + parameter on every API request, elevating the calling (OAuth-authenticated) + user's access to a run they would otherwise not be able to read. Args: run_id (str): The ID of the application run. - cache_token (bool): Whether to use the cached OAuth token. Ignored - when *share_token* is supplied. - share_token (str | None): Optional share token secret. When provided - no OAuth login is required. + cache_token (bool): Whether to use the cached OAuth token. + share_token (str | None): Optional share token secret. The caller must + still be authenticated; the token grants access to the shared run. Returns: Run: The initialized Run instance. Example:: - # Authenticated access + # Authenticated access to a run you own run = Run.for_run_id("run-abc123") - # Share-token access (no OAuth required) + # Share-token access to a run shared with you (still authenticated) run = Run.for_run_id("run-abc123", share_token="shr_xxxx") details = run.details() for item in run.results(): @@ -299,6 +306,10 @@ def for_run_id(cls, run_id: str, cache_token: bool = True, share_token: str | No """ from aignostics.platform._client import Client # noqa: PLC0415 + # Share-token access still authenticates as the calling user: the platform + # requires an OAuth Bearer token on every request. The share_token is + # forwarded as a query parameter (see details/results/Artifact) and elevates + # the authenticated user's access to the shared resource. api = Client.get_api_client(cache_token=cache_token) return cls(api, run_id, share_token=share_token) @@ -323,8 +334,11 @@ def details(self, nocache: bool = False, hide_platform_queue_position: bool = Fa """ share_token = self._share_token + # share_token is threaded as an explicit argument (not a closure capture) so it + # participates in the operation cache key, keeping share-token reads isolated + # from authenticated reads of the same run_id. @cached_operation(ttl=settings().run_cache_ttl, token_provider=self._api.token_provider) - def details_with_retry(run_id: str, _share_token: str | None = None) -> RunData: + def details_with_retry(run_id: str, share_token: str | None = None) -> RunData: def _fetch() -> RunData: return Retrying( retry=retry_if_exception_type(exception_types=RETRYABLE_EXCEPTIONS), @@ -337,7 +351,7 @@ def _fetch() -> RunData: )( lambda: self._api.get_run_v1_runs_run_id_get( run_id, - share_token=_share_token, + share_token=share_token, _request_timeout=settings().run_timeout, _headers={"User-Agent": user_agent()}, ) @@ -352,7 +366,7 @@ def _fetch() -> RunData: reraise=True, )(_fetch) - run_data: RunData = details_with_retry(self.run_id, _share_token=share_token, nocache=nocache) # type: ignore[call-arg] + run_data: RunData = details_with_retry(self.run_id, share_token=share_token, nocache=nocache) # type: ignore[call-arg] if hide_platform_queue_position: run_data = run_data.model_copy(deep=True) run_data.num_preceding_items_platform = None @@ -420,9 +434,11 @@ def results( # noqa: PLR0913 share_token = self._share_token # Create a wrapper function that applies retry logic and caching to each API call - # Caching at this level ensures having a fresh iterator on cache hits + # Caching at this level ensures having a fresh iterator on cache hits. + # share_token is an explicit argument (not a closure capture) so it participates + # in the operation cache key, isolating share-token reads from authenticated reads. @cached_operation(ttl=settings().run_cache_ttl, token_provider=self._api.token_provider) - def results_with_retry(run_id: str, _share_token: str | None = None, **kwargs: object) -> list[ItemResultData]: + def results_with_retry(run_id: str, share_token: str | None = None, **kwargs: object) -> list[ItemResultData]: return Retrying( retry=retry_if_exception_type(exception_types=RETRYABLE_EXCEPTIONS), stop=stop_after_attempt(settings().run_retry_attempts), @@ -432,7 +448,7 @@ def results_with_retry(run_id: str, _share_token: str | None = None, **kwargs: o )( lambda: self._api.list_run_items_v1_runs_run_id_items_get( run_id=run_id, - share_token=_share_token, + share_token=share_token, _request_timeout=settings().run_timeout, _headers={"User-Agent": user_agent()}, **kwargs, # pyright: ignore[reportArgumentType] @@ -453,7 +469,7 @@ def results_with_retry(run_id: str, _share_token: str | None = None, **kwargs: o return paginate( lambda **kwargs: results_with_retry( - self.run_id, _share_token=share_token, nocache=nocache, **filter_kwargs, **kwargs + self.run_id, share_token=share_token, nocache=nocache, **filter_kwargs, **kwargs ) ) diff --git a/tests/aignostics/application/cli_test.py b/tests/aignostics/application/cli_test.py index 2439d345e..efaf706e6 100644 --- a/tests/aignostics/application/cli_test.py +++ b/tests/aignostics/application/cli_test.py @@ -1041,6 +1041,7 @@ def test_cli_run_describe_with_share_token_json(runner: CliRunner, record_proper ) assert result.exit_code == 0, f"Unexpected exit: {result.output}" + mock_svc_cls.return_value.application_run.assert_called_once_with("run-shared-002", share_token="s3cr3t") # noqa: S106 data = json.loads(result.stdout) assert data["run_id"] == "run-shared-002" assert "items" in data @@ -1171,6 +1172,62 @@ def test_cli_result_download_with_share_token_forbidden( assert "Access denied" in normalize_output(result.output) +@pytest.mark.integration +def test_cli_run_describe_without_share_token_passes_none(runner: CliRunner, record_property: object) -> None: + """Run describe without --share-token calls the service with share_token=None (no leakage).""" + record_property("tested-item-id", "PYSDK-145") + mock_run = _make_mock_run("run-noshare") + + with patch(APPLICATION_CLI_SERVICE_PATCH_TARGET) as mock_svc_cls: + mock_svc_cls.return_value.application_run.return_value = mock_run + result = runner.invoke(cli, ["application", "run", "describe", "run-noshare"]) + + assert result.exit_code == 0, f"Unexpected exit: {result.output}" + mock_svc_cls.return_value.application_run.assert_called_once_with("run-noshare", share_token=None) + + +@pytest.mark.integration +def test_cli_run_dump_metadata_with_share_token_not_found(runner: CliRunner, record_property: object) -> None: + """Run dump-metadata --share-token exits 2 when run does not exist.""" + record_property("tested-item-id", "PYSDK-145") + with patch(APPLICATION_CLI_SERVICE_PATCH_TARGET) as mock_svc_cls: + mock_svc_cls.return_value.application_run.side_effect = ApiNotFound(status=404, reason="Not Found") + result = runner.invoke(cli, ["application", "run", "dump-metadata", "bad-run-id", "--share-token", "s3cr3t"]) + + assert result.exit_code == 2 + assert "not found" in normalize_output(result.output).lower() + + +@pytest.mark.integration +def test_cli_run_dump_item_metadata_with_share_token_not_found(runner: CliRunner, record_property: object) -> None: + """Run dump-item-metadata --share-token exits 2 when run does not exist.""" + record_property("tested-item-id", "PYSDK-145") + with patch(APPLICATION_CLI_SERVICE_PATCH_TARGET) as mock_svc_cls: + mock_svc_cls.return_value.application_run.side_effect = ApiNotFound(status=404, reason="Not Found") + result = runner.invoke( + cli, ["application", "run", "dump-item-metadata", "bad-run-id", "item-id", "--share-token", "s3cr3t"] + ) + + assert result.exit_code == 2 + assert "not found" in normalize_output(result.output).lower() + + +@pytest.mark.integration +def test_cli_result_download_with_share_token_not_found( + runner: CliRunner, tmp_path: Path, record_property: object +) -> None: + """Result download --share-token exits 2 when run does not exist.""" + record_property("tested-item-id", "PYSDK-145") + with patch(APPLICATION_CLI_SERVICE_PATCH_TARGET) as mock_svc_cls: + mock_svc_cls.return_value.application_run_download.side_effect = ApiNotFound(status=404, reason="Not Found") + result = runner.invoke( + cli, ["application", "run", "result", "download", "bad-run-id", str(tmp_path), "--share-token", "s3cr3t"] + ) + + assert result.exit_code == 2 + assert "not found" in normalize_output(result.output).lower() + + @pytest.mark.e2e def test_cli_run_cancel_invalid_run_id(runner: CliRunner, record_property) -> None: """Check run cancel command fails as expected on run not found.""" diff --git a/tests/aignostics/application/service_test.py b/tests/aignostics/application/service_test.py index 4ab026072..5b5b4ec75 100644 --- a/tests/aignostics/application/service_test.py +++ b/tests/aignostics/application/service_test.py @@ -5,7 +5,7 @@ from unittest.mock import MagicMock, patch import pytest -from aignx.codegen.exceptions import ApiException +from aignx.codegen.exceptions import ApiException, ForbiddenException from aignx.codegen.models import SubjectType from typer.testing import CliRunner @@ -1070,3 +1070,21 @@ def test_application_run_revoke_share_token_not_found(mock_get_client: MagicMock with pytest.raises(NotFoundException, match="No grant found"): ApplicationService().application_run_revoke_share_token("run-123", "tok-missing") + + +@pytest.mark.unit +def test_application_run_download_reraises_forbidden(tmp_path, record_property: object) -> None: + """A 403 from run.details() propagates as ForbiddenException, not wrapped into RuntimeError. + + Guards the CLI's share-token 'access denied' handler: the download path must not + swallow ForbiddenException into RuntimeError via its generic ApiException branch. + """ + record_property("tested-item-id", "PYSDK-145") + mock_run = MagicMock() + mock_run.details.side_effect = ForbiddenException(status=403, reason="Forbidden") + + with ( + patch.object(ApplicationService, "application_run", return_value=mock_run), + pytest.raises(ForbiddenException), + ): + ApplicationService().application_run_download("run-id", tmp_path, share_token="s3cr3t") # noqa: S106 diff --git a/tests/aignostics/platform/resources/runs_test.py b/tests/aignostics/platform/resources/runs_test.py index 3a0975834..867cf1af5 100644 --- a/tests/aignostics/platform/resources/runs_test.py +++ b/tests/aignostics/platform/resources/runs_test.py @@ -19,7 +19,12 @@ ) from aignostics.platform._api import _AuthenticatedApi -from aignostics.platform.resources.runs import LIST_APPLICATION_RUNS_MAX_PAGE_SIZE, Artifact, Run, Runs +from aignostics.platform.resources.runs import ( + LIST_APPLICATION_RUNS_MAX_PAGE_SIZE, + Artifact, + Run, + Runs, +) from aignostics.platform.resources.utils import PAGE_SIZE _PLATFORM_HOST = "https://platform-staging.aignostics.com" @@ -1402,3 +1407,118 @@ def test_update_item_custom_metadata_no_enrich_skips_sdk_metadata_builders(app_r request = call_kwargs["custom_metadata_update_request"] assert request.custom_metadata == {"key": "value"} assert "sdk" not in request.custom_metadata + + +# ───────────────────────────────────────────────────────────────────────────── +# share_token: forwarding to the API layer + OAuth-less wiring (PYSDK-145) +# ───────────────────────────────────────────────────────────────────────────── + + +@pytest.mark.unit +def test_details_forwards_share_token(mock_api, record_property) -> None: + """Run.details() forwards a configured share_token to the get-run API call.""" + record_property("tested-item-id", "PYSDK-145") + mock_api.get_run_v1_runs_run_id_get.return_value = RunReadResponse.model_construct(run_id="test-run-id") + + Run(mock_api, "test-run-id", share_token="s3cr3t").details() # noqa: S106 + + assert mock_api.get_run_v1_runs_run_id_get.call_args.kwargs["share_token"] == "s3cr3t" # noqa: S105 + + +@pytest.mark.unit +def test_details_share_token_none_when_not_supplied(app_run, mock_api, record_property) -> None: + """Run.details() sends share_token=None when no share token was supplied (no leakage).""" + record_property("tested-item-id", "PYSDK-145") + mock_api.get_run_v1_runs_run_id_get.return_value = RunReadResponse.model_construct(run_id="test-run-id") + + app_run.details() + + assert mock_api.get_run_v1_runs_run_id_get.call_args.kwargs["share_token"] is None + + +@pytest.mark.unit +def test_results_forwards_share_token(mock_api, record_property) -> None: + """Run.results() forwards a configured share_token to the list-items API call on every page.""" + record_property("tested-item-id", "PYSDK-145") + mock_api.list_run_items_v1_runs_run_id_items_get.return_value = [] + + list(Run(mock_api, "test-run-id", share_token="s3cr3t").results()) # noqa: S106 + + assert mock_api.list_run_items_v1_runs_run_id_items_get.call_args.kwargs["share_token"] == "s3cr3t" # noqa: S105 + + +@pytest.mark.unit +def test_results_share_token_none_when_not_supplied(app_run, mock_api, record_property) -> None: + """Run.results() sends share_token=None when no share token was supplied (no leakage).""" + record_property("tested-item-id", "PYSDK-145") + mock_api.list_run_items_v1_runs_run_id_items_get.return_value = [] + + list(app_run.results()) + + assert mock_api.list_run_items_v1_runs_run_id_items_get.call_args.kwargs["share_token"] is None + + +@pytest.mark.unit +def test_artifact_get_download_url_appends_share_token(configured_api, record_property) -> None: + """Artifact.get_download_url() adds the share_token as a query parameter on the /file URL.""" + record_property("tested-item-id", "PYSDK-145") + art = Artifact(configured_api, _RUN_ID, _ARTIFACT_ID, share_token="s3cr3t") # noqa: S106 + response = _redirect_response(_PRESIGNED_URL) + + with patch(_PATCH_GET_TOKEN, return_value="t"), patch(_PATCH_REQUESTS_GET, return_value=response) as mock_get: + art.get_download_url() + + assert "share_token=s3cr3t" in mock_get.call_args.args[0] + + +@pytest.mark.unit +def test_artifact_get_download_url_percent_encodes_share_token(configured_api, record_property) -> None: + """A share_token with reserved characters is percent-encoded, not injected as extra query params.""" + record_property("tested-item-id", "PYSDK-145") + art = Artifact(configured_api, _RUN_ID, _ARTIFACT_ID, share_token="a b&x=1") # noqa: S106 + response = _redirect_response(_PRESIGNED_URL) + + with patch(_PATCH_GET_TOKEN, return_value="t"), patch(_PATCH_REQUESTS_GET, return_value=response) as mock_get: + art.get_download_url() + + url = mock_get.call_args.args[0] + assert "share_token=a+b%26x%3D1" in url + # The raw "&" must not have created a second query parameter. + assert url.count("?") == 1 + assert "&" not in url.split("?", 1)[1] + + +@pytest.mark.unit +def test_artifact_get_download_url_sends_bearer_with_share_token(configured_api, record_property) -> None: + """The /file request always sends the OAuth Bearer header alongside the share_token query param.""" + record_property("tested-item-id", "PYSDK-145") + art = Artifact(configured_api, _RUN_ID, _ARTIFACT_ID, share_token="s3cr3t") # noqa: S106 + response = _redirect_response(_PRESIGNED_URL) + + with patch(_PATCH_GET_TOKEN, return_value="t"), patch(_PATCH_REQUESTS_GET, return_value=response) as mock_get: + art.get_download_url() + + assert mock_get.call_args.kwargs["headers"]["Authorization"] == "Bearer t" + assert "share_token=s3cr3t" in mock_get.call_args.args[0] + + +@pytest.mark.unit +def test_for_run_id_with_share_token_uses_authenticated_client(mock_api, record_property) -> None: + """Run.for_run_id(share_token=...) uses the normal authenticated client and stores the token.""" + record_property("tested-item-id", "PYSDK-145") + with patch("aignostics.platform._client.Client.get_api_client", return_value=mock_api) as mock_get_api_client: + run = Run.for_run_id("run-abc", share_token="s3cr3t") # noqa: S106 + + mock_get_api_client.assert_called_once_with(cache_token=True) + assert run._share_token == "s3cr3t" # noqa: S105 + + +@pytest.mark.unit +def test_for_run_id_without_share_token_uses_oauth(mock_api, record_property) -> None: + """Run.for_run_id() without a share token uses the normal cached OAuth client.""" + record_property("tested-item-id", "PYSDK-145") + with patch("aignostics.platform._client.Client.get_api_client", return_value=mock_api) as mock_get_api_client: + run = Run.for_run_id("run-abc", cache_token=True) + + mock_get_api_client.assert_called_once_with(cache_token=True) + assert run._share_token is None diff --git a/uv.lock b/uv.lock index 35106bc28..754d8a224 100644 --- a/uv.lock +++ b/uv.lock @@ -60,7 +60,6 @@ dependencies = [ { name = "humanize" }, { name = "idc-index-data" }, { name = "ijson" }, - { name = "joserfc" }, { name = "jsf" }, { name = "jsonschema", extra = ["format-nongpl"] }, { name = "loguru" }, @@ -85,9 +84,7 @@ dependencies = [ { name = "pygments" }, { name = "pyjwt", extra = ["crypto"] }, { name = "python-dateutil" }, - { name = "python-engineio" }, { name = "python-multipart" }, - { name = "python-socketio" }, { name = "pywin32", marker = "sys_platform == 'win32'" }, { name = "pyyaml" }, { name = "requests" }, @@ -207,7 +204,6 @@ requires-dist = [ { name = "idc-index-data", specifier = "==24.0.3" }, { name = "ijson", specifier = ">=3.4.0.post0,<4" }, { name = "ipython", marker = "extra == 'marimo'", specifier = ">=9.8.0,<10" }, - { name = "joserfc", specifier = ">=1.6.7" }, { name = "jsf", specifier = ">=0.11.2,<1" }, { name = "jsonschema", extras = ["format-nongpl"], specifier = ">=4.25.1,<5" }, { name = "jupyter", marker = "extra == 'jupyter'", specifier = ">=1.1.1,<2" }, @@ -241,9 +237,7 @@ requires-dist = [ { name = "pyinstaller", marker = "extra == 'pyinstaller'", specifier = ">=6.14.0,<7" }, { name = "pyjwt", extras = ["crypto"], specifier = ">=2.13.0,<3" }, { name = "python-dateutil", specifier = ">=2.9.0.post0,<3" }, - { name = "python-engineio", specifier = ">=4.13.3" }, { name = "python-multipart", specifier = ">=0.0.26" }, - { name = "python-socketio", specifier = ">=5.16.3" }, { name = "pywin32", marker = "sys_platform == 'win32'", specifier = ">=312,<313" }, { name = "pyyaml", specifier = ">=6.0.3,<7" }, { name = "requests", specifier = ">=2.33.0,<3" }, From 71dc1733a94ed25e6fb267c86e4067e92f4d3f28 Mon Sep 17 00:00:00 2001 From: Oliver Meyer Date: Wed, 5 Aug 2026 11:20:30 +0200 Subject: [PATCH 4/6] fix(application): correct share-token docstrings and 403 test source Address code-review findings on PR #688: - Fix two _service.py docstrings that wrongly described share-token access as "without OAuth"/"unauthenticated"; share tokens elevate an already OAuth-authenticated user's access. - Rewrite the three forbidden CLI tests to raise ForbiddenException from the real source (run.details()/run.results()) instead of application_run, which wraps all exceptions into RuntimeError in production. - Normalize an empty --share-token to None at the application_run choke point so a blank value falls back to the normal authenticated read; add service tests for both branches. - Make share_token_access_denied_message a pure builder and move the warning log to the four CLI call sites (command-query separation). Co-Authored-By: Claude Opus 4.8 --- src/aignostics/application/_cli.py | 4 +++ src/aignostics/application/_service.py | 13 ++++++-- src/aignostics/application/_utils.py | 9 +++-- tests/aignostics/application/cli_test.py | 35 ++++++++++++++++---- tests/aignostics/application/service_test.py | 30 +++++++++++++++++ 5 files changed, 77 insertions(+), 14 deletions(-) diff --git a/src/aignostics/application/_cli.py b/src/aignostics/application/_cli.py index 65f84cc4c..14570a42c 100644 --- a/src/aignostics/application/_cli.py +++ b/src/aignostics/application/_cli.py @@ -1003,6 +1003,7 @@ def run_describe( # noqa: PLR0912 console.print(f"[warning]Warning:[/warning] Run with ID '{run_id}' not found.") sys.exit(2) except ForbiddenException: + logger.warning("Access denied for run '{}'", run_id) msg = share_token_access_denied_message(run_id, share_token) if format == "json": print(json.dumps({"error": "access_denied", "message": msg}), file=sys.stderr) @@ -1059,6 +1060,7 @@ def run_dump_metadata( console.print(f"[warning]Warning:[/warning] Run with ID '{run_id}' not found.") sys.exit(2) except ForbiddenException: + logger.warning("Access denied for run '{}'", run_id) console.print(f"[error]Error:[/error] {share_token_access_denied_message(run_id, share_token)}") sys.exit(1) except Exception as e: @@ -1126,6 +1128,7 @@ def run_dump_item_metadata( print(f"Warning: Run with ID '{run_id}' not found.", file=sys.stderr) sys.exit(2) except ForbiddenException: + logger.warning("Access denied for run '{}'", run_id) print(f"Error: {share_token_access_denied_message(run_id, share_token)}", file=sys.stderr) sys.exit(1) except Exception as e: @@ -1848,6 +1851,7 @@ def update_progress(progress: DownloadProgress) -> None: # noqa: C901 console.print(f"[warning]Warning:[/warning] Bad input to download results of run with ID '{run_id}': {e}") sys.exit(2) except ForbiddenException: + logger.warning("Access denied for run '{}'", run_id) console.print(f"[error]Error:[/error] {share_token_access_denied_message(run_id, share_token)}") sys.exit(1) except Exception as e: diff --git a/src/aignostics/application/_service.py b/src/aignostics/application/_service.py index 776f2f7df..554edc6e0 100644 --- a/src/aignostics/application/_service.py +++ b/src/aignostics/application/_service.py @@ -805,8 +805,10 @@ def application_run(self, run_id: str, share_token: str | None = None) -> Run: Args: run_id (str): The ID of the run to find. - share_token (str | None): Optional share token secret. When provided the run - is accessed via the ``share_token`` query parameter without OAuth. + share_token (str | None): Optional share token secret. When provided the run is + accessed with the ``share_token`` forwarded as a query parameter, elevating + the calling (OAuth-authenticated) user's access to a run shared with them. + The caller must still be authenticated. An empty string is treated as absent. Returns: Run: The run that can be fetched using the .details() call. @@ -814,6 +816,9 @@ def application_run(self, run_id: str, share_token: str | None = None) -> Run: Raises: RuntimeError: If initializing the client fails or the run cannot be retrieved. """ + # Treat an empty --share-token the same as "not supplied" so a blank value falls back + # to the normal authenticated read instead of forwarding an empty share_token query param. + share_token = share_token or None try: if share_token is not None: return Run.for_run_id(run_id, share_token=share_token) @@ -1696,7 +1701,9 @@ def application_run_download( # noqa: C901, PLR0912, PLR0913, PLR0915 of the destination directory. download_progress_queue (Queue | None): Queue for GUI progress updates. download_progress_callable (Callable | None): Callback for CLI progress updates. - share_token (str | None): Optional share token secret for unauthenticated access. + share_token (str | None): Optional share token secret forwarded as the + ``share_token`` query parameter, elevating the authenticated caller's access + to a run shared with them. OAuth authentication is still required. Returns: Path: The directory containing downloaded results. diff --git a/src/aignostics/application/_utils.py b/src/aignostics/application/_utils.py index 9cfd0cc3f..d5a61284a 100644 --- a/src/aignostics/application/_utils.py +++ b/src/aignostics/application/_utils.py @@ -602,11 +602,11 @@ def get_supported_extensions_for_application(application_id: str) -> set[str]: def share_token_access_denied_message(run_id: str, share_token: str | None) -> str: - """Compose the operator-facing "access denied" message for a run and log a warning. + """Compose the operator-facing "access denied" message for a run. - Centralizes the wording, share-token hint, and warning log shared by the run CLI - commands that support ``--share-token``. The caller owns the output sink (console, - stderr, or JSON) and the exit code. + Pure message builder: it centralizes the wording and the share-token hint shared by + the run CLI commands that support ``--share-token``. It has no side effects — the + caller owns logging, the output sink (console, stderr, or JSON), and the exit code. Args: run_id (str): The run access was denied for. @@ -616,7 +616,6 @@ def share_token_access_denied_message(run_id: str, share_token: str | None) -> s Returns: str: The composed message. """ - logger.warning("Access denied for run '{}'", run_id) message = f"Access denied for run '{run_id}'." if share_token is not None: message += " The share token may be invalid, expired, or revoked." diff --git a/tests/aignostics/application/cli_test.py b/tests/aignostics/application/cli_test.py index efaf706e6..73de8e89c 100644 --- a/tests/aignostics/application/cli_test.py +++ b/tests/aignostics/application/cli_test.py @@ -1061,14 +1061,23 @@ def test_cli_run_describe_with_share_token_not_found(runner: CliRunner, record_p @pytest.mark.integration def test_cli_run_describe_with_share_token_forbidden(runner: CliRunner, record_property: object) -> None: - """Run describe --share-token exits 1 when token is invalid, expired, or has no access.""" + """Run describe --share-token exits 1 when token is invalid, expired, or has no access. + + The 403 is raised from run.details() (the real network-call source), not from + application_run() which wraps all exceptions into RuntimeError and can never raise a + bare ForbiddenException in production. + """ record_property("tested-item-id", "PYSDK-145") + mock_run = MagicMock() + mock_run.details.side_effect = ForbiddenException(status=403, reason="Forbidden") + with patch(APPLICATION_CLI_SERVICE_PATCH_TARGET) as mock_svc_cls: - mock_svc_cls.return_value.application_run.side_effect = ForbiddenException(status=403, reason="Forbidden") + mock_svc_cls.return_value.application_run.return_value = mock_run result = runner.invoke(cli, ["application", "run", "describe", "run-id", "--share-token", "bad-token"]) assert result.exit_code == 1 assert "Access denied" in normalize_output(result.output) + assert "bad-token" not in result.output # the token secret must never be echoed back @pytest.mark.integration @@ -1088,10 +1097,17 @@ def test_cli_run_dump_metadata_with_share_token_success(runner: CliRunner, recor @pytest.mark.integration def test_cli_run_dump_metadata_with_share_token_forbidden(runner: CliRunner, record_property: object) -> None: - """Run dump-metadata --share-token exits 1 on forbidden.""" + """Run dump-metadata --share-token exits 1 on forbidden. + + The 403 is raised from run.details() (the real source) rather than from application_run, + which wraps exceptions into RuntimeError in production. + """ record_property("tested-item-id", "PYSDK-145") + mock_run = MagicMock() + mock_run.details.side_effect = ForbiddenException(status=403, reason="Forbidden") + with patch(APPLICATION_CLI_SERVICE_PATCH_TARGET) as mock_svc_cls: - mock_svc_cls.return_value.application_run.side_effect = ForbiddenException(status=403, reason="Forbidden") + mock_svc_cls.return_value.application_run.return_value = mock_run result = runner.invoke(cli, ["application", "run", "dump-metadata", "run-id", "--share-token", "bad"]) assert result.exit_code == 1 @@ -1123,10 +1139,17 @@ def test_cli_run_dump_item_metadata_with_share_token_success(runner: CliRunner, @pytest.mark.integration def test_cli_run_dump_item_metadata_with_share_token_forbidden(runner: CliRunner, record_property: object) -> None: - """Run dump-item-metadata --share-token exits 1 on forbidden.""" + """Run dump-item-metadata --share-token exits 1 on forbidden. + + The 403 is raised from run.results() (the real source iterated by the command) rather + than from application_run, which wraps exceptions into RuntimeError in production. + """ record_property("tested-item-id", "PYSDK-145") + mock_run = MagicMock() + mock_run.results.side_effect = ForbiddenException(status=403, reason="Forbidden") + with patch(APPLICATION_CLI_SERVICE_PATCH_TARGET) as mock_svc_cls: - mock_svc_cls.return_value.application_run.side_effect = ForbiddenException(status=403, reason="Forbidden") + mock_svc_cls.return_value.application_run.return_value = mock_run result = runner.invoke( cli, ["application", "run", "dump-item-metadata", "run-id", "item-id", "--share-token", "bad"] ) diff --git a/tests/aignostics/application/service_test.py b/tests/aignostics/application/service_test.py index 5b5b4ec75..a44141e6f 100644 --- a/tests/aignostics/application/service_test.py +++ b/tests/aignostics/application/service_test.py @@ -1088,3 +1088,33 @@ def test_application_run_download_reraises_forbidden(tmp_path, record_property: pytest.raises(ForbiddenException), ): ApplicationService().application_run_download("run-id", tmp_path, share_token="s3cr3t") # noqa: S106 + + +@pytest.mark.unit +@patch("aignostics.application._service.Run.for_run_id") +@patch("aignostics.application._service.Service._get_platform_client") +def test_application_run_empty_share_token_uses_oauth_path( + mock_get_client: MagicMock, mock_for_run_id: MagicMock, record_property: object +) -> None: + """An empty --share-token is normalized to None and takes the normal authenticated path.""" + record_property("tested-item-id", "PYSDK-145") + + ApplicationService().application_run("run-id", share_token="") + + mock_get_client.return_value.run.assert_called_once_with("run-id") + mock_for_run_id.assert_not_called() + + +@pytest.mark.unit +@patch("aignostics.application._service.Run.for_run_id") +@patch("aignostics.application._service.Service._get_platform_client") +def test_application_run_with_share_token_uses_share_token_path( + mock_get_client: MagicMock, mock_for_run_id: MagicMock, record_property: object +) -> None: + """A non-empty share token takes the Run.for_run_id path and is forwarded verbatim.""" + record_property("tested-item-id", "PYSDK-145") + + ApplicationService().application_run("run-id", share_token="s3cr3t") # noqa: S106 + + mock_for_run_id.assert_called_once_with("run-id", share_token="s3cr3t") # noqa: S106 + mock_get_client.return_value.run.assert_not_called() From 0f3760967a5da738b177c5aebcba26923f83d80b Mon Sep 17 00:00:00 2001 From: Oliver Meyer Date: Thu, 6 Aug 2026 10:21:09 +0200 Subject: [PATCH 5/6] docs: update docs for run sharing --- requirements/SHR-APPLICATION-4.md | 3 +- requirements/SWR-APPLICATION-4-3.md | 10 +++ specifications/SPEC-APPLICATION-SERVICE.md | 10 ++- specifications/SPEC_PLATFORM_SERVICE.md | 10 ++- .../application/TC-APPLICATION-CLI-08.feature | 79 +++++++++++++++++++ tests/aignostics/application/cli_test.py | 28 +++---- tests/aignostics/application/service_test.py | 6 +- .../platform/resources/runs_test.py | 18 ++--- 8 files changed, 131 insertions(+), 33 deletions(-) create mode 100644 requirements/SWR-APPLICATION-4-3.md create mode 100644 tests/aignostics/application/TC-APPLICATION-CLI-08.feature diff --git a/requirements/SHR-APPLICATION-4.md b/requirements/SHR-APPLICATION-4.md index ba4574352..ed7603d09 100644 --- a/requirements/SHR-APPLICATION-4.md +++ b/requirements/SHR-APPLICATION-4.md @@ -8,4 +8,5 @@ Requirement type: ENVIRONMENT ## Description Users shall be able to share access to application runs with other authenticated platform users, and shall be able to -manage (list and revoke) the access grants they have created. +manage (list and revoke) the access grants they have created. Users with whom a run has been shared shall be able to +read that run — its status, results, and metadata — using the access granted to them. diff --git a/requirements/SWR-APPLICATION-4-3.md b/requirements/SWR-APPLICATION-4-3.md new file mode 100644 index 000000000..c80a1f4a0 --- /dev/null +++ b/requirements/SWR-APPLICATION-4-3.md @@ -0,0 +1,10 @@ +--- +itemId: SWR-APPLICATION-4-3 +itemTitle: Read a Shared Application Run via Share Token +itemHasParent: SHR-APPLICATION-4 +itemType: Requirement +Requirement type: FUNCTIONAL +Layer: System (backend logic) +--- + +System shall enable an authenticated user who holds a share token secret to read an application run shared with them through the CLI, without requiring the run to have been granted to their account directly. The user shall be able to retrieve run status and details, dump run and item custom metadata, and download run results by supplying the share token secret; OAuth authentication remains required and the share token elevates the authenticated user's access to the shared run. When access is denied because the token is invalid, expired, or revoked, the system shall report a clear access-denied message and exit with code 1; when the run does not exist, the system shall exit with code 2. When no share token is supplied, the commands shall behave exactly as for a normal authenticated read. diff --git a/specifications/SPEC-APPLICATION-SERVICE.md b/specifications/SPEC-APPLICATION-SERVICE.md index f21b58aa0..b056cee50 100644 --- a/specifications/SPEC-APPLICATION-SERVICE.md +++ b/specifications/SPEC-APPLICATION-SERVICE.md @@ -2,11 +2,11 @@ itemId: SPEC-APPLICATION-SERVICE itemTitle: Application Module Specification itemType: Software Item Spec -itemFulfills: SWR-APPLICATION-1-1, SWR-APPLICATION-1-2, SWR-APPLICATION-1-3, SWR-APPLICATION-2-3, SWR-APPLICATION-2-4, SHR-APPLICATION-3, SWR-APPLICATION-2-12, SWR-APPLICATION-2-11, SWR-APPLICATION-2-13, SWR-APPLICATION-2-14, SWR-APPLICATION-2-15, SWR-APPLICATION-2-16, SWR-APPLICATION-2-17, SWR-APPLICATION-2-5, SWR-APPLICATION-2-7, SWR-APPLICATION-2-8, SWR-APPLICATION-2-9, SWR-APPLICATION-3-3 +itemFulfills: SWR-APPLICATION-1-1, SWR-APPLICATION-1-2, SWR-APPLICATION-1-3, SWR-APPLICATION-2-3, SWR-APPLICATION-2-4, SHR-APPLICATION-3, SWR-APPLICATION-2-12, SWR-APPLICATION-2-11, SWR-APPLICATION-2-13, SWR-APPLICATION-2-14, SWR-APPLICATION-2-15, SWR-APPLICATION-2-16, SWR-APPLICATION-2-17, SWR-APPLICATION-2-5, SWR-APPLICATION-2-7, SWR-APPLICATION-2-8, SWR-APPLICATION-2-9, SWR-APPLICATION-3-3, SWR-APPLICATION-4-3 Module: Application Layer: Domain Service Version: 0.2.107 -Date: 2026-04-29 +Date: 2026-08-06 --- ## 1. Description @@ -28,6 +28,7 @@ The Application Module shall: - **FR-05** **Result Download**: Progressive download of analysis results with resumable operations and organized directory hierarchies - **FR-06** **QuPath Integration**: Automatic QuPath project creation with downloaded results for pathology analysis - **FR-07** **Multi-Modal Interface**: Provide CLI, GUI, and programmatic interfaces for different user workflows +- **FR-08** **Shared Run Access via Share Token**: Read a run shared with you via `--share-token` (status, metadata, result download); denied token exits 1, missing run exits 2, no token behaves as before ### 1.3 Non-Functional Requirements @@ -481,6 +482,9 @@ uvx aignostics application [subcommand] [options] `--checksum` guards the write with optimistic concurrency control (exit code 3 on conflict); `--enrich-sdk-metadata / --no-enrich-sdk-metadata` (default enrich) controls whether the SDK merges auto-generated tracking context into the `sdk` field or forwards it verbatim +- `--share-token ` (on `run describe`, `run dump-metadata`, `run dump-item-metadata`, + `run result download`): read a run shared with you; OAuth login still required. Denied token + exits 1, missing run exits 2; omitted behaves as before ### 4.3 GUI Interface @@ -558,7 +562,7 @@ Configuration is managed through environment variables with the prefix `AIGNOSTI | `NotFoundException` | Missing runs or applications | Graceful rejection with info | Clear resource not found info | | `FileNotFoundError` | Missing input files | File validation before upload | File path verification help | | `ApiException` | Platform API failures | Retry mechanism with recovery | API error details and guidance | -| `ForbiddenException` | Caller not authorized for the requested org | Caught in CLI; exit 2 with access-denied message | User informed they lack permission | +| `ForbiddenException` | Not authorized for the org, or share-token read denied | Caught in CLI; org denial exits 2, share-token denial exits 1 (token never echoed) | Lacks permission / token may be invalid, expired, or revoked | | `ConcurrencyConflictError` | Custom-metadata update rejected (HTTP 412): metadata modified since the checksum was read | `ValueError` subclass; caught in CLI, exit 3 | User told to re-read and retry the update | ### 7.2 Input Validation diff --git a/specifications/SPEC_PLATFORM_SERVICE.md b/specifications/SPEC_PLATFORM_SERVICE.md index 1fe3b7a0a..2fc8bb5cb 100644 --- a/specifications/SPEC_PLATFORM_SERVICE.md +++ b/specifications/SPEC_PLATFORM_SERVICE.md @@ -2,11 +2,11 @@ itemId: SPEC-PLATFORM-SERVICE itemTitle: Platform Module Specification itemType: Software Item Spec -itemFulfills: SWR-APPLICATION-1-1, SWR-APPLICATION-1-2, SWR-APPLICATION-1-3, SWR-APPLICATION-2-1, SWR-APPLICATION-2-5, SWR-APPLICATION-2-6, SWR-APPLICATION-2-7, SWR-APPLICATION-2-9, SWR-APPLICATION-2-14, SWR-APPLICATION-2-15, SWR-APPLICATION-2-16, SWR-APPLICATION-2-17, SWR-APPLICATION-3-1, SWR-APPLICATION-3-2, SWR-APPLICATION-3-3, SWR-APPLICATION-4-1, SWR-APPLICATION-4-2 +itemFulfills: SWR-APPLICATION-1-1, SWR-APPLICATION-1-2, SWR-APPLICATION-1-3, SWR-APPLICATION-2-1, SWR-APPLICATION-2-5, SWR-APPLICATION-2-6, SWR-APPLICATION-2-7, SWR-APPLICATION-2-9, SWR-APPLICATION-2-14, SWR-APPLICATION-2-15, SWR-APPLICATION-2-16, SWR-APPLICATION-2-17, SWR-APPLICATION-3-1, SWR-APPLICATION-3-2, SWR-APPLICATION-3-3, SWR-APPLICATION-4-1, SWR-APPLICATION-4-2, SWR-APPLICATION-4-3 Module: Platform Layer: Platform Service Version: 1.2.0 -Date: 2026-06-09 +Date: 2026-08-06 --- ## 1. Description @@ -33,6 +33,7 @@ The Platform Module shall: - **[FR-12]** Generate signed URLs for secure Google Cloud Storage access - **[FR-13]** Provide user and organization information retrieval with sensitive data masking options - **[FR-14]** Support external token providers to bypass internal OAuth 2.0 flows for machine-to-machine, service account, or custom token lifecycle scenarios. +- **[FR-15]** Forward an optional share token (percent-encoded) as the `share_token` query parameter on the run read endpoints while always sending the OAuth Bearer, isolate it in the operation-cache key, and propagate `ForbiddenException` unchanged for caller-side handling ### 1.3 Non-Functional Requirements @@ -717,6 +718,9 @@ class Artifact: ``allow_redirects=False`` and returns the presigned URL from the redirect ``Location`` header. The presigned URL is short-lived; resolve immediately before downloading. + + A share token, when present, is appended as the URL-encoded ``share_token`` + query parameter (Bearer still sent). """ ``` @@ -846,7 +850,7 @@ The Platform module provides foundational services but does not directly expose | `NetworkError` | Connection timeouts or proxy issues | Retry with backoff; fallback to device flow | Automatic retry or alternative auth flow | | `TokenExpiredError` | JWT token past expiration | Automatic refresh using refresh token | Transparent token renewal | | `ValidationError` | Invalid input parameters or file formats | Input sanitization and validation | Clear validation error messages | -| `ForbiddenException` | Caller not authorized for the requested org | Caught in CLI; exit 2 with access-denied message | User informed they lack permission | +| `ForbiddenException` | Not authorized for the org, or share-token read denied | Propagated unchanged; org denial exits 2, share-token denial exits 1 | Lacks permission / token may be invalid, expired, or revoked | ### 7.2 Input Validation diff --git a/tests/aignostics/application/TC-APPLICATION-CLI-08.feature b/tests/aignostics/application/TC-APPLICATION-CLI-08.feature new file mode 100644 index 000000000..b731fd85e --- /dev/null +++ b/tests/aignostics/application/TC-APPLICATION-CLI-08.feature @@ -0,0 +1,79 @@ +Feature: Read a Shared Application Run via Share Token + + The system allows an authenticated user who holds a share token secret to read + an application run shared with them via the CLI — retrieving run status and + details, dumping run and item custom metadata, and downloading results — by + supplying the token as a command option. OAuth authentication remains required; + the share token elevates the authenticated user's access to the shared run. + + @tests:SPEC-APPLICATION-SERVICE + @tests:SPEC-PLATFORM-SERVICE + @tests:SWR-APPLICATION-4-3 + @id:TC-APPLICATION-CLI-08-01 + Scenario: System describes a shared run when a valid share token is supplied + Given a run has been shared with the authenticated user via a share token + When the user runs the describe command with the run identifier and the share token + Then the system shall return the run details identically to the authenticated path + + @tests:SPEC-APPLICATION-SERVICE + @tests:SPEC-PLATFORM-SERVICE + @tests:SWR-APPLICATION-4-3 + @id:TC-APPLICATION-CLI-08-02 + Scenario: System dumps run custom metadata when a valid share token is supplied + Given a run has been shared with the authenticated user via a share token + When the user runs the dump-metadata command with the run identifier and the share token + Then the system shall emit the run's custom metadata identically to the authenticated path + + @tests:SPEC-APPLICATION-SERVICE + @tests:SPEC-PLATFORM-SERVICE + @tests:SWR-APPLICATION-4-3 + @id:TC-APPLICATION-CLI-08-03 + Scenario: System dumps item custom metadata when a valid share token is supplied + Given a run has been shared with the authenticated user via a share token + When the user runs the dump-item-metadata command with the run identifier, an item external identifier, and the share token + Then the system shall emit the item's custom metadata identically to the authenticated path + + @tests:SPEC-APPLICATION-SERVICE + @tests:SPEC-PLATFORM-SERVICE + @tests:SWR-APPLICATION-4-3 + @id:TC-APPLICATION-CLI-08-04 + Scenario: System downloads shared run results when a valid share token is supplied + Given a run has been shared with the authenticated user via a share token + When the user runs the result download command with the run identifier, a destination directory, and the share token + Then the system shall download the run results identically to the authenticated path + + @tests:SPEC-APPLICATION-SERVICE + @tests:SPEC-PLATFORM-SERVICE + @tests:SWR-APPLICATION-4-3 + @id:TC-APPLICATION-CLI-08-05 + Scenario: System denies access when the share token is invalid, expired, or revoked + Given the user supplies a share token that is invalid, expired, or revoked + When the user runs a share-token read command for the run + Then the system shall report an access-denied message hinting the token may be invalid, expired, or revoked + And the system shall exit with code 1 + + @tests:SPEC-APPLICATION-SERVICE + @tests:SPEC-PLATFORM-SERVICE + @tests:SWR-APPLICATION-4-3 + @id:TC-APPLICATION-CLI-08-06 + Scenario: System reports run-not-found distinctly from access denied + Given the user supplies a share token for a run identifier that does not exist + When the user runs a share-token read command for that run + Then the system shall report the run as not found and exit with code 2 + + @tests:SPEC-APPLICATION-SERVICE + @tests:SPEC-PLATFORM-SERVICE + @tests:SWR-APPLICATION-4-3 + @id:TC-APPLICATION-CLI-08-07 + Scenario: System preserves authenticated behaviour when no share token is supplied + Given the user has direct access to an application run + When the user runs a read command without a share token + Then the system shall behave exactly as before, passing no share token to the platform + + @tests:SPEC-PLATFORM-SERVICE + @tests:SWR-APPLICATION-4-3 + @id:TC-APPLICATION-CLI-08-08 + Scenario: System propagates a forbidden error from the download path without wrapping it + Given a share-token download is denied by the platform with a forbidden response + When the download path handles the forbidden response + Then the system shall propagate the forbidden error unchanged rather than wrapping it into a generic runtime error diff --git a/tests/aignostics/application/cli_test.py b/tests/aignostics/application/cli_test.py index 73de8e89c..f4c9ce0b8 100644 --- a/tests/aignostics/application/cli_test.py +++ b/tests/aignostics/application/cli_test.py @@ -1015,7 +1015,7 @@ def _make_mock_run(run_id: str = "run-shared-001", custom_metadata: dict | None @pytest.mark.integration def test_cli_run_describe_with_share_token_success(runner: CliRunner, record_property: object) -> None: """Run describe --share-token succeeds without OAuth and returns run details.""" - record_property("tested-item-id", "PYSDK-145") + record_property("tested-item-id", "TC-APPLICATION-CLI-08-01, SWR-APPLICATION-4-3") mock_run = _make_mock_run("run-shared-001") with patch(APPLICATION_CLI_SERVICE_PATCH_TARGET) as mock_svc_cls: @@ -1030,7 +1030,7 @@ def test_cli_run_describe_with_share_token_success(runner: CliRunner, record_pro @pytest.mark.integration def test_cli_run_describe_with_share_token_json(runner: CliRunner, record_property: object) -> None: """Run describe --share-token --format json returns valid JSON without OAuth.""" - record_property("tested-item-id", "PYSDK-145") + record_property("tested-item-id", "TC-APPLICATION-CLI-08-01, SWR-APPLICATION-4-3") mock_run = _make_mock_run("run-shared-002") with patch(APPLICATION_CLI_SERVICE_PATCH_TARGET) as mock_svc_cls: @@ -1050,7 +1050,7 @@ def test_cli_run_describe_with_share_token_json(runner: CliRunner, record_proper @pytest.mark.integration def test_cli_run_describe_with_share_token_not_found(runner: CliRunner, record_property: object) -> None: """Run describe --share-token exits 2 when run does not exist.""" - record_property("tested-item-id", "PYSDK-145") + record_property("tested-item-id", "TC-APPLICATION-CLI-08-06, SWR-APPLICATION-4-3") with patch(APPLICATION_CLI_SERVICE_PATCH_TARGET) as mock_svc_cls: mock_svc_cls.return_value.application_run.side_effect = ApiNotFound(status=404, reason="Not Found") result = runner.invoke(cli, ["application", "run", "describe", "bad-run-id", "--share-token", "s3cr3t"]) @@ -1067,7 +1067,7 @@ def test_cli_run_describe_with_share_token_forbidden(runner: CliRunner, record_p application_run() which wraps all exceptions into RuntimeError and can never raise a bare ForbiddenException in production. """ - record_property("tested-item-id", "PYSDK-145") + record_property("tested-item-id", "TC-APPLICATION-CLI-08-05, SWR-APPLICATION-4-3") mock_run = MagicMock() mock_run.details.side_effect = ForbiddenException(status=403, reason="Forbidden") @@ -1083,7 +1083,7 @@ def test_cli_run_describe_with_share_token_forbidden(runner: CliRunner, record_p @pytest.mark.integration def test_cli_run_dump_metadata_with_share_token_success(runner: CliRunner, record_property: object) -> None: """Run dump-metadata --share-token returns custom metadata JSON without OAuth.""" - record_property("tested-item-id", "PYSDK-145") + record_property("tested-item-id", "TC-APPLICATION-CLI-08-02, SWR-APPLICATION-4-3") mock_run = _make_mock_run("run-001", custom_metadata={"key": "value"}) with patch(APPLICATION_CLI_SERVICE_PATCH_TARGET) as mock_svc_cls: @@ -1102,7 +1102,7 @@ def test_cli_run_dump_metadata_with_share_token_forbidden(runner: CliRunner, rec The 403 is raised from run.details() (the real source) rather than from application_run, which wraps exceptions into RuntimeError in production. """ - record_property("tested-item-id", "PYSDK-145") + record_property("tested-item-id", "TC-APPLICATION-CLI-08-05, SWR-APPLICATION-4-3") mock_run = MagicMock() mock_run.details.side_effect = ForbiddenException(status=403, reason="Forbidden") @@ -1117,7 +1117,7 @@ def test_cli_run_dump_metadata_with_share_token_forbidden(runner: CliRunner, rec @pytest.mark.integration def test_cli_run_dump_item_metadata_with_share_token_success(runner: CliRunner, record_property: object) -> None: """Run dump-item-metadata --share-token finds an item and returns its metadata without OAuth.""" - record_property("tested-item-id", "PYSDK-145") + record_property("tested-item-id", "TC-APPLICATION-CLI-08-03, SWR-APPLICATION-4-3") mock_item = MagicMock() mock_item.external_id = "slide-001.svs" mock_item.custom_metadata = {"slide": "meta"} @@ -1144,7 +1144,7 @@ def test_cli_run_dump_item_metadata_with_share_token_forbidden(runner: CliRunner The 403 is raised from run.results() (the real source iterated by the command) rather than from application_run, which wraps exceptions into RuntimeError in production. """ - record_property("tested-item-id", "PYSDK-145") + record_property("tested-item-id", "TC-APPLICATION-CLI-08-05, SWR-APPLICATION-4-3") mock_run = MagicMock() mock_run.results.side_effect = ForbiddenException(status=403, reason="Forbidden") @@ -1163,7 +1163,7 @@ def test_cli_result_download_with_share_token_passes_token_to_service( runner: CliRunner, tmp_path: Path, record_property: object ) -> None: """Result download --share-token forwards the token to application_run_download.""" - record_property("tested-item-id", "PYSDK-145") + record_property("tested-item-id", "TC-APPLICATION-CLI-08-04, SWR-APPLICATION-4-3") with patch(APPLICATION_CLI_SERVICE_PATCH_TARGET) as mock_svc_cls: mock_svc_cls.return_value.application_run_download.return_value = tmp_path result = runner.invoke( @@ -1181,7 +1181,7 @@ def test_cli_result_download_with_share_token_forbidden( runner: CliRunner, tmp_path: Path, record_property: object ) -> None: """Result download --share-token exits 1 on forbidden.""" - record_property("tested-item-id", "PYSDK-145") + record_property("tested-item-id", "TC-APPLICATION-CLI-08-05, SWR-APPLICATION-4-3") with patch(APPLICATION_CLI_SERVICE_PATCH_TARGET) as mock_svc_cls: mock_svc_cls.return_value.application_run_download.side_effect = ForbiddenException( status=403, reason="Forbidden" @@ -1198,7 +1198,7 @@ def test_cli_result_download_with_share_token_forbidden( @pytest.mark.integration def test_cli_run_describe_without_share_token_passes_none(runner: CliRunner, record_property: object) -> None: """Run describe without --share-token calls the service with share_token=None (no leakage).""" - record_property("tested-item-id", "PYSDK-145") + record_property("tested-item-id", "TC-APPLICATION-CLI-08-07, SWR-APPLICATION-4-3") mock_run = _make_mock_run("run-noshare") with patch(APPLICATION_CLI_SERVICE_PATCH_TARGET) as mock_svc_cls: @@ -1212,7 +1212,7 @@ def test_cli_run_describe_without_share_token_passes_none(runner: CliRunner, rec @pytest.mark.integration def test_cli_run_dump_metadata_with_share_token_not_found(runner: CliRunner, record_property: object) -> None: """Run dump-metadata --share-token exits 2 when run does not exist.""" - record_property("tested-item-id", "PYSDK-145") + record_property("tested-item-id", "TC-APPLICATION-CLI-08-06, SWR-APPLICATION-4-3") with patch(APPLICATION_CLI_SERVICE_PATCH_TARGET) as mock_svc_cls: mock_svc_cls.return_value.application_run.side_effect = ApiNotFound(status=404, reason="Not Found") result = runner.invoke(cli, ["application", "run", "dump-metadata", "bad-run-id", "--share-token", "s3cr3t"]) @@ -1224,7 +1224,7 @@ def test_cli_run_dump_metadata_with_share_token_not_found(runner: CliRunner, rec @pytest.mark.integration def test_cli_run_dump_item_metadata_with_share_token_not_found(runner: CliRunner, record_property: object) -> None: """Run dump-item-metadata --share-token exits 2 when run does not exist.""" - record_property("tested-item-id", "PYSDK-145") + record_property("tested-item-id", "TC-APPLICATION-CLI-08-06, SWR-APPLICATION-4-3") with patch(APPLICATION_CLI_SERVICE_PATCH_TARGET) as mock_svc_cls: mock_svc_cls.return_value.application_run.side_effect = ApiNotFound(status=404, reason="Not Found") result = runner.invoke( @@ -1240,7 +1240,7 @@ def test_cli_result_download_with_share_token_not_found( runner: CliRunner, tmp_path: Path, record_property: object ) -> None: """Result download --share-token exits 2 when run does not exist.""" - record_property("tested-item-id", "PYSDK-145") + record_property("tested-item-id", "TC-APPLICATION-CLI-08-06, SWR-APPLICATION-4-3") with patch(APPLICATION_CLI_SERVICE_PATCH_TARGET) as mock_svc_cls: mock_svc_cls.return_value.application_run_download.side_effect = ApiNotFound(status=404, reason="Not Found") result = runner.invoke( diff --git a/tests/aignostics/application/service_test.py b/tests/aignostics/application/service_test.py index a44141e6f..07c2ff6f0 100644 --- a/tests/aignostics/application/service_test.py +++ b/tests/aignostics/application/service_test.py @@ -1079,7 +1079,7 @@ def test_application_run_download_reraises_forbidden(tmp_path, record_property: Guards the CLI's share-token 'access denied' handler: the download path must not swallow ForbiddenException into RuntimeError via its generic ApiException branch. """ - record_property("tested-item-id", "PYSDK-145") + record_property("tested-item-id", "TC-APPLICATION-CLI-08-08, SWR-APPLICATION-4-3") mock_run = MagicMock() mock_run.details.side_effect = ForbiddenException(status=403, reason="Forbidden") @@ -1097,7 +1097,7 @@ def test_application_run_empty_share_token_uses_oauth_path( mock_get_client: MagicMock, mock_for_run_id: MagicMock, record_property: object ) -> None: """An empty --share-token is normalized to None and takes the normal authenticated path.""" - record_property("tested-item-id", "PYSDK-145") + record_property("tested-item-id", "TC-APPLICATION-CLI-08-07, SWR-APPLICATION-4-3") ApplicationService().application_run("run-id", share_token="") @@ -1112,7 +1112,7 @@ def test_application_run_with_share_token_uses_share_token_path( mock_get_client: MagicMock, mock_for_run_id: MagicMock, record_property: object ) -> None: """A non-empty share token takes the Run.for_run_id path and is forwarded verbatim.""" - record_property("tested-item-id", "PYSDK-145") + record_property("tested-item-id", "TC-APPLICATION-CLI-08-01, SWR-APPLICATION-4-3, SPEC-APPLICATION-SERVICE") ApplicationService().application_run("run-id", share_token="s3cr3t") # noqa: S106 diff --git a/tests/aignostics/platform/resources/runs_test.py b/tests/aignostics/platform/resources/runs_test.py index 867cf1af5..53f37b498 100644 --- a/tests/aignostics/platform/resources/runs_test.py +++ b/tests/aignostics/platform/resources/runs_test.py @@ -1417,7 +1417,7 @@ def test_update_item_custom_metadata_no_enrich_skips_sdk_metadata_builders(app_r @pytest.mark.unit def test_details_forwards_share_token(mock_api, record_property) -> None: """Run.details() forwards a configured share_token to the get-run API call.""" - record_property("tested-item-id", "PYSDK-145") + record_property("tested-item-id", "SWR-APPLICATION-4-3, SPEC-PLATFORM-SERVICE") mock_api.get_run_v1_runs_run_id_get.return_value = RunReadResponse.model_construct(run_id="test-run-id") Run(mock_api, "test-run-id", share_token="s3cr3t").details() # noqa: S106 @@ -1428,7 +1428,7 @@ def test_details_forwards_share_token(mock_api, record_property) -> None: @pytest.mark.unit def test_details_share_token_none_when_not_supplied(app_run, mock_api, record_property) -> None: """Run.details() sends share_token=None when no share token was supplied (no leakage).""" - record_property("tested-item-id", "PYSDK-145") + record_property("tested-item-id", "TC-APPLICATION-CLI-08-07, SWR-APPLICATION-4-3, SPEC-PLATFORM-SERVICE") mock_api.get_run_v1_runs_run_id_get.return_value = RunReadResponse.model_construct(run_id="test-run-id") app_run.details() @@ -1439,7 +1439,7 @@ def test_details_share_token_none_when_not_supplied(app_run, mock_api, record_pr @pytest.mark.unit def test_results_forwards_share_token(mock_api, record_property) -> None: """Run.results() forwards a configured share_token to the list-items API call on every page.""" - record_property("tested-item-id", "PYSDK-145") + record_property("tested-item-id", "SWR-APPLICATION-4-3, SPEC-PLATFORM-SERVICE") mock_api.list_run_items_v1_runs_run_id_items_get.return_value = [] list(Run(mock_api, "test-run-id", share_token="s3cr3t").results()) # noqa: S106 @@ -1450,7 +1450,7 @@ def test_results_forwards_share_token(mock_api, record_property) -> None: @pytest.mark.unit def test_results_share_token_none_when_not_supplied(app_run, mock_api, record_property) -> None: """Run.results() sends share_token=None when no share token was supplied (no leakage).""" - record_property("tested-item-id", "PYSDK-145") + record_property("tested-item-id", "TC-APPLICATION-CLI-08-07, SWR-APPLICATION-4-3, SPEC-PLATFORM-SERVICE") mock_api.list_run_items_v1_runs_run_id_items_get.return_value = [] list(app_run.results()) @@ -1461,7 +1461,7 @@ def test_results_share_token_none_when_not_supplied(app_run, mock_api, record_pr @pytest.mark.unit def test_artifact_get_download_url_appends_share_token(configured_api, record_property) -> None: """Artifact.get_download_url() adds the share_token as a query parameter on the /file URL.""" - record_property("tested-item-id", "PYSDK-145") + record_property("tested-item-id", "SWR-APPLICATION-4-3, SPEC-PLATFORM-SERVICE") art = Artifact(configured_api, _RUN_ID, _ARTIFACT_ID, share_token="s3cr3t") # noqa: S106 response = _redirect_response(_PRESIGNED_URL) @@ -1474,7 +1474,7 @@ def test_artifact_get_download_url_appends_share_token(configured_api, record_pr @pytest.mark.unit def test_artifact_get_download_url_percent_encodes_share_token(configured_api, record_property) -> None: """A share_token with reserved characters is percent-encoded, not injected as extra query params.""" - record_property("tested-item-id", "PYSDK-145") + record_property("tested-item-id", "SWR-APPLICATION-4-3, SPEC-PLATFORM-SERVICE") art = Artifact(configured_api, _RUN_ID, _ARTIFACT_ID, share_token="a b&x=1") # noqa: S106 response = _redirect_response(_PRESIGNED_URL) @@ -1491,7 +1491,7 @@ def test_artifact_get_download_url_percent_encodes_share_token(configured_api, r @pytest.mark.unit def test_artifact_get_download_url_sends_bearer_with_share_token(configured_api, record_property) -> None: """The /file request always sends the OAuth Bearer header alongside the share_token query param.""" - record_property("tested-item-id", "PYSDK-145") + record_property("tested-item-id", "SWR-APPLICATION-4-3, SPEC-PLATFORM-SERVICE") art = Artifact(configured_api, _RUN_ID, _ARTIFACT_ID, share_token="s3cr3t") # noqa: S106 response = _redirect_response(_PRESIGNED_URL) @@ -1505,7 +1505,7 @@ def test_artifact_get_download_url_sends_bearer_with_share_token(configured_api, @pytest.mark.unit def test_for_run_id_with_share_token_uses_authenticated_client(mock_api, record_property) -> None: """Run.for_run_id(share_token=...) uses the normal authenticated client and stores the token.""" - record_property("tested-item-id", "PYSDK-145") + record_property("tested-item-id", "SWR-APPLICATION-4-3, SPEC-PLATFORM-SERVICE") with patch("aignostics.platform._client.Client.get_api_client", return_value=mock_api) as mock_get_api_client: run = Run.for_run_id("run-abc", share_token="s3cr3t") # noqa: S106 @@ -1516,7 +1516,7 @@ def test_for_run_id_with_share_token_uses_authenticated_client(mock_api, record_ @pytest.mark.unit def test_for_run_id_without_share_token_uses_oauth(mock_api, record_property) -> None: """Run.for_run_id() without a share token uses the normal cached OAuth client.""" - record_property("tested-item-id", "PYSDK-145") + record_property("tested-item-id", "TC-APPLICATION-CLI-08-07, SWR-APPLICATION-4-3, SPEC-PLATFORM-SERVICE") with patch("aignostics.platform._client.Client.get_api_client", return_value=mock_api) as mock_get_api_client: run = Run.for_run_id("run-abc", cache_token=True) From cd81e88f2f29f11a689e60612eea2e8488617add Mon Sep 17 00:00:00 2001 From: Oliver Meyer Date: Thu, 6 Aug 2026 11:41:43 +0200 Subject: [PATCH 6/6] refactor: address review comments --- src/aignostics/application/_cli.py | 12 +++++------ src/aignostics/application/_utils.py | 6 +----- src/aignostics/platform/resources/runs.py | 12 +++++------ .../platform/resources/runs_test.py | 21 ++----------------- 4 files changed, 14 insertions(+), 37 deletions(-) diff --git a/src/aignostics/application/_cli.py b/src/aignostics/application/_cli.py index 14570a42c..2a4266953 100644 --- a/src/aignostics/application/_cli.py +++ b/src/aignostics/application/_cli.py @@ -38,7 +38,7 @@ print_runs_verbose, read_metadata_csv_to_dict, retrieve_and_print_run_details, - share_token_access_denied_message, + run_access_denied_message, validate_mappings, write_metadata_dict_to_csv, ) @@ -961,7 +961,7 @@ def run_list( # noqa: PLR0913 @run_app.command("describe") -def run_describe( # noqa: PLR0912 +def run_describe( run_id: Annotated[str, typer.Argument(help="Id of the run to describe")], format: Annotated[ # noqa: A002 str, @@ -1004,7 +1004,7 @@ def run_describe( # noqa: PLR0912 sys.exit(2) except ForbiddenException: logger.warning("Access denied for run '{}'", run_id) - msg = share_token_access_denied_message(run_id, share_token) + msg = run_access_denied_message(run_id, share_token) if format == "json": print(json.dumps({"error": "access_denied", "message": msg}), file=sys.stderr) else: @@ -1061,7 +1061,7 @@ def run_dump_metadata( sys.exit(2) except ForbiddenException: logger.warning("Access denied for run '{}'", run_id) - console.print(f"[error]Error:[/error] {share_token_access_denied_message(run_id, share_token)}") + console.print(f"[error]Error:[/error] {run_access_denied_message(run_id, share_token)}") sys.exit(1) except Exception as e: logger.exception(f"Failed to dump custom metadata for run with ID '{run_id}'") @@ -1129,7 +1129,7 @@ def run_dump_item_metadata( sys.exit(2) except ForbiddenException: logger.warning("Access denied for run '{}'", run_id) - print(f"Error: {share_token_access_denied_message(run_id, share_token)}", file=sys.stderr) + print(f"Error: {run_access_denied_message(run_id, share_token)}", file=sys.stderr) sys.exit(1) except Exception as e: logger.exception(f"Failed to dump custom metadata for item '{external_id}' in run with ID '{run_id}'") @@ -1852,7 +1852,7 @@ def update_progress(progress: DownloadProgress) -> None: # noqa: C901 sys.exit(2) except ForbiddenException: logger.warning("Access denied for run '{}'", run_id) - console.print(f"[error]Error:[/error] {share_token_access_denied_message(run_id, share_token)}") + console.print(f"[error]Error:[/error] {run_access_denied_message(run_id, share_token)}") sys.exit(1) except Exception as e: logger.exception(f"Failed to download results of run with ID '{run_id}'") diff --git a/src/aignostics/application/_utils.py b/src/aignostics/application/_utils.py index d5a61284a..5a78be36b 100644 --- a/src/aignostics/application/_utils.py +++ b/src/aignostics/application/_utils.py @@ -601,13 +601,9 @@ def get_supported_extensions_for_application(application_id: str) -> set[str]: raise RuntimeError(message) -def share_token_access_denied_message(run_id: str, share_token: str | None) -> str: +def run_access_denied_message(run_id: str, share_token: str | None) -> str: """Compose the operator-facing "access denied" message for a run. - Pure message builder: it centralizes the wording and the share-token hint shared by - the run CLI commands that support ``--share-token``. It has no side effects — the - caller owns logging, the output sink (console, stderr, or JSON), and the exit code. - Args: run_id (str): The run access was denied for. share_token (str | None): The share token supplied, if any. When set, a hint diff --git a/src/aignostics/platform/resources/runs.py b/src/aignostics/platform/resources/runs.py index 3aa82351a..557147e05 100644 --- a/src/aignostics/platform/resources/runs.py +++ b/src/aignostics/platform/resources/runs.py @@ -12,7 +12,6 @@ from pathlib import Path from time import sleep from typing import Any, cast -from urllib.parse import urlencode import requests from aignx.codegen.exceptions import ApiException, NotFoundException, ServiceException @@ -149,10 +148,6 @@ def get_download_url(self) -> str: configuration = self._api.api_client.configuration host = configuration.host.rstrip("/") endpoint_url = f"{host}/api/v1/runs/{self.run_id}/artifacts/{self.artifact_id}/file" - if self._share_token is not None: - # Percent-encode the secret so reserved characters (& # = space) cannot - # corrupt the URL or inject extra query parameters. - endpoint_url += f"?{urlencode({'share_token': self._share_token})}" proxy = getattr(configuration, "proxy", None) ssl_ca_cert = getattr(configuration, "ssl_ca_cert", None) verify_ssl = getattr(configuration, "verify_ssl", True) @@ -202,10 +197,13 @@ def _fetch_redirect_url( try: # Always send the OAuth Bearer token: the platform requires an # authenticated account on every request. When a share_token is present - # it is carried as a query parameter on ``endpoint_url`` and elevates - # that authenticated user's access to the shared resource. + # it is passed via the ``params`` argument as a ``share_token`` query + # parameter and elevates that authenticated user's access to the shared + # resource. + params = {"share_token": self._share_token} if self._share_token else {} with requests.get( endpoint_url, + params=params, headers={ "Authorization": f"Bearer {token_provider()}", "User-Agent": user_agent(), diff --git a/tests/aignostics/platform/resources/runs_test.py b/tests/aignostics/platform/resources/runs_test.py index 53f37b498..6275980f1 100644 --- a/tests/aignostics/platform/resources/runs_test.py +++ b/tests/aignostics/platform/resources/runs_test.py @@ -1468,24 +1468,7 @@ def test_artifact_get_download_url_appends_share_token(configured_api, record_pr with patch(_PATCH_GET_TOKEN, return_value="t"), patch(_PATCH_REQUESTS_GET, return_value=response) as mock_get: art.get_download_url() - assert "share_token=s3cr3t" in mock_get.call_args.args[0] - - -@pytest.mark.unit -def test_artifact_get_download_url_percent_encodes_share_token(configured_api, record_property) -> None: - """A share_token with reserved characters is percent-encoded, not injected as extra query params.""" - record_property("tested-item-id", "SWR-APPLICATION-4-3, SPEC-PLATFORM-SERVICE") - art = Artifact(configured_api, _RUN_ID, _ARTIFACT_ID, share_token="a b&x=1") # noqa: S106 - response = _redirect_response(_PRESIGNED_URL) - - with patch(_PATCH_GET_TOKEN, return_value="t"), patch(_PATCH_REQUESTS_GET, return_value=response) as mock_get: - art.get_download_url() - - url = mock_get.call_args.args[0] - assert "share_token=a+b%26x%3D1" in url - # The raw "&" must not have created a second query parameter. - assert url.count("?") == 1 - assert "&" not in url.split("?", 1)[1] + assert mock_get.call_args.kwargs["params"] == {"share_token": "s3cr3t"} @pytest.mark.unit @@ -1499,7 +1482,7 @@ def test_artifact_get_download_url_sends_bearer_with_share_token(configured_api, art.get_download_url() assert mock_get.call_args.kwargs["headers"]["Authorization"] == "Bearer t" - assert "share_token=s3cr3t" in mock_get.call_args.args[0] + assert mock_get.call_args.kwargs["params"] == {"share_token": "s3cr3t"} @pytest.mark.unit