Skip to content

fix(api): extend federation target/source guard to azure-self-source and gcp-self-source combos #140

@cristim

Description

@cristim

Summary

Hardening follow-up to #42 (closed). The current validateFederationTargetSource only rejects the AWS-cross-account combo (target=aws & source=aws & sourceCloud()!=aws). Two analogous combos slip through and produce broken bundles that fail at terraform apply instead of at download time:

Requested target/source Requires CUDly source identity Ships broken when CUDly runs on
target=azure & source=azure Azure subscription + tenant ID (CUDly's) AWS, GCP
target=gcp & source=gcp GCP project ID (CUDly's) AWS, Azure

These combos render an Azure / GCP "self-source" bundle whose subscription_id / tenant_id / project_id placeholders end up empty (the source identity isn't populated when sourceCloud() doesn't match), producing a tfvars file that breaks at apply with a missing-field error rather than a clean HTTP 400 at download.

Note on severity: strictly milder than #42's AWS case — the failure is a generic "missing field at apply" rather than the IAM trust-policy mismatch #42 covered. That's why the original issue's table didn't include them. Filing as a hardening follow-up so the consistency is explicit and the catch happens at the same layer.

Current behaviour

internal/api/handler_federation.go::validateFederationTargetSource (line ~241):

func validateFederationTargetSource(target, source string) error {
    if target == "aws" && source == "aws" && sourceCloud() != "aws" {
        return NewClientError(400, fmt.Sprintf(
            "target=aws-cross-account requires CUDly to be deployed on AWS; "+
                "this deployment is on %s", sourceCloud()))
    }
    return nil
}

The Azure-self-source and GCP-self-source equivalents are missing.

Expected behaviour

target=azure-self-source requires CUDly to be deployed on Azure; this deployment is on aws
target=gcp-self-source requires CUDly to be deployed on GCP; this deployment is on azure

(The exact target labels can be debated — azure-self-source / gcp-self-source mirror the existing aws-cross-account naming. Pick whatever reads best in the UI.)

Proposed fix

Generalize the guard to a single rule: "self-source bundles require CUDly to be deployed on the matching cloud":

func validateFederationTargetSource(target, source string) error {
    if target == source && sourceCloud() != target {
        return NewClientError(400, fmt.Sprintf(
            "target=%s-self-source requires CUDly to be deployed on %s; "+
                "this deployment is on %s", target, strings.ToUpper(target), sourceCloud()))
    }
    return nil
}

This subsumes #42's original AWS check and adds the Azure/GCP coverage in one rule. The existing #42 test case (cudly-on-aws-allows-aws-cross-account) becomes the regression guard for the general form.

Test plan

Extend TestGetFederationIaC_RejectsImpossibleTargetSourceCombo with the new combos:

CUDLY_SOURCE_CLOUD target source expected
aws azure azure 400
gcp azure azure 400
azure azure azure 200 (regression guard)
aws gcp gcp 400
azure gcp gcp 400
gcp gcp gcp 200 (regression guard)

Plus existing AWS rows already covered by #42.

References

Severity

Low — same severity reasoning as #42 (rare edge case, current behaviour is "ships and breaks at apply" rather than "ships and breaks at trust-policy check"; a clear 400 at download time is strictly better but not a regression).

Effort

Small — ~5 LOC (or fewer if the generalized form lands), 4–6 new test rows.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions