diff --git a/README.md b/README.md index 8940250..7b14e6b 100644 --- a/README.md +++ b/README.md @@ -79,13 +79,13 @@ deployments: # Org secrets/variables: org is destination.organization, or the owner of # repository (acme from acme/web). Org-only destinations work for this scope. - # Token needs admin:org (classic PAT). visibility defaults to private. + # Token needs admin:org (classic PAT). visibility is required. - name: github-org set: production destination: github scope: kind: organization - visibility: private # all | private | selected (+ selected_repository_ids) + visibility: private # required: all | private | selected (+ selectedRepositoryIds) secrets: apiKey: API_KEY variables: @@ -104,8 +104,8 @@ Vercel destination modes (selected by `scope.kind`): destinations: vercel: connector: vercel - teamId: team_xyz # required - project: prj_abc # optional; required for kind: environment + teamId: team_xyz # required + project: prj_abc # optional; required for kind: environment auth: tokenEnv: VERCEL_TOKEN @@ -124,7 +124,7 @@ deployments: scope: kind: shared-environment targets: [production] - projects: [prj_abc, prj_def] # optional link set + projects: [prj_abc, prj_def] # optional link set secrets: sharedSecret: SHARED_SECRET ``` @@ -176,6 +176,8 @@ Useful flags: `--config`, `--format json`, `--verbose`, `--quiet`, `--deployment With `--prune`, SecretSync lists remote names at plan time (secrets and variables separately) and treats YAML as the full desired inventory for each destination scope + kind — remote entries not listed in the config are planned for deletion. Without `--prune`, apply is put-only. +For Vercel, a remote env var belongs to a deployment only when its target set **exactly** matches `scope.targets` (and, for shared env, `scope.projects`). A multi-target remote such as `[production, preview]` is owned by a deployment that declares that same multi-target scope — not by a production-only or preview-only sibling. + ## Supported Destinations We currently support these destinations. diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 58bf1b4..c05b8be 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -65,6 +65,8 @@ With `--prune` on `plan` / `apply` (or the TUI prune checkbox): There is no local last-applied state file — every prune plan reflects the live remote inventory. Auth/list failures fail the plan; they are not skipped. Connectors without `list_names` + delete support refuse prune with a clear error. +Vercel ownership is exact target-set equality (`scope.targets` == remote `target`), for both `environment` and `shared-environment`. Overlap matching would let a multi-target inventory unit prune sibling single-target rows. + ## Connector boundary Connectors own batching and provider adaptation. The coordinator groups mutations by destination and deployment, then calls `apply` with `PutMutation` values and optional `DeleteMutation`s. Each mutation must receive exactly one result (`applied` / `failed` / `skipped`). @@ -107,7 +109,7 @@ See security tests: [`tests/security/test_envfile_pipe.py`](../tests/security/te ## HTTP client -[`infrastructure/http.py`](../src/secretsync/infrastructure/http.py) wraps httpx with bounded retries (429/502/503/504), no wire body logging, and `redact_headers` / `response_debug_meta` for safe diagnostics. Errors map to `SafeConnectorError` codes without provider body text. +[`infrastructure/http.py`](../src/secretsync/infrastructure/http.py) wraps httpx with bounded retries (429/502/503/504), no Authorization wire logging, and `redact_headers` / `response_debug_meta` for safe diagnostics. HTTP failures map to `SafeConnectorError` with a bounded, secret-redacted provider error detail (also logged at DEBUG and written to the mutation audit line). ## Errors and reports diff --git a/docs/github-token-permissions.png b/docs/github-token-permissions.png new file mode 100644 index 0000000..d511379 Binary files /dev/null and b/docs/github-token-permissions.png differ diff --git a/examples/.env.tpl b/examples/.env.tpl index 8138faf..602ec76 100644 --- a/examples/.env.tpl +++ b/examples/.env.tpl @@ -9,3 +9,15 @@ AWS_REGION="us-east-1" SECRET_ONE_PROD="op://secretsync-example/production/secret one" SECRET_ONE_STAGING="op://secretsync-example/staging/secret one" SECRET_TWO_COMMON="op://secretsync-example/common/secret two" + +SECRET_THREE_ORG_PROD="op://secretsync-example/org/api key production" +SECRET_THREE_ORG_STAGING="op://secretsync-example/org/api key staging" + + +PUBLIC_APP_URL_PROD="public-app-url-prod.com" +PUBLIC_APP_URL_STAGING="public-app-url-staging.com" + +ORG_NAME="achadha-team" + +LOG_LEVEL_PROD="info" +LOG_LEVEL_STAGING="debug" \ No newline at end of file diff --git a/examples/secretsync.yaml b/examples/secretsync.yaml index 2df612f..1bd7d85 100644 --- a/examples/secretsync.yaml +++ b/examples/secretsync.yaml @@ -9,21 +9,38 @@ secrets: secretTwoCommon: env: SECRET_TWO_COMMON + secretThreeOrgProd: + env: SECRET_THREE_ORG_PROD + secretThreeOrgStaging: + env: SECRET_THREE_ORG_STAGING + variables: - publicAppUrl: - env: PUBLIC_APP_URL - logLevel: - env: LOG_LEVEL + publicAppUrlProd: + env: PUBLIC_APP_URL_PROD + publicAppUrlStaging: + env: PUBLIC_APP_URL_STAGING + logLevelProd: + env: LOG_LEVEL_PROD + logLevelStaging: + env: LOG_LEVEL_STAGING + orgName: + env: ORG_NAME sets: common: - include: [secretTwoCommon, logLevel] + include: [secretTwoCommon, orgName] production: extends: common - include: [secretOneProd, publicAppUrl] + include: [secretOneProd, secretThreeOrgProd, publicAppUrlProd, logLevelProd] staging: extends: common - include: [secretOneStaging, publicAppUrl] + include: + [ + secretOneStaging, + secretThreeOrgStaging, + publicAppUrlStaging, + logLevelStaging, + ] destinations: github: @@ -39,10 +56,15 @@ destinations: tokenEnv: VERCEL_TOKEN sst: connector: sst - workingDirectory: ./examples/secretsync-sst-example + workingDirectory: ./secretsync-sst-example executable: bunx deployments: + ### Examples for GitHub deployment + + ########################################################################################## + ## Environment-level secrets [Production] + ########################################################################################## - name: github-production set: production destination: github @@ -53,8 +75,10 @@ deployments: secretOneProd: SECRET_ONE secretTwoCommon: SECRET_TWO variables: - logLevel: LOG_LEVEL - publicAppUrl: PUBLIC_APP_URL + logLevelProd: LOG_LEVEL + publicAppUrlProd: PUBLIC_APP_URL + + ## Environment-level secrets [Staging] - name: github-staging set: staging destination: github @@ -65,7 +89,25 @@ deployments: secretOneStaging: SECRET_ONE secretTwoCommon: SECRET_TWO variables: - logLevel: LOG_LEVEL + logLevelStaging: LOG_LEVEL + + ### Organization-level secrets + # - name: github-org + # set: production + # destination: github + # scope: + # kind: organization + # visibility: all + # secrets: + # secretThreeOrgProd: SECRET_THREE_ORG + # variables: + # orgName: ORG_NAME + + ########################################################################################## + ## Vercel deployments + ########################################################################################## + + ## Vercel deployments [Production] - name: vercel-production set: production destination: vercel @@ -74,10 +116,12 @@ deployments: targets: [production] secrets: secretOneProd: SECRET_ONE - secretTwoCommon: SECRET_TWO variables: - publicAppUrl: PUBLIC_APP_URL - - name: vercel-preview + publicAppUrlProd: PUBLIC_APP_URL + logLevelProd: LOG_LEVEL + + ## Vercel deployments [Previews] + - name: vercel-staging set: staging destination: vercel scope: @@ -86,7 +130,48 @@ deployments: secrets: secretOneStaging: SECRET_ONE variables: - publicAppUrl: PUBLIC_APP_URL + publicAppUrlStaging: PUBLIC_APP_URL + logLevelStaging: LOG_LEVEL + + ## Vercel organization-level secrets + - name: vercel-shared-production + set: production + destination: vercel + scope: + kind: shared-environment + targets: [production] + secrets: + secretThreeOrgProd: SECRET_THREE_ORG + variables: + logLevelProd: LOG_LEVEL + + - name: vercel-shared-preview + set: staging + destination: vercel + scope: + kind: shared-environment + targets: [preview] + secrets: + secretThreeOrgStaging: SECRET_THREE_ORG + variables: + logLevelStaging: LOG_LEVEL + + - name: vercel-shared-common + set: common + destination: vercel + scope: + kind: shared-environment + targets: [production, preview] + secrets: + secretTwoCommon: SECRET_TWO + variables: + orgName: ORG_NAME + + ########################################################################################## + ## SST deployments + ########################################################################################## + + ## SST deployments [Staging] - name: sst-staging set: staging destination: sst diff --git a/src/secretsync/application/apply.py b/src/secretsync/application/apply.py index 14252a0..21a8ec3 100644 --- a/src/secretsync/application/apply.py +++ b/src/secretsync/application/apply.py @@ -12,6 +12,7 @@ from typing import Any, Literal import anyio +from loguru import logger from secretsync.application.plan import build_plan_async from secretsync.application.services import AppServices @@ -135,8 +136,6 @@ async def run_apply_async( run_id: str | None = None, ) -> ApplyReport: """Async apply entry used by the Textual TUI workers.""" - from loguru import logger - started = services.clock.now() validation = validate_config( services, @@ -513,6 +512,15 @@ def _audit_mutations( for mutation in request.mutations: result = by_id[mutation.mutation_id] scope = mutation.scopes[0] if mutation.scopes else {} + if result.status == "failed" and result.error is not None: + logger.debug( + "mutation failed dest={} name={} [{}] {}{}", + destination_id, + mutation.name, + result.error.code, + result.error.message, + f" hint={result.error.hint}" if result.error.hint else "", + ) record_mutation_audit( config_path=config_path, run_id=run_id, @@ -526,10 +534,21 @@ def _audit_mutations( effect=result.effect, correlation_id=correlation_id, error_code=result.error.code if result.error else None, + error_message=result.error.message if result.error else None, + error_hint=result.error.hint if result.error else None, ) for deletion in request.deletes: result = by_id[deletion.mutation_id] scope = deletion.scopes[0] if deletion.scopes else {} + if result.status == "failed" and result.error is not None: + logger.debug( + "mutation failed dest={} name={} [{}] {}{}", + destination_id, + deletion.name, + result.error.code, + result.error.message, + f" hint={result.error.hint}" if result.error.hint else "", + ) record_mutation_audit( config_path=config_path, run_id=run_id, @@ -543,6 +562,8 @@ def _audit_mutations( effect=result.effect, correlation_id=correlation_id, error_code=result.error.code if result.error else None, + error_message=result.error.message if result.error else None, + error_hint=result.error.hint if result.error else None, ) diff --git a/src/secretsync/application/plan.py b/src/secretsync/application/plan.py index d50f581..5bfd8e6 100644 --- a/src/secretsync/application/plan.py +++ b/src/secretsync/application/plan.py @@ -60,7 +60,11 @@ def build_plan( for deployment in selected: available = composed_sets[deployment.set] for logical_id, destination_name in deployment.secrets.items(): - source = available.require(logical_id) + source = available.require( + logical_id, + deployment=deployment.name, + destination=deployment.destination, + ) puts.append( PlannedPut( mutation_id=stable_mutation_id(deployment.name, destination_name), @@ -70,7 +74,11 @@ def build_plan( ) ) for logical_id, destination_name in deployment.variables.items(): - source = available.require(logical_id) + source = available.require( + logical_id, + deployment=deployment.name, + destination=deployment.destination, + ) puts.append( PlannedPut( mutation_id=stable_mutation_id(deployment.name, destination_name), diff --git a/src/secretsync/application/validate.py b/src/secretsync/application/validate.py index 7d60b98..abdeebd 100644 --- a/src/secretsync/application/validate.py +++ b/src/secretsync/application/validate.py @@ -62,6 +62,9 @@ def validate_config( destinations=destinations, ) except SecretSyncError as exc: + logger.error("{}", exc.safe.message) + if exc.safe.hint: + logger.error("hint: {}", exc.safe.hint) return ValidationResult( issues=[ValidationIssue(code=exc.code, message=exc.safe.message, hint=exc.safe.hint)], exit_code=exit_code_for(exc), @@ -93,6 +96,9 @@ def validate_loaded( selected_deployments=tuple(d.name for d in selected), ) except SecretSyncError as exc: + logger.error("{}", exc.safe.message) + if exc.safe.hint: + logger.error("hint: {}", exc.safe.hint) return ValidationResult( issues=[ValidationIssue(code=exc.code, message=exc.safe.message, hint=exc.safe.hint)], exit_code=exit_code_for(exc), @@ -134,12 +140,18 @@ def _validate_deployments( if destination.connector == "vercel": _validate_vercel_deployment(deployment, destination) + elif destination.connector == "github-actions": + _validate_github_deployment(deployment) available = composed[deployment.set] kinds_used: set[ValueKind] = set() for logical_id, dest_name in deployment.secrets.items(): - ref = available.require(logical_id) + ref = available.require( + logical_id, + deployment=deployment.name, + destination=deployment.destination, + ) if ref.kind is not ValueKind.SECRET: raise ConfigInvalidError( f"Deployment '{deployment.name}' maps '{logical_id}' under secrets, " @@ -158,7 +170,11 @@ def _validate_deployments( ) for logical_id, dest_name in deployment.variables.items(): - ref = available.require(logical_id) + ref = available.require( + logical_id, + deployment=deployment.name, + destination=deployment.destination, + ) if ref.kind is not ValueKind.VARIABLE: raise ConfigInvalidError( f"Deployment '{deployment.name}' maps '{logical_id}' under variables, " @@ -234,6 +250,21 @@ def _validate_vercel_deployment(deployment: DeploymentDefinition, destination: o ) +def _validate_github_deployment(deployment: DeploymentDefinition) -> None: + from secretsync.destinations.github_actions import _validate_scope + + reason = _validate_scope(deployment.scope) + if reason: + raise ConfigInvalidError( + f"Deployment '{deployment.name}' has invalid GitHub scope: {reason}", + hint=( + "Use scope.kind: repository | environment | organization. " + "For organization, set visibility: all|private|selected " + "(selected requires selectedRepositoryIds)." + ), + ) + + def _record_target( seen_targets: set[tuple[str, str, str, str]], deployment: DeploymentDefinition, @@ -267,7 +298,11 @@ def _check_environment_presence( for deployment in selected: available = composed[deployment.set] for logical_id in (*deployment.secrets, *deployment.variables): - ref = available.require(logical_id) + ref = available.require( + logical_id, + deployment=deployment.name, + destination=deployment.destination, + ) required_source.add(ref.env_name) destination = config.destinations[deployment.destination] diff --git a/src/secretsync/config/compose.py b/src/secretsync/config/compose.py index 3e77691..b94e9ad 100644 --- a/src/secretsync/config/compose.py +++ b/src/secretsync/config/compose.py @@ -15,12 +15,30 @@ def __init__(self, set_id: str, members: dict[str, SecretRef]) -> None: self._members = members self.order: tuple[str, ...] = tuple(members.keys()) - def require(self, logical_id: str) -> SecretRef: + def require( + self, + logical_id: str, + *, + deployment: str | None = None, + destination: str | None = None, + ) -> SecretRef: try: return self._members[logical_id] except KeyError as exc: + where = "" + if deployment is not None: + where = f" (deployment '{deployment}'" + if destination is not None: + where += f", destination '{destination}'" + where += ")" raise ConfigInvalidError( - f"Logical id '{logical_id}' is not available in set '{self.set_id}'" + f"Logical id '{logical_id}' is not available in set '{self.set_id}'{where}", + hint=( + f"Add '{logical_id}' to set '{self.set_id}' (or an ancestor), " + f"or point the deployment at a set that includes it." + if deployment is not None + else None + ), ) from exc def get(self, logical_id: str) -> SecretRef | None: diff --git a/src/secretsync/destinations/github_actions.py b/src/secretsync/destinations/github_actions.py index ce7057c..caa5391 100644 --- a/src/secretsync/destinations/github_actions.py +++ b/src/secretsync/destinations/github_actions.py @@ -236,7 +236,7 @@ def _scope_key(scope: Mapping[str, JsonValue]) -> str: return f"environment:{scope.get('environment', '')}" if kind == "organization": visibility = scope.get("visibility", "private") - ids = scope.get("selected_repository_ids") + ids = scope.get("selectedRepositoryIds") if visibility == "selected" and isinstance(ids, list): return f"organization:{visibility}:{','.join(str(i) for i in ids)}" return f"organization:{visibility}" @@ -250,14 +250,15 @@ def _public_key_cache_key(scope: Mapping[str, JsonValue]) -> str: return _scope_key(scope) -def _org_visibility_payload(scope: Mapping[str, JsonValue]) -> dict[str, JsonValue] | str: - """Return visibility fields for org create/update, or an error message.""" - visibility = scope.get("visibility", "private") +def _org_visibility_error(scope: Mapping[str, JsonValue]) -> str | None: + """Validate organization visibility / selectedRepositoryIds; None if ok.""" + if "visibility" not in scope: + return "organization scope requires visibility; require all|private|selected" + visibility = scope.get("visibility") if visibility not in ORG_VISIBILITIES: return "Invalid organization visibility; require all|private|selected" - payload: dict[str, JsonValue] = {"visibility": str(visibility)} if visibility == "selected": - ids = scope.get("selected_repository_ids") + ids = scope.get("selectedRepositoryIds") # bool is a subclass of int; reject it explicitly. valid_ids = ( isinstance(ids, list) @@ -266,10 +267,26 @@ def _org_visibility_payload(scope: Mapping[str, JsonValue]) -> dict[str, JsonVal ) if not valid_ids: return ( - "selected_repository_ids required as non-empty int array " - "when visibility is selected" + "selectedRepositoryIds required as non-empty int array when visibility is selected" ) - payload["selected_repository_ids"] = list(ids) # type: ignore[arg-type] + return None + + +def _org_visibility_payload(scope: Mapping[str, JsonValue]) -> dict[str, JsonValue] | str: + """Return visibility fields for org create/update, or an error message.""" + # Apply keeps a private default for callers that skip offline validate. + if "visibility" not in scope: + scope = {**scope, "visibility": "private"} + err = _org_visibility_error(scope) + if err is not None: + return err + visibility = str(scope["visibility"]) + payload: dict[str, JsonValue] = {"visibility": visibility} + if visibility == "selected": + ids = scope["selectedRepositoryIds"] + assert isinstance(ids, list) # narrowed by _org_visibility_error + # GitHub Actions API field name is snake_case. + payload["selected_repository_ids"] = list(ids) return payload @@ -282,6 +299,16 @@ def _invalid_scope_kind(scope: Mapping[str, JsonValue]) -> str | None: return None +def _validate_scope(scope: Mapping[str, JsonValue]) -> str | None: + """Offline / preflight scope checks. Return error message or None.""" + kind_err = _invalid_scope_kind(scope) + if kind_err is not None: + return kind_err + if scope.get("kind") == "organization": + return _org_visibility_error(scope) + return None + + @dataclass class GitHubActionsDestination: manifest: DestinationManifest @@ -397,9 +424,7 @@ async def list_names( from secretsync.infrastructure.http import error_for_status raise ListNamesError( - error_for_status( - response.status_code, correlation_id=context.correlation_id - ) + error_for_status(response, correlation_id=context.correlation_id) ) payload = response.json() items = payload.get(collection_key, []) if isinstance(payload, dict) else [] @@ -698,9 +723,7 @@ async def _get_public_key( if response.status_code != 200: from secretsync.infrastructure.http import error_for_status - raise HttpRequestError( - error_for_status(response.status_code, correlation_id=correlation_id) - ) + raise HttpRequestError(error_for_status(response, correlation_id=correlation_id)) payload = response.json() key_id = str(payload["key_id"]) key_b64 = str(payload["key"]) @@ -793,9 +816,10 @@ async def _put_secret( mutation_id=mutation.mutation_id, status="failed", error=error_for_status( - response.status_code, + response, mutation_id=mutation.mutation_id, correlation_id=correlation_id, + secrets=[bytes(mutation.value).decode("utf-8", errors="replace")], ), ), 1, @@ -930,9 +954,10 @@ async def _put_variable( mutation_id=mutation.mutation_id, status="failed", error=error_for_status( - update.status_code, + update, mutation_id=mutation.mutation_id, correlation_id=correlation_id, + secrets=[bytes(mutation.value).decode("utf-8", errors="replace")], ), ), 2, @@ -944,9 +969,10 @@ async def _put_variable( mutation_id=mutation.mutation_id, status="failed", error=error_for_status( - create.status_code, + create, mutation_id=mutation.mutation_id, correlation_id=correlation_id, + secrets=[bytes(mutation.value).decode("utf-8", errors="replace")], ), ), 1, @@ -1038,7 +1064,7 @@ async def _delete_variable( mutation_id=deletion.mutation_id, status="failed", error=error_for_status( - response.status_code, + response, mutation_id=deletion.mutation_id, correlation_id=correlation_id, ), @@ -1132,7 +1158,7 @@ async def _delete_secret( mutation_id=deletion.mutation_id, status="failed", error=error_for_status( - response.status_code, + response, mutation_id=deletion.mutation_id, correlation_id=correlation_id, ), diff --git a/src/secretsync/destinations/vercel.py b/src/secretsync/destinations/vercel.py index 80319a4..4542a1d 100644 --- a/src/secretsync/destinations/vercel.py +++ b/src/secretsync/destinations/vercel.py @@ -156,6 +156,11 @@ def _targets_and_type_match( *, kind: ValueKind, ) -> bool: + """True when remote target set equals scope.targets (exact ownership). + + Overlap matching is wrong: a shared deployment with targets [production, preview] + must not own (list/update/prune) rows that only target production or only preview. + """ targets_raw = scope.get("targets") if not isinstance(targets_raw, list): return False @@ -164,7 +169,7 @@ def _targets_and_type_match( if not isinstance(remote_targets, list): return False remote = {str(t) for t in remote_targets} - if not wanted.intersection(remote): + if wanted != remote: return False remote_type = str(item.get("type", "")) if kind is ValueKind.SECRET: @@ -524,7 +529,11 @@ async def _upsert_chunk( ) return edited, 1 + n - err = error_for_status(response.status_code, correlation_id=correlation_id) + err = error_for_status( + response, + correlation_id=correlation_id, + secrets=[bytes(m.value).decode("utf-8", errors="replace") for m in mutations], + ) return ( { m.mutation_id: MutationResult( @@ -557,9 +566,7 @@ async def _list_envs( client, "GET", list_url, params=params, correlation_id=correlation_id ) if listed.status_code != 200: - raise ListNamesError( - error_for_status(listed.status_code, correlation_id=correlation_id) - ) + raise ListNamesError(error_for_status(listed, correlation_id=correlation_id)) return _parse_env_list(listed.json()), 1 async def _list_shared_envs( @@ -582,9 +589,7 @@ async def _list_shared_envs( ) requests += 1 if listed.status_code != 200: - raise ListNamesError( - error_for_status(listed.status_code, correlation_id=correlation_id) - ) + raise ListNamesError(error_for_status(listed, correlation_id=correlation_id)) page, next_ts = _parse_shared_env_page(listed.json()) items.extend(page) if next_ts is None: @@ -750,7 +755,11 @@ async def _create_shared( }, 1, ) - err = error_for_status(response.status_code, correlation_id=correlation_id) + err = error_for_status( + response, + correlation_id=correlation_id, + secrets=[bytes(m.value).decode("utf-8", errors="replace") for m in mutations], + ) return ( { m.mutation_id: MutationResult( @@ -786,8 +795,10 @@ async def _patch_shared( "value": bytes(mutation.value).decode("utf-8"), "type": _env_type(mutation.kind), "target": [str(t) for t in targets_raw], - "projectId": sorted(_scope_projects(scope)), } + projects = sorted(_scope_projects(scope)) + if projects: + entry["projectId"] = projects payload_updates[env_id] = entry url = f"{VERCEL_API}{SHARED_ENV_PATH}" params = {"teamId": team_id} @@ -830,7 +841,11 @@ async def _patch_shared( }, 1, ) - err = error_for_status(response.status_code, correlation_id=correlation_id) + err = error_for_status( + response, + correlation_id=correlation_id, + secrets=[bytes(m.value).decode("utf-8", errors="replace") for m, _ in updates], + ) return ( { m.mutation_id: MutationResult( @@ -938,7 +953,7 @@ async def _delete_shared( effect="deleted", ) else: - err = error_for_status(response.status_code, correlation_id=correlation_id) + err = error_for_status(response, correlation_id=correlation_id) for deletion, _ in chunk: results[deletion.mutation_id] = MutationResult( mutation_id=deletion.mutation_id, @@ -1042,7 +1057,7 @@ async def _delete_many( mutation_id=deletion.mutation_id, status="failed", error=error_for_status( - response.status_code, + response, mutation_id=deletion.mutation_id, correlation_id=correlation_id, ), @@ -1160,9 +1175,10 @@ async def _edit_fallback( mutation_id=mutation.mutation_id, status="failed", error=error_for_status( - edited.status_code, + edited, mutation_id=mutation.mutation_id, correlation_id=correlation_id, + secrets=[bytes(mutation.value).decode("utf-8", errors="replace")], ), ) return results, requests diff --git a/src/secretsync/infrastructure/audit.py b/src/secretsync/infrastructure/audit.py index b9de161..e7db661 100644 --- a/src/secretsync/infrastructure/audit.py +++ b/src/secretsync/infrastructure/audit.py @@ -119,6 +119,8 @@ def record_mutation_audit( effect: str | None, correlation_id: str, error_code: str | None = None, + error_message: str | None = None, + error_hint: str | None = None, cwd: Path | None = None, ) -> Path: """Append one value-free per-secret mutation line.""" @@ -142,10 +144,20 @@ def record_mutation_audit( f"correlation={correlation_id}", f"error={error_code or '-'}", ] + if error_message: + parts.append(f"error_message={_audit_quote(error_message)}") + if error_hint: + parts.append(f"error_hint={_audit_quote(error_hint)}") _append_line(audit_path, " ".join(parts)) return audit_path +def _audit_quote(value: str) -> str: + """Quote audit field values that may contain spaces or newlines.""" + escaped = value.replace("\\", "\\\\").replace('"', '\\"').replace("\n", "\\n") + return f'"{escaped}"' + + def _append_line(audit_path: Path, line: str) -> None: with audit_path.open("a", encoding="utf-8") as handle: handle.write(line + "\n") diff --git a/src/secretsync/infrastructure/http.py b/src/secretsync/infrastructure/http.py index 638ee0b..00807b5 100644 --- a/src/secretsync/infrastructure/http.py +++ b/src/secretsync/infrastructure/http.py @@ -1,19 +1,25 @@ -"""httpx client factory with safe retries and no wire logging.""" +"""httpx client factory with safe retries and bounded provider-error diagnostics.""" from __future__ import annotations import asyncio import random +from collections.abc import Sequence from dataclasses import dataclass, field from typing import Any import httpx +from loguru import logger -from secretsync.destinations.base import SafeConnectorError -from secretsync.infrastructure.redaction import SENSITIVE_HEADER_NAMES +from secretsync.domain.errors import SafeError +from secretsync.infrastructure.redaction import SENSITIVE_HEADER_NAMES, sanitize_provider_message RETRYABLE_STATUS = frozenset({429, 502, 503, 504}) DEFAULT_MAX_ATTEMPTS = 5 +BOUNDED_PROVIDER_ERROR = 512 + +# Spec alias: connector-facing errors are SafeError payloads. +SafeConnectorError = SafeError @dataclass(frozen=True, slots=True) @@ -52,34 +58,94 @@ def response_debug_meta(response: httpx.Response) -> dict[str, object]: } +def provider_error_detail( + response: httpx.Response, + secrets: Sequence[str] | None = None, +) -> str | None: + """Bounded, redacted provider error text for SafeError / audit / verbose logs.""" + raw = (response.text or "")[: BOUNDED_PROVIDER_ERROR * 2] + if not raw.strip(): + return None + + detail: str | None = None + try: + payload = response.json() + except (ValueError, httpx.DecodingError): + payload = None + + if isinstance(payload, dict): + message = payload.get("message") + if isinstance(message, str) and message.strip(): + detail = message.strip() + else: + error = payload.get("error") + if isinstance(error, dict): + bits: list[str] = [] + code = error.get("code") + emsg = error.get("message") + if isinstance(code, str) and code.strip(): + bits.append(code.strip()) + if isinstance(emsg, str) and emsg.strip(): + bits.append(emsg.strip()) + if bits: + detail = ": ".join(bits) + elif isinstance(error, str) and error.strip(): + detail = error.strip() + + if detail is None: + detail = " ".join(raw.split()) + + detail = sanitize_provider_message(detail, list(secrets) if secrets else None) + if len(detail) > BOUNDED_PROVIDER_ERROR: + detail = detail[:BOUNDED_PROVIDER_ERROR] + "…" + return detail or None + + def error_for_status( - status_code: int, + response_or_status: httpx.Response | int, *, mutation_id: str | None = None, correlation_id: str | None = None, + secrets: Sequence[str] | None = None, ) -> SafeConnectorError: - if status_code in {401, 403}: - return SafeConnectorError( - code="DESTINATION_PERMISSION_DENIED", - message=f"Provider rejected authorization (HTTP {status_code})", - mutation_id=mutation_id, - correlation_id=correlation_id, - retryable=False, - ) - if status_code == 429: - return SafeConnectorError( - code="DESTINATION_RATE_LIMITED", - message="Provider rate limit remained after bounded retry", - mutation_id=mutation_id, - correlation_id=correlation_id, - retryable=True, + if isinstance(response_or_status, int): + status_code = response_or_status + response: httpx.Response | None = None + detail: str | None = None + else: + response = response_or_status + status_code = response.status_code + detail = provider_error_detail(response, secrets) + + if response is not None: + method = response.request.method if response.request else "?" + logger.debug( + "provider HTTP {} {} -> {}{}", + method, + response.url, + status_code, + f": {detail}" if detail else "", ) + + if status_code in {401, 403}: + base = f"Provider rejected authorization (HTTP {status_code})" + code = "DESTINATION_PERMISSION_DENIED" + retryable = False + elif status_code == 429: + base = "Provider rate limit remained after bounded retry" + code = "DESTINATION_RATE_LIMITED" + retryable = True + else: + base = f"Provider rejected request (HTTP {status_code})" + code = "DESTINATION_INVALID" + retryable = False + return SafeConnectorError( - code="DESTINATION_INVALID", - message=f"Provider rejected request (HTTP {status_code})", + code=code, + message=f"{base}: {detail}" if detail else base, mutation_id=mutation_id, correlation_id=correlation_id, - retryable=False, + retryable=retryable, ) @@ -132,7 +198,7 @@ async def request_with_retries( if last_response is not None: raise HttpRequestError( error_for_status( - last_response.status_code, + last_response, mutation_id=mutation_id, correlation_id=correlation_id, ) diff --git a/src/secretsync/presentation/human.py b/src/secretsync/presentation/human.py index 125c16a..1d767d3 100644 --- a/src/secretsync/presentation/human.py +++ b/src/secretsync/presentation/human.py @@ -83,6 +83,8 @@ def render_apply_human(report: ApplyReport) -> str: if result.error is not None: line += f" [{result.error.code}] {result.error.message}" lines.append(line) + if result.error is not None and result.error.hint: + lines.append(f" hint: {result.error.hint}") if report.cancelled: lines.append("Interrupted: completed writes were not rolled back.") return "\n".join(lines) diff --git a/tests/fixtures/github_org_invalid_visibility.yaml b/tests/fixtures/github_org_invalid_visibility.yaml new file mode 100644 index 0000000..4339307 --- /dev/null +++ b/tests/fixtures/github_org_invalid_visibility.yaml @@ -0,0 +1,23 @@ +version: 1 +changeDetection: always-write +secrets: + apiKey: + env: API_KEY +sets: + production: + include: [apiKey] +destinations: + github: + connector: github-actions + organization: acme + auth: + tokenEnv: GITHUB_TOKEN +deployments: + - name: github-org + set: production + destination: github + scope: + kind: organization + visibility: public + secrets: + apiKey: API_KEY diff --git a/tests/fixtures/github_org_missing_visibility.yaml b/tests/fixtures/github_org_missing_visibility.yaml new file mode 100644 index 0000000..548d0e3 --- /dev/null +++ b/tests/fixtures/github_org_missing_visibility.yaml @@ -0,0 +1,22 @@ +version: 1 +changeDetection: always-write +secrets: + apiKey: + env: API_KEY +sets: + production: + include: [apiKey] +destinations: + github: + connector: github-actions + organization: acme + auth: + tokenEnv: GITHUB_TOKEN +deployments: + - name: github-org + set: production + destination: github + scope: + kind: organization + secrets: + apiKey: API_KEY diff --git a/tests/fixtures/github_org_selected_missing_ids.yaml b/tests/fixtures/github_org_selected_missing_ids.yaml new file mode 100644 index 0000000..91bc95a --- /dev/null +++ b/tests/fixtures/github_org_selected_missing_ids.yaml @@ -0,0 +1,23 @@ +version: 1 +changeDetection: always-write +secrets: + apiKey: + env: API_KEY +sets: + production: + include: [apiKey] +destinations: + github: + connector: github-actions + organization: acme + auth: + tokenEnv: GITHUB_TOKEN +deployments: + - name: github-org + set: production + destination: github + scope: + kind: organization + visibility: selected + secrets: + apiKey: API_KEY diff --git a/tests/integration/test_github_actions.py b/tests/integration/test_github_actions.py index b4e416c..d62673e 100644 --- a/tests/integration/test_github_actions.py +++ b/tests/integration/test_github_actions.py @@ -30,7 +30,7 @@ def _mutation( if visibility is not None: scope["visibility"] = visibility if selected_repository_ids is not None: - scope["selected_repository_ids"] = selected_repository_ids + scope["selectedRepositoryIds"] = selected_repository_ids return PutMutation( mutation_id=f"dep:{name}", name=name, @@ -321,7 +321,7 @@ async def test_organization_selected_requires_repository_ids() -> None: ) assert result.results[0].status == "failed" assert result.results[0].error is not None - assert "selected_repository_ids" in result.results[0].error.message + assert "selectedRepositoryIds" in result.results[0].error.message @pytest.mark.asyncio diff --git a/tests/integration/test_vercel.py b/tests/integration/test_vercel.py index c4a3de5..e03bce4 100644 --- a/tests/integration/test_vercel.py +++ b/tests/integration/test_vercel.py @@ -443,6 +443,7 @@ async def test_conflict_edit_fallback() -> None: @pytest.mark.asyncio @respx.mock async def test_list_names_filters_by_targets() -> None: + """Exact target-set match: multi-target remotes are not owned by single-target scopes.""" respx.get("https://api.vercel.com/v9/projects/web/env").mock( return_value=httpx.Response( 200, @@ -467,7 +468,132 @@ async def test_list_names_filters_by_targets() -> None: OperationContext(correlation_id="c1"), kind=ValueKind.VARIABLE, ) - assert names == frozenset({"KEEP", "BOTH"}) + assert names == frozenset({"KEEP"}) + + +@pytest.mark.asyncio +@respx.mock +async def test_shared_list_exact_targets_partition() -> None: + """Three-way target partition: common must not list production- or preview-only rows.""" + respx.get("https://api.vercel.com/v1/env").mock( + return_value=httpx.Response( + 200, + json={ + "data": [ + { + "id": "1", + "key": "SECRET_THREE_ORG", + "type": "sensitive", + "target": ["production"], + "projectId": [], + }, + { + "id": "2", + "key": "SECRET_THREE_ORG", + "type": "sensitive", + "target": ["preview"], + "projectId": [], + }, + { + "id": "3", + "key": "SECRET_TWO", + "type": "sensitive", + "target": ["production", "preview"], + "projectId": [], + }, + { + "id": "4", + "key": "LOG_LEVEL", + "type": "encrypted", + "target": ["preview"], + "projectId": [], + }, + { + "id": "5", + "key": "ORG_NAME", + "type": "encrypted", + "target": ["production", "preview"], + "projectId": [], + }, + ], + "pagination": {"count": 5, "next": None, "prev": None}, + }, + ) + ) + dest = VercelFactory().create(_services()) + common_secrets = await dest.list_names( + _dest_config(), # type: ignore[arg-type] + {"kind": "shared-environment", "targets": ["production", "preview"]}, # type: ignore[arg-type] + OperationContext(correlation_id="c1"), + kind=ValueKind.SECRET, + ) + preview_secrets = await dest.list_names( + _dest_config(), # type: ignore[arg-type] + {"kind": "shared-environment", "targets": ["preview"]}, # type: ignore[arg-type] + OperationContext(correlation_id="c1"), + kind=ValueKind.SECRET, + ) + preview_vars = await dest.list_names( + _dest_config(), # type: ignore[arg-type] + {"kind": "shared-environment", "targets": ["preview"]}, # type: ignore[arg-type] + OperationContext(correlation_id="c1"), + kind=ValueKind.VARIABLE, + ) + assert common_secrets == frozenset({"SECRET_TWO"}) + assert preview_secrets == frozenset({"SECRET_THREE_ORG"}) + assert preview_vars == frozenset({"LOG_LEVEL"}) + + +@pytest.mark.asyncio +@respx.mock +async def test_shared_upsert_exact_targets_creates_when_overlap_only() -> None: + """Preview-only put must create, not patch a production-only row with the same key.""" + respx.get("https://api.vercel.com/v1/env").mock( + return_value=httpx.Response( + 200, + json={ + "data": [ + { + "id": "env_prod", + "key": "SECRET_THREE_ORG", + "type": "sensitive", + "target": ["production"], + "projectId": [], + } + ], + "pagination": {"count": 1, "next": None, "prev": None}, + }, + ) + ) + create = respx.post("https://api.vercel.com/v1/env").mock( + return_value=httpx.Response(201, json={"created": [], "failed": []}) + ) + patch = respx.patch("https://api.vercel.com/v1/env").mock( + return_value=httpx.Response(200, json={"updated": [], "failed": []}) + ) + dest = VercelFactory().create(_services()) + result = await dest.apply( + ApplyDestinationRequest( + deployment_id="preview", + destination_config=_dest_config(), # type: ignore[arg-type] + mutations=[ + _mutation( + "SECRET_THREE_ORG", + scope_kind="shared-environment", + targets=["preview"], + value=b"staging-value", + ) + ], + ), + OperationContext(correlation_id="c1"), + ) + assert result.results[0].status == "applied" + assert result.results[0].effect == "upserted" + assert create.called + assert not patch.called + body = json.loads(create.calls[0].request.read()) + assert body["target"] == ["preview"] + assert body["evs"][0]["key"] == "SECRET_THREE_ORG" @pytest.mark.asyncio @@ -622,6 +748,53 @@ async def test_shared_patch_existing() -> None: assert body["updates"]["env_shared_1"]["projectId"] == ["prj_a", "prj_b"] +@pytest.mark.asyncio +@respx.mock +async def test_shared_patch_omits_empty_projects() -> None: + respx.get("https://api.vercel.com/v1/env").mock( + return_value=httpx.Response( + 200, + json={ + "data": [ + { + "id": "env_unlinked", + "key": "SECRET_TWO", + "type": "sensitive", + "target": ["production", "preview"], + "projectId": [], + } + ], + "pagination": {"count": 1, "next": None, "prev": None}, + }, + ) + ) + patch = respx.patch("https://api.vercel.com/v1/env").mock( + return_value=httpx.Response(200, json={"updated": [], "failed": []}) + ) + dest = VercelFactory().create(_services()) + result = await dest.apply( + ApplyDestinationRequest( + deployment_id="common", + destination_config=_dest_config(), # type: ignore[arg-type] + mutations=[ + _mutation( + "SECRET_TWO", + scope_kind="shared-environment", + targets=["production", "preview"], + value=b"rotated", + ) + ], + ), + OperationContext(correlation_id="c1"), + ) + assert result.results[0].status == "applied" + assert result.results[0].effect == "updated" + assert patch.called + body = json.loads(patch.calls[0].request.read()) + assert "env_unlinked" in body["updates"] + assert "projectId" not in body["updates"]["env_unlinked"] + + @pytest.mark.asyncio @respx.mock async def test_shared_list_exact_projects_match() -> None: diff --git a/tests/security/test_canary_channels.py b/tests/security/test_canary_channels.py index d2e90c5..88a3935 100644 --- a/tests/security/test_canary_channels.py +++ b/tests/security/test_canary_channels.py @@ -120,9 +120,16 @@ async def test_canary_http_error_bodies() -> None: response = await request_with_retries(client, "GET", "https://example.test/secret") assert route.called assert response.status_code == 403 - # Body must never be pulled into SafeError helpers — only status mapping from secretsync.infrastructure.http import error_for_status + # Status-only mapping stays value-free. err = error_for_status(403, correlation_id="c1") assert_canary_absent(err.message, label="http safe error") assert CANARY not in (err.hint or "") + + # Provider body may be included when redacting known secrets. + err_with_body = error_for_status(response, correlation_id="c1", secrets=[CANARY]) + assert_canary_absent(err_with_body.message, label="http safe error with body") + assert "Provider rejected authorization (HTTP 403)" in err_with_body.message + assert "denied" in err_with_body.message + assert "***" in err_with_body.message diff --git a/tests/unit/test_audit.py b/tests/unit/test_audit.py index 5c94d7a..23ec630 100644 --- a/tests/unit/test_audit.py +++ b/tests/unit/test_audit.py @@ -65,3 +65,27 @@ def test_apply_writes_mutation_audit_without_values(tmp_path) -> None: assert "name=DATABASE_URL" in log assert "name=STRIPE_SECRET_KEY" in log assert "name=API_TOKEN" in log + + +def test_apply_writes_failed_mutation_error_message(tmp_path) -> None: + from secretsync.infrastructure.audit import record_mutation_audit + + path = record_mutation_audit( + config_path=None, + run_id="runfail00001", + destination_id="github-org", + connector_id="github-actions", + deployment_id="production", + op="put", + name="SECRET_THREE_ORG", + scope={"kind": "organization", "visibility": "all"}, + status="failed", + effect=None, + correlation_id="corr-1", + error_code="DESTINATION_INVALID", + error_message="Provider rejected request (HTTP 404): Not Found", + cwd=tmp_path, + ) + line = path.read_text(encoding="utf-8").strip() + assert 'error_message="Provider rejected request (HTTP 404): Not Found"' in line + assert "error=DESTINATION_INVALID" in line diff --git a/tests/unit/test_http_retry.py b/tests/unit/test_http_retry.py index 4f3f503..cdb8349 100644 --- a/tests/unit/test_http_retry.py +++ b/tests/unit/test_http_retry.py @@ -4,7 +4,12 @@ import pytest import respx -from secretsync.infrastructure.http import HttpRequestError, request_with_retries +from secretsync.infrastructure.http import ( + HttpRequestError, + error_for_status, + provider_error_detail, + request_with_retries, +) @pytest.mark.asyncio @@ -35,7 +40,9 @@ async def test_non_retryable_401_returns_immediately() -> None: @pytest.mark.asyncio @respx.mock async def test_exhausted_retries_raise_rate_limited() -> None: - respx.get("https://example.test/limited").mock(return_value=httpx.Response(429)) + respx.get("https://example.test/limited").mock( + return_value=httpx.Response(429, json={"message": "slow down"}) + ) async with httpx.AsyncClient() as client: with pytest.raises(HttpRequestError) as exc: await request_with_retries( @@ -45,6 +52,7 @@ async def test_exhausted_retries_raise_rate_limited() -> None: max_attempts=3, ) assert exc.value.safe.code == "DESTINATION_RATE_LIMITED" + assert "slow down" in exc.value.safe.message @pytest.mark.asyncio @@ -55,3 +63,32 @@ async def test_400_not_retried() -> None: response = await request_with_retries(client, "POST", "https://example.test/bad") assert response.status_code == 400 assert route.call_count == 1 + + +def test_error_for_status_includes_github_message() -> None: + response = httpx.Response( + 404, + json={"message": "Not Found", "documentation_url": "https://docs.github.com"}, + request=httpx.Request("PUT", "https://api.github.com/orgs/acme/actions/secrets/X"), + ) + err = error_for_status(response, correlation_id="c1") + assert err.code == "DESTINATION_INVALID" + assert err.message == "Provider rejected request (HTTP 404): Not Found" + + +def test_error_for_status_includes_vercel_error() -> None: + response = httpx.Response( + 400, + json={"error": {"code": "bad_request", "message": "Invalid key"}}, + request=httpx.Request("POST", "https://api.vercel.com/v10/projects/p/env"), + ) + err = error_for_status(response) + assert "bad_request: Invalid key" in err.message + + +def test_provider_error_detail_redacts_secrets() -> None: + response = httpx.Response(400, json={"message": "value sk_live_x is invalid"}) + detail = provider_error_detail(response, secrets=["sk_live_x"]) + assert detail is not None + assert "sk_live_x" not in detail + assert "***" in detail diff --git a/tests/unit/test_validate_plan.py b/tests/unit/test_validate_plan.py index d76e928..1b0c838 100644 --- a/tests/unit/test_validate_plan.py +++ b/tests/unit/test_validate_plan.py @@ -28,6 +28,12 @@ def test_unknown_secret_in_deployment() -> None: result = validate_config(services, fixture_path("unknown_secret.yaml")) assert not result.ok assert result.issues[0].code == "CONFIG_INVALID" + message = result.issues[0].message + assert "missingSecret" in message + assert "set 's'" in message + assert "deployment 'd1'" in message + assert "destination 'github'" in message + assert result.issues[0].hint is not None def test_duplicate_target_rejected() -> None: @@ -149,6 +155,27 @@ def test_vercel_environment_requires_project_offline() -> None: assert "project" in result.issues[0].message.lower() or "project" in result.issues[0].message +def test_github_org_rejects_invalid_visibility_offline() -> None: + services = create_services({"API_KEY": "x", "GITHUB_TOKEN": "t"}) + result = validate_config(services, fixture_path("github_org_invalid_visibility.yaml")) + assert not result.ok + assert "visibility" in result.issues[0].message + + +def test_github_org_requires_visibility_offline() -> None: + services = create_services({"API_KEY": "x", "GITHUB_TOKEN": "t"}) + result = validate_config(services, fixture_path("github_org_missing_visibility.yaml")) + assert not result.ok + assert "visibility" in result.issues[0].message + + +def test_github_org_selected_requires_repository_ids_offline() -> None: + services = create_services({"API_KEY": "x", "GITHUB_TOKEN": "t"}) + result = validate_config(services, fixture_path("github_org_selected_missing_ids.yaml")) + assert not result.ok + assert "selectedRepositoryIds" in result.issues[0].message + + def test_mixed_variables_plan_emits_kinds() -> None: from secretsync.domain.models import ValueKind