From ce133cd34f3fb11c6f0856161fe4183aedafb5ba Mon Sep 17 00:00:00 2001 From: vasques1 Date: Wed, 28 Oct 2026 09:42:01 -0400 Subject: [PATCH 1/2] fix(jobs): mask credentials in the logged container overrides submit_batch_job logged the full container overrides, environment included, so any credential passed to a job reached this service's logs in plain text. Values whose names look like credentials (TOKEN, KEY, SECRET, PASSWORD) are now masked in the log line; the overrides sent to AWS Batch are unchanged. Co-Authored-By: Claude Opus 5.5 --- api/jobs/services.py | 18 +++++++++++++++++- tests/api/test_jobs.py | 23 +++++++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/api/jobs/services.py b/api/jobs/services.py index 1e333e2f..5b954bf4 100644 --- a/api/jobs/services.py +++ b/api/jobs/services.py @@ -219,6 +219,22 @@ def update_batch_job( return job +# Environment variable names that mark a value as a credential. The values reach +# the container; they are kept out of this service's logs. +_SECRET_NAME_MARKERS = ("TOKEN", "KEY", "SECRET", "PASSWORD") + + +def _loggable_overrides(container_overrides: Dict[str, Any]) -> Dict[str, Any]: + """A copy of container overrides with credential values masked, for logging.""" + environment = [ + {**env, "value": ""} + if any(marker in env.get("name", "").upper() for marker in _SECRET_NAME_MARKERS) + else env + for env in container_overrides.get("environment", []) + ] + return {**container_overrides, "environment": environment} + + def submit_batch_job( session: Session, job_name: str, @@ -264,7 +280,7 @@ def submit_batch_job( container_overrides["environment"].append( {"name": "NGS360_API_ENDPOINT", "value": settings.client_origin} ) - logger.info(f"Container overrides: {container_overrides}") + logger.info(f"Container overrides: {_loggable_overrides(container_overrides)}") try: batch_client = boto3.client("batch", region_name=settings.AWS_REGION) diff --git a/tests/api/test_jobs.py b/tests/api/test_jobs.py index 926b8b4e..39430fed 100644 --- a/tests/api/test_jobs.py +++ b/tests/api/test_jobs.py @@ -1360,3 +1360,26 @@ def test_submit_batch_job_injects_alongside_existing_env( env_dict = {e["name"]: e["value"] for e in env} assert env_dict["MY_VAR"] == "my_value" assert env_dict["NGS360_API_ENDPOINT"] == get_settings().client_origin + + +def test_loggable_overrides_masks_credentials(): + """Test credential values are masked in the logged overrides, and only those""" + from api.jobs.services import _loggable_overrides + + overrides = { + "command": ["launch"], + "environment": [ + {"name": "NGS360_AUTH_TOKEN", "value": "ngs360_secret"}, + {"name": "PROJECT_ID", "value": "P-1"}, + ], + } + + logged = _loggable_overrides(overrides) + + assert logged["environment"] == [ + {"name": "NGS360_AUTH_TOKEN", "value": ""}, + {"name": "PROJECT_ID", "value": "P-1"}, + ] + assert logged["command"] == ["launch"] + # The overrides sent to AWS Batch are untouched + assert overrides["environment"][0]["value"] == "ngs360_secret" From 8256f4dd27ca88739d4c88cb05254237344f8663 Mon Sep 17 00:00:00 2001 From: vasques1 Date: Wed, 28 Oct 2026 09:42:01 -0400 Subject: [PATCH 2/2] feat(actions): mint a per-job API key for the submitter when the config asks A job that calls back into NGS360 as the user who started it -- a pipeline launcher overseeing its own child runs -- outlives any session token that user holds, and until now had to be handed a long-lived credential by hand. An action config can now set aws_batch.user_api_key_hours. On submit, APIServer mints an ngs360_ API key owned by the authenticated submitter, expiring after that many hours, and injects it as NGS360_AUTH_TOKEN with its id as NGS360_AUTH_KEY_ID so the job can retire it on exit. Every call the job makes is then authorized against the submitter's own roles and project memberships. The key is renamed for the Batch job id once there is one, and revoked if AWS Batch refuses the job. The key is minted only from the authenticated user, never from a username string, and the field lives on the action-only batch config so POST /jobs and tool configs cannot ask for one. Co-Authored-By: Claude Opus 5.5 --- api/actions/models.py | 13 +++- api/project/routes.py | 3 +- api/project/services.py | 61 ++++++++++++++--- tests/api/test_projects.py | 135 ++++++++++++++++++++++++++++++++++++- 4 files changed, 197 insertions(+), 15 deletions(-) diff --git a/api/actions/models.py b/api/actions/models.py index 95486cf6..9b854963 100644 --- a/api/actions/models.py +++ b/api/actions/models.py @@ -4,7 +4,7 @@ from enum import Enum from typing import List, Dict, Any -from sqlmodel import SQLModel +from sqlmodel import SQLModel, Field from api.jobs.models import AwsBatchConfig @@ -36,6 +36,15 @@ class PlatformConfig(SQLModel): export_command: str | None = None +class ActionAwsBatchConfig(AwsBatchConfig): + """AWS Batch configuration for an action, which runs as the submitting user.""" + # Hours an API key minted for the submitter stays valid, for a job (such as a + # pipeline launcher) that calls back into NGS360 as that user. Unset means no + # key is minted. Sized to the job definition's timeout: the job retires the + # key when it exits, and the expiry covers a job that is killed first. + user_api_key_hours: int | None = Field(default=None, gt=0, le=24 * 7) + + class ActionConfig(SQLModel): """Model for action workflow configuration.""" workflow_id: str | None = None @@ -44,7 +53,7 @@ class ActionConfig(SQLModel): inputs: List[ActionInput] | None = None platforms: Dict[str, PlatformConfig] export_command: str | None = None - aws_batch: AwsBatchConfig | None = None + aws_batch: ActionAwsBatchConfig | None = None class ActionConfigsResponse(SQLModel): diff --git a/api/project/routes.py b/api/project/routes.py index 44bca910..59a21598 100755 --- a/api/project/routes.py +++ b/api/project/routes.py @@ -626,7 +626,8 @@ def submit_pipeline_job( email=current_user.email, reference=request.reference, auto_release=request.auto_release, - s3_client=s3_client + s3_client=s3_client, + submitter=current_user, ) return BatchJobPublic.model_validate(batch_job) diff --git a/api/project/services.py b/api/project/services.py index 8cefb101..b16a803a 100755 --- a/api/project/services.py +++ b/api/project/services.py @@ -2,7 +2,7 @@ Services for the Project API """ -from datetime import datetime, timezone as tz +from datetime import datetime, timedelta, timezone as tz from typing import Literal import boto3 import logging @@ -21,6 +21,8 @@ from api.actions.services import get_all_action_configs from api.actions.models import ActionOption, ActionPlatform from api.jobs.services import submit_batch_job +from api.auth.models import APIKeyCreate, User +from api.auth.services import create_user_api_key, revoke_user_api_key from core.utils import define_search_body, interpolate @@ -672,7 +674,8 @@ def submit_pipeline_job( email: str | None = None, reference: str | None = None, auto_release: bool | None = None, - s3_client=None + s3_client=None, + submitter: User | None = None, ) -> BatchJob: """ Submit a pipeline job to AWS Batch. @@ -692,6 +695,8 @@ def submit_pipeline_job( reference: Export reference label (required for export action) auto_release: Auto-release flag (only valid for export action) s3_client: Optional boto3 S3 client + submitter: The submitting user, who owns the job's API key when the + config's aws_batch.user_api_key_hours asks for one Returns: BatchJob instance @@ -838,16 +843,50 @@ def submit_pipeline_job( detail=f"Failed to interpolate environment variable '{env.name}': {str(e)}" ) from e + # A job that calls back into NGS360 as the submitter (a pipeline launcher) + # outlives any session token they hold, so it gets an API key of its own. + # The key is the submitter's: every call the job makes is authorized against + # their roles and project memberships, as if they had made it themselves. + api_key = None + if action_config.aws_batch.user_api_key_hours: + if submitter is None: + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail="An API key was requested for a job with no submitter" + ) + api_key, raw_key = create_user_api_key(session, submitter, APIKeyCreate( + name=f"Batch job {interpolated_job_name}"[:100], + expires_at=datetime.now(tz.utc) + timedelta( + hours=action_config.aws_batch.user_api_key_hours), + )) + container_overrides["environment"].extend([ + {"name": "NGS360_AUTH_TOKEN", "value": raw_key}, + # Lets the job retire its own key when it exits + {"name": "NGS360_AUTH_KEY_ID", "value": str(api_key.id)}, + ]) + # Submit job to AWS Batch - batch_job = submit_batch_job( - session=session, - job_name=interpolated_job_name, - container_overrides=container_overrides, - job_def=action_config.aws_batch.job_definition, - job_queue=action_config.aws_batch.job_queue, - user=username, - project_id=project.project_id - ) + try: + batch_job = submit_batch_job( + session=session, + job_name=interpolated_job_name, + container_overrides=container_overrides, + job_def=action_config.aws_batch.job_definition, + job_queue=action_config.aws_batch.job_queue, + user=username, + project_id=project.project_id + ) + except Exception: + if api_key is not None: + revoke_user_api_key(session, submitter, str(api_key.id)) + raise + + if api_key is not None: + # Renamed for the job's id once there is one, so a key in the user's list + # can be matched to the job that held it. + api_key.name = f"Batch job {batch_job.id}"[:100] + session.add(api_key) + session.commit() return batch_job diff --git a/tests/api/test_projects.py b/tests/api/test_projects.py index dd5fd844..8567568d 100644 --- a/tests/api/test_projects.py +++ b/tests/api/test_projects.py @@ -3,9 +3,13 @@ """ +from datetime import datetime, timedelta, timezone from unittest.mock import patch, MagicMock +import uuid + +import botocore.exceptions from fastapi.testclient import TestClient -from sqlmodel import Session +from sqlmodel import Session, select import yaml from api.project.models import Project, ProjectAttribute @@ -915,6 +919,135 @@ def test_submit_create_project_job( assert call_args["jobDefinition"] == "pipeline-job-def:1" +def _submit_rna_seq_job(client, project, mock_s3_client, aws_batch_extra=None): + """Submit a create-project job against a minimal RNA-Seq action config""" + pipeline_config = { + "project_type": "RNA-Seq", + "project_admins": ["admin@example.com"], + "platforms": { + "Arvados": {"create_project_command": "launch {{projectid}}"} + }, + "aws_batch": { + "job_name": "launch-{{projectid}}", + "job_definition": "launcher-job-def:1", + "job_queue": "batch-queue", + "command": "launch", + **(aws_batch_extra or {}), + } + } + mock_s3_client.setup_bucket( + "ngs360-resources", "pipeline_configs/", + [{"Key": "pipeline_configs/rna-seq_pipeline.yaml"}], [] + ) + mock_s3_client.uploaded_files["ngs360-resources"] = { + "pipeline_configs/rna-seq_pipeline.yaml": yaml.dump(pipeline_config).encode("utf-8") + } + return client.post( + f"/api/v1/projects/{project.project_id}/actions/submit", + json={"action": "create-project", "platform": "Arvados", "project_type": "RNA-Seq"} + ) + + +def _batch_environment(mock_batch: MagicMock) -> dict: + """The environment a mocked submit_job was called with, as a dict""" + overrides = mock_batch.submit_job.call_args[1]["containerOverrides"] + return {env["name"]: env["value"] for env in overrides["environment"]} + + +@patch("api.jobs.services.boto3.client") +@patch("api.actions.services.get_setting_value") +def test_submit_job_mints_submitter_api_key( + mock_get_setting: MagicMock, + mock_boto_client: MagicMock, + client: TestClient, + session: Session, + test_project: Project, + mock_s3_client +): + """Test a config asking for a key gets one owned by the submitter, named for the job""" + from api.auth.models import APIKey, User + from core.security import hash_api_key + + mock_get_setting.return_value = "s3://ngs360-resources/pipeline_configs/" + mock_batch = MagicMock() + mock_batch.submit_job.return_value = {"jobId": "aws-batch-job-123"} + mock_boto_client.return_value = mock_batch + + before = datetime.now(timezone.utc) + response = _submit_rna_seq_job( + client, test_project, mock_s3_client, {"user_api_key_hours": 72} + ) + + assert response.status_code == 201 + environment = _batch_environment(mock_batch) + api_key = session.get(APIKey, uuid.UUID(environment["NGS360_AUTH_KEY_ID"])) + assert api_key.hashed_key == hash_api_key(environment["NGS360_AUTH_TOKEN"]) + assert session.get(User, api_key.user_id).username == "testuser" + assert api_key.is_active + assert api_key.name == "Batch job aws-batch-job-123" + expires_at = api_key.expires_at.replace(tzinfo=timezone.utc) + ttl = timedelta(hours=72) + assert before + ttl <= expires_at <= datetime.now(timezone.utc) + ttl + + +@patch("api.jobs.services.boto3.client") +@patch("api.actions.services.get_setting_value") +def test_submit_job_without_key_config_mints_nothing( + mock_get_setting: MagicMock, + mock_boto_client: MagicMock, + client: TestClient, + session: Session, + test_project: Project, + mock_s3_client +): + """Test jobs whose config does not ask for a key get no credential""" + from api.auth.models import APIKey + + mock_get_setting.return_value = "s3://ngs360-resources/pipeline_configs/" + mock_batch = MagicMock() + mock_batch.submit_job.return_value = {"jobId": "aws-batch-job-123"} + mock_boto_client.return_value = mock_batch + + response = _submit_rna_seq_job(client, test_project, mock_s3_client) + + assert response.status_code == 201 + environment = _batch_environment(mock_batch) + assert "NGS360_AUTH_TOKEN" not in environment + assert "NGS360_AUTH_KEY_ID" not in environment + assert session.exec(select(APIKey)).all() == [] + + +@patch("api.jobs.services.boto3.client") +@patch("api.actions.services.get_setting_value") +def test_submit_job_failure_revokes_minted_key( + mock_get_setting: MagicMock, + mock_boto_client: MagicMock, + client: TestClient, + session: Session, + test_project: Project, + mock_s3_client +): + """Test a key minted for a job AWS Batch refused does not stay usable""" + from api.auth.models import APIKey + + mock_get_setting.return_value = "s3://ngs360-resources/pipeline_configs/" + mock_batch = MagicMock() + mock_batch.submit_job.side_effect = botocore.exceptions.ClientError( + {"Error": {"Code": "ClientException", "Message": "no such queue"}}, "SubmitJob" + ) + mock_boto_client.return_value = mock_batch + + response = _submit_rna_seq_job( + client, test_project, mock_s3_client, {"user_api_key_hours": 72} + ) + + assert response.status_code == 500 + (api_key,) = session.exec(select(APIKey)).all() + session.refresh(api_key) + assert not api_key.is_active + assert api_key.revoked_at is not None + + @patch("api.jobs.services.boto3.client") @patch("api.actions.services.get_setting_value") def test_submit_export_results_job(