From a8f85968ff238b7c058588fffa901b5a46bdfc95 Mon Sep 17 00:00:00 2001 From: captainpacket Date: Wed, 22 Jul 2026 06:55:25 -0500 Subject: [PATCH] feat: harden production operations and release workflows --- .github/RELEASE_NOTES_TEMPLATE.md | 65 ++-- .github/workflows/ci.yml | 26 +- .github/workflows/release.yml | 58 ++-- CONTRIBUTING.md | 17 ++ Makefile | 20 +- README.md | 456 +++++++---------------------- SECURITY.md | 11 + cmd/awssync/main.go | 7 + cmd/awssync/main_test.go | 9 + docs/architecture-flow.md | 3 + docs/aws-account-sync-procedure.md | 5 + docs/quick-start.md | 4 + internal/api/client.go | 151 +++++++--- internal/api/client_test.go | 114 ++++++++ internal/app/run.go | 42 ++- internal/app/run_test.go | 37 +++ 16 files changed, 560 insertions(+), 465 deletions(-) create mode 100644 CONTRIBUTING.md create mode 100644 SECURITY.md diff --git a/.github/RELEASE_NOTES_TEMPLATE.md b/.github/RELEASE_NOTES_TEMPLATE.md index f6b1e02..bc9f24b 100644 --- a/.github/RELEASE_NOTES_TEMPLATE.md +++ b/.github/RELEASE_NOTES_TEMPLATE.md @@ -2,64 +2,41 @@ ### Highlights -- Added `--max-removals` and `--max-removal-percent` blast-radius ceilings across NQE sync, manifest sync, saved-plan apply, preflight, and webhook workflows. -- Added release installation, checksum, provenance verification, automation audit handling, and explicit External ID rollback guidance. -- Fixed the release checksum manifest so downloaded assets verify directly with `sha256sum -c sha256sums.txt`. -- Added reversible one-time External ID migration for existing AWS setups with `external-id --value` and `external-id --clear`. -- Added AWS GovCloud workflows for both regular Forward Organizations/NQE discovery and reviewed standalone-account manifests. -- Added `onboard-accounts` and `sync-accounts` for environments where AWS Organizations is unavailable by policy. -- Preserved `arn:aws-us-gov` IAM role partitions and rejected mixed or region-mismatched role ARNs. -- Blocked GovCloud removals without positive Organizations evidence; authoritative manifest removals require explicit review and `--allow-removals`. -- Added collector instance-profile onboarding payloads for self-managed GovCloud collectors. -- Hardened `apply-plan` so saved payloads cannot bypass current-state or GovCloud removal validation. -- Added a dedicated GovCloud operator guide with product-enhancement escalation criteria. +- Generated payload, manual, and audit files are now atomically replaced with owner-only `0600` permissions, including outputs that may contain static AWS credentials. +- Forward API reads, NQE queries, and full-state PATCH operations retry bounded transient `429`, `502`, `503`, and `504` responses. Non-idempotent create POSTs remain single-attempt. +- `awssync --version` now reports the release, source commit, and build date. +- CI now runs formatting, vet, tests, the race detector, and `govulncheck` with read-only repository permissions and commit-pinned actions. +- Release jobs use least-privilege permissions and continue to publish checksums and build-provenance attestations. +- The README now starts with the workflow decision diagram and routes detailed operator procedures to focused runbooks. +- Contribution guidance requires human attribution and excludes automation/tool identities from contributor metadata. ### Download and verify -Assets include platform binaries, tarballs, checksums, and release attestations: - -- `awssync-linux-amd64` -- `awssync-linux-arm64` -- `awssync-darwin-amd64` -- `awssync-darwin-arm64` -- `awssync-linux-amd64.tar.gz` -- `awssync-linux-arm64.tar.gz` -- `awssync-darwin-amd64.tar.gz` -- `awssync-darwin-arm64.tar.gz` -- `sha256sums.txt` - -### Quick usage +Assets include native Linux and macOS binaries for amd64 and arm64, tar archives, `sha256sums.txt`, and GitHub build-provenance attestations. ```bash -# Add a customer-defined External ID to an existing setup -./awssync external-id \ - --network-id \ - --setup-id \ - --value \ - --output aws_external_id_payload.json +tar -xzf awssync-linux-amd64.tar.gz +sha256sum -c sha256sums.txt --ignore-missing +gh attestation verify awssync-linux-amd64 \ + --repo forwardnetworks/aws-sync +./awssync-linux-amd64 --version +``` -# Generate a GovCloud onboarding payload from a reviewed manifest -./awssync onboard-accounts \ - --accounts-file govcloud-accounts.json \ - --partition aws-us-gov \ - --credential-mode instance-profile \ - --setup-id \ - --role-name ForwardReadOnlyAccess \ - --collect-region us-gov-west-1 +### Start safely -# Dry-run an existing setup against an authoritative manifest -./awssync sync-accounts \ +```bash +./awssync-linux-amd64 preflight \ --network-id \ --setup-id \ - --accounts-file govcloud-accounts.json \ + --max-snapshot-age 24h \ --format human -# Verify the regular Organizations/NQE path before applying -./awssync preflight \ +./awssync-linux-amd64 \ --network-id \ --setup-id \ --max-snapshot-age 24h \ + --output aws_sync_payload.json \ --format human ``` -See `docs/govcloud-workflow.md` for the complete GovCloud Organizations and standalone-account procedures. +See the README workflow diagram, `docs/aws-account-sync-procedure.md`, and `docs/govcloud-workflow.md` before enabling apply automation or account removals. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d377067..343a9d4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -6,20 +6,34 @@ on: branches: - main +permissions: + contents: read + +concurrency: + group: ci-${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + jobs: test: runs-on: ubuntu-latest + timeout-minutes: 20 steps: - - uses: actions/checkout@v7 - - uses: actions/setup-go@v6 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 + with: + persist-credentials: false + - uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6 with: go-version-file: go.mod cache: true - name: Check formatting - run: test -z "$(gofmt -l ./cmd ./internal)" + run: make fmt-check - name: Vet - run: go vet ./... + run: make vet - name: Test - run: go test ./... + run: make test + - name: Race detector + run: make race + - name: Vulnerability scan + run: make vuln - name: Build - run: go build -o bin/awssync ./cmd/awssync + run: make build diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index fe7ba82..89bb3d8 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -1,8 +1,4 @@ name: release -permissions: - contents: write - id-token: write - attestations: write on: workflow_dispatch: @@ -10,24 +6,35 @@ on: tags: - "v*" +permissions: + contents: read + jobs: test: runs-on: ubuntu-latest + timeout-minutes: 20 steps: - - uses: actions/checkout@v7 - - uses: actions/setup-go@v6 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 + with: + persist-credentials: false + - uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6 with: go-version-file: go.mod cache: true - name: Check formatting - run: test -z "$(gofmt -l ./cmd ./internal)" + run: make fmt-check - name: Vet - run: go vet ./... + run: make vet - name: Test - run: go test ./... + run: make test + - name: Race detector + run: make race + - name: Vulnerability scan + run: make vuln build: runs-on: ubuntu-latest + timeout-minutes: 15 needs: test strategy: matrix: @@ -41,8 +48,10 @@ jobs: - goos: darwin goarch: arm64 steps: - - uses: actions/checkout@v7 - - uses: actions/setup-go@v6 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 + with: + persist-credentials: false + - uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6 with: go-version-file: go.mod cache: true @@ -51,22 +60,35 @@ jobs: GOOS: ${{ matrix.goos }} GOARCH: ${{ matrix.goarch }} CGO_ENABLED: "0" + VERSION: ${{ github.ref_name }} + COMMIT: ${{ github.sha }} run: | - mkdir -p dist - go build -trimpath -o "dist/awssync-${GOOS}-${GOARCH}" ./cmd/awssync - - uses: actions/upload-artifact@v7 + BUILD_DATE="$(git show -s --format=%cI "$GITHUB_SHA")" + make build \ + BINARY="dist/awssync-${GOOS}-${GOARCH}" \ + VERSION="${VERSION}" \ + COMMIT="${COMMIT}" \ + BUILD_DATE="${BUILD_DATE}" + - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 with: name: awssync-${{ matrix.goos }}-${{ matrix.goarch }} path: dist/awssync-${{ matrix.goos }}-${{ matrix.goarch }} publish: runs-on: ubuntu-latest + timeout-minutes: 15 needs: build if: startsWith(github.ref, 'refs/tags/') + permissions: + contents: write + id-token: write + attestations: write steps: - - uses: actions/checkout@v7 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 + with: + persist-credentials: false - name: Download all build artifacts - uses: actions/download-artifact@v8 + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8 with: path: dist merge-multiple: true @@ -103,7 +125,7 @@ jobs: sha256sum -c sha256sums.txt ) - name: Generate build provenance - uses: actions/attest-build-provenance@v4 + uses: actions/attest-build-provenance@0f67c3f4856b2e3261c31976d6725780e5e4c373 # v4 with: subject-path: | dist/awssync-linux-amd64 @@ -118,7 +140,7 @@ jobs: run: | sed "s|{{VERSION}}|${{ github.ref_name }}|g" .github/RELEASE_NOTES_TEMPLATE.md > dist/release-notes.md - name: Publish GitHub release - uses: softprops/action-gh-release@v3 + uses: softprops/action-gh-release@3d0d9888cb7fd7b750713d6e236d1fcb99157228 # v3 with: tag_name: ${{ github.ref_name }} name: ${{ github.ref_name }} diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000..f0cd8b5 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,17 @@ +# Contributing + +## Changes + +Open a focused pull request with tests and documentation for operator-visible behavior. Run the same checks used by CI before publishing: + +```bash +make ci +``` + +Keep destructive behavior opt-in, preserve dry-run output, and add regression tests for safety checks. Never include customer credentials, tenant data, generated payloads, or private communications in commits, issues, test fixtures, or workflow logs. + +## Attribution + +Commits and pull requests must identify the human authors responsible for the change. Automation and generative tools are tools, not contributors: do not add tool identities through `Author`, `Co-authored-by`, contributor lists, acknowledgements, or similar attribution metadata. + +Use a verified human email address for commits. Maintainers may ask for attribution metadata to be corrected before merge. diff --git a/Makefile b/Makefile index e24e2aa..e99b391 100644 --- a/Makefile +++ b/Makefile @@ -1,17 +1,31 @@ BINARY := bin/awssync +VERSION ?= dev +COMMIT ?= $(shell git rev-parse --short=12 HEAD 2>/dev/null || printf unknown) +BUILD_DATE ?= unknown +LDFLAGS := -X main.version=$(VERSION) -X main.commit=$(COMMIT) -X main.buildDate=$(BUILD_DATE) -.PHONY: build test fmt vet ci +.PHONY: build test race fmt fmt-check vet vuln ci build: - go build -o $(BINARY) ./cmd/awssync + mkdir -p $(dir $(BINARY)) + go build -trimpath -buildvcs=false -ldflags "$(LDFLAGS)" -o $(BINARY) ./cmd/awssync test: go test ./... +race: + go test -race ./... + fmt: gofmt -w ./cmd ./internal +fmt-check: + test -z "$$(gofmt -l ./cmd ./internal)" + vet: go vet ./... -ci: fmt vet test build +vuln: + go run golang.org/x/vuln/cmd/govulncheck@v1.6.0 ./... + +ci: fmt-check vet test race vuln build diff --git a/README.md b/README.md index 2593cf0..04a14d4 100644 --- a/README.md +++ b/README.md @@ -1,95 +1,67 @@ # aws-sync -`awssync` discovers AWS accounts through Forward NQE, builds one PATCH payload per existing AWS cloud setup, writes those payloads to disk, and can optionally PATCH them back into Forward. -It also has a `discover-org` onboarding mode that reads AWS Organizations directly, writes the Forward UI `fwd_accounts_data` upload JSON, and writes the Forward create-setup POST JSON for new AWS setups. +`awssync` safely reconciles AWS account inventory with Forward Networks AWS cloud setups. It supports AWS Organizations discovery, reviewed account manifests, GovCloud, customer-defined External IDs, dry plans, guarded apply, and snapshot-ready automation. -For new AWS Organizations onboarding, the native IaC workflow is now the Forward Terraform provider. Use Terraform when the organization can use a stable Forward collection role name across accounts and one of Forward's supported credential models: Forward assume-role, static collector keys, or collector instance profile. Use `awssync discover-org` when you need manual JSON files, a break-glass workflow, or static-key collector payloads that should stay outside Terraform state. +## Choose a Workflow -The repository is structured like `awsfilter`: Cobra/Viper CLI entrypoint, raw API client package, and isolated run/planning logic with tests. +```mermaid +flowchart TD + A[What are you doing?] -->|Update an existing setup| B{Complete Organizations inventory
is visible in Forward NQE?} + A -->|Create a new setup| C{Can the customer use
AWS Organizations?} + A -->|Change External ID only| X[Run external-id dry plan
then apply once] -## What it does + B -->|Yes| D[Use preflight and the default NQE sync] + B -->|No or standalone GovCloud accounts| E[Use a reviewed authoritative manifest] + C -->|Yes| F[Prefer Forward Terraform provider
Use discover-org for CLI/manual fallback] + C -->|No| G[Use onboard-accounts with
a reviewed authoritative manifest] -1. Calls `POST /api/nqe?networkId={networkId}` and pages through AWS account rows using the selected NQE query. When `--snapshot-id` is provided, the NQE run is pinned to that snapshot. -2. Calls `GET /api/networks/{networkId}/cloudAccounts` to load existing AWS setup metadata. -3. Groups discovered accounts by setup ID when the query includes that column. -4. Rebuilds `assumeRoleInfos` for each eligible AWS setup using the existing role name, optional external ID, proxy server ID, and region timestamps. -5. Writes the full PATCH payload map to JSON. -6. Optionally writes setup-keyed manual JSON for UI drag-and-drop workflows. -7. Optionally calls `PATCH /api/networks/{networkId}/cloudAccounts/{setupId}` for each planned setup. - -`discover-org` is separate. It is for a new Forward AWS setup that is not onboarded yet, and it does not PATCH existing setups. It uses the AWS SDK default credential chain or `--aws-profile` to call `organizations:DescribeOrganization`, `organizations:ListAccounts`, and `organizations:ListParents`, then writes: - -- `fwd_accounts_data_.json`: flat account array for the Forward UI drag-and-drop flow. -- `aws_create_payload_.json`: body for `POST /api/networks/{networkId}/cloudAccounts`. - -Terraform examples for AWS-side prerequisites live in [examples/terraform](examples/terraform). They create AWS Organizations read roles, Forward collection roles through StackSets, and an optional GitHub OIDC role for running `discover-org` without long-lived AWS keys. For a fully Terraform-native Forward onboarding workflow, use the Forward Terraform provider's `forward_aws_assume_role_external_id`, `forward_aws_organization_accounts`, and `forward_aws_cloud_account` resources/data sources. - -The Forward collection IAM role name must be the same in every AWS account that should be collected. `awssync` uses the role name from the existing Forward AWS setup as the template for generated role ARNs. - -Both Forward IAM role and IAM user/access-key multi-account setups are supported. In IAM user/access-key mode, Forward still uses the configured access key to assume the per-account role ARNs in `assumeRoleInfos`; the PATCH updates those account entries and leaves stored credentials unchanged. - -An existing setup can add, replace, or clear its per-account External ID without changing those stored credentials. Use the dedicated one-time migration command; its dry run reads the current setup directly and does not depend on NQE or a new snapshot: - -```bash -./bin/awssync external-id \ - --setup-id AWS-PROD \ - --value customer-defined-value \ - --output aws_external_id_payload.json \ - --format human - -./bin/awssync external-id \ - --setup-id AWS-PROD \ - --value customer-defined-value \ - --output aws_external_id_payload.json \ - --apply \ - --yes + D --> H[Create and review dry plan] + E --> H + F --> H + G --> H + H --> I{Any removals?} + I -->|No| J[Apply] + I -->|Yes| K[Verify account lifecycle independently
Set allow-removals and blast-radius limits] + K --> J ``` -The value is written to every existing `assumeRoleInfos` entry for that setup. Review and apply the Forward payload first, test a representative account, and then update the target-role trust policies to require the identical value. After the migration PATCH, normal syncs preserve the stored External ID without rerunning this command. For rollback, relax or remove the mandatory `sts:ExternalId` trust-policy condition first, confirm the role can still be assumed, and only then apply `external-id --clear`. Stored IAM access keys and secrets are not included in or changed by the PATCH. - -## Procedure +The key choice is the inventory source. Use Forward NQE only when a current snapshot contains complete AWS Organizations evidence. Use a reviewed manifest when Organizations is unavailable, intentionally excluded, or not reliably represented—including standalone GovCloud environments. -For an end-to-end flow diagram showing connection types and required permissions, see [AWS Account Sync End-to-End Flow](docs/architecture-flow.md). +## Safety Model -For a short quick start, see [AWS Account Sync Quick Start](docs/quick-start.md). +- Dry run is the default; writes require `--apply` and confirmation or `--yes`. +- Additions can be automated. Removals are blocked unless `--allow-removals` is explicit. +- `--max-removals` and `--max-removal-percent` impose independent blast-radius ceilings. +- Empty candidate inventory, stale snapshots, missing Organizations evidence, and unsafe GovCloud removal plans fail closed. +- Saved plans are revalidated against current Forward state before apply. +- Generated payload and audit files are written atomically with owner-only `0600` permissions. +- Transient API failures are retried only for idempotent reads and full-state updates; create operations are never automatically retried. -For the full procedure, including AWS Organizations prerequisites, management-account or delegated-account discovery checks, IAM role checks, dry-run review, apply, and post-apply validation, see [AWS Account Sync Procedure](docs/aws-account-sync-procedure.md). +## Install a Verified Release -For GovCloud Organizations and standalone-account workflows, including collector instance-profile credentials, GovCloud ARN validation, a reviewed account-manifest fallback, and stricter removal gates, see [AWS GovCloud Account Workflow](docs/govcloud-workflow.md). - -## Build +Download the archive and checksum manifest for the required platform from [Releases](https://github.com/forwardnetworks/aws-sync/releases): ```bash -make build +tar -xzf awssync-linux-amd64.tar.gz +chmod +x awssync-linux-amd64 +sha256sum -c sha256sums.txt --ignore-missing +gh attestation verify awssync-linux-amd64 \ + --repo forwardnetworks/aws-sync +./awssync-linux-amd64 --version ``` -## Install a Release +Release assets are available for Linux and macOS on amd64 and arm64. Each release includes SHA-256 checksums and GitHub build-provenance attestations. -Prefer the tarball because it preserves the executable bit. Download the tarball and checksum manifest for the required platform, verify both the checksum and GitHub build provenance, then extract it: +To build locally: ```bash -VERSION=v2.1.2 -PLATFORM=linux-amd64 - -gh release download "$VERSION" \ - --repo forwardnetworks/aws-sync \ - --pattern "awssync-${PLATFORM}.tar.gz" \ - --pattern sha256sums.txt - -grep " awssync-${PLATFORM}.tar.gz$" sha256sums.txt | sha256sum -c - -gh attestation verify "awssync-${PLATFORM}.tar.gz" \ - --repo forwardnetworks/aws-sync \ - --signer-workflow forwardnetworks/aws-sync/.github/workflows/release.yml - -tar -xzf "awssync-${PLATFORM}.tar.gz" -./"awssync-${PLATFORM}" --help +make build +./bin/awssync --version ``` -On macOS, use `PLATFORM=darwin-amd64` or `PLATFORM=darwin-arm64` and replace `sha256sum -c -` with `shasum -a 256 -c -`. A raw binary downloaded directly from GitHub may need `chmod +x`; the tarball does not. - -## Usage +## Existing Setup Quick Start -Set common inputs through environment variables: +Set credentials without putting the password on the command line: ```bash export FWD_HOST=https://fwd.app @@ -98,336 +70,110 @@ export FWD_PASS='secret' export FWD_NETWORK_ID=NETWORK_ID ``` -Use the Forward base URL for `FWD_HOST`; it can be SaaS or an on-prem Forward instance. - -Plan and write payloads only: - -```bash -./bin/awssync -``` +In automation, always supply the network and setup IDs explicitly. Interactive runs can prompt when more than one is visible. -Run preflight checks before planning or applying: +Check the current snapshot and Organizations evidence: ```bash ./bin/awssync preflight \ - --max-snapshot-age 24h -``` - -Use readable output with: - -```bash -./bin/awssync preflight --format human -./bin/awssync --format human -``` - -`--format` accepts `json` (default) or `human`. - -Use `--manual-output` if you also want UI-friendly drag-and-drop JSON: - -```bash -./bin/awssync \ - --manual-output aws_sync_manual_payload.json -``` - -Discover an AWS Organization before Forward has collected it: - -Prefer the Forward Terraform provider for native IaC onboarding. This CLI mode is best for manual review files, break-glass onboarding, or environments that cannot yet use the provider. - -```bash -AWS_PROFILE=org-readonly ./bin/awssync discover-org \ --setup-id AWS-PROD \ - --role-name ForwardRole \ - --collect-region us-east-1 \ - --collect-region us-west-2 \ - --external-id Org:12345 + --max-snapshot-age 24h \ + --format human ``` -If Forward credentials are supplied, `discover-org` can fetch the Forward-generated AWS external ID and validate that the setup name does not already exist: +Create a dry plan: ```bash -AWS_PROFILE=org-readonly ./bin/awssync discover-org \ - --host "$FWD_HOST" \ - --username "$FWD_USER" \ - --password "$FWD_PASS" \ - --network-id "$FWD_NETWORK_ID" \ +./bin/awssync \ --setup-id AWS-PROD \ - --role-name ForwardRole \ - --collect-region us-east-1 -``` - -To create the new Forward setup through the API after writing both JSON files, add `--post --yes`. For static IAM key collection, use `--credential-mode static-keys --collector-access-key-id KEY_ID` and provide the secret through `AWSSYNC_COLLECTOR_SECRET_ACCESS_KEY`; otherwise the create payload contains a placeholder and is not POST-ready. - -Optional Terraform bootstrap for the CLI fallback: - -```bash -terraform -chdir=examples/terraform/aws-org-discovery-role init -terraform -chdir=examples/terraform/aws-org-discovery-role apply - -terraform -chdir=examples/terraform/forward-collection-role-stackset init -terraform -chdir=examples/terraform/forward-collection-role-stackset apply + --max-snapshot-age 24h \ + --output aws_sync_payload.json \ + --format human ``` -Apply the generated payloads back into Forward: +Review `added_accounts`, `removed_accounts`, the role name, External ID state, regions, and payload hash. If no removals are planned, apply the freshly recomputed plan: ```bash ./bin/awssync \ + --setup-id AWS-PROD \ --max-snapshot-age 24h \ - --apply \ - --yes -``` - -When interactive, `--apply` without `--yes` now performs a dry plan pass first and then prompts: - -```text -Planned changes: add=2 remove=0. -Type 'apply' to continue: + --output aws_sync_payload.json \ + --apply --yes ``` -If the plan removes accounts from a Forward setup, `--apply` fails unless `--allow-removals` is also provided. -Use `--max-removals` to cap the aggregate removal count across all selected setups and `--max-removal-percent` to cap each setup independently. Both are optional, apply-time safety ceilings; a value of `0` disables that limit. -If removals are included and no uncollected candidate rows are visible, add `--allow-no-candidates` only after confirming AWS Organizations discovery. -If removals are included and there is no candidate or Organizational Unit signal, add `--allow-no-org-evidence` only after independent discovery verification. -In a run with multiple `--setup-id` values, this is enforced per setup and the check output includes the setup IDs that are missing signals. - -For example, an approved removal run can still be limited to no more than 10 accounts overall and no more than 5% of any setup: +For an independently verified removal, add both approval and quantitative limits: ```bash ./bin/awssync \ - --apply \ - --yes \ + --setup-id AWS-PROD \ + --max-snapshot-age 24h \ + --output aws_sync_payload.json \ --allow-removals \ - --max-removals 10 \ - --max-removal-percent 5 -``` - -Apply a reviewed payload file without recomputing the plan: - -```bash -./bin/awssync apply-plan \ - --plan aws_sync_payload.json \ - --yes -``` - -Run the planner against a specific snapshot: - -```bash -./bin/awssync \ - --snapshot-id SNAPSHOT_ID -``` - -Command-line flags are also supported for one-off runs: - -```bash -./bin/awssync \ - --host https://fwd.app \ - --username you@example.com \ - --password 'secret' \ - --network-id NETWORK_ID -``` - -`AWSSYNC_*` environment variables are also accepted. `FWD_USERNAME` and `FWD_PASSWORD` are accepted for compatibility with the original script. - -`--query-id` is optional. By default, the tool sends an inline Forward NQE source query that includes `cloudAccount.cloudSetupId` as `Cloud Setup ID`, which is required to separate accounts when a network has multiple AWS setups. When exactly one `--setup-id` is selected, the inline query is parameterized with that setup ID so Forward can scope the query before returning rows. Use `--query-id` only when intentionally overriding that query; saved query overrides must also return `Cloud Setup ID` for multi-setup sync. - -If a saved query declares a String setup parameter, pass `--query-setup-param PARAM_NAME` with exactly one `--setup-id`: - -```bash -./bin/awssync preflight \ - --query-id Q_... \ - --query-setup-param setupId \ - --setup-id AWS_SETUP_ID -``` - -`--network-id` can be omitted when the Forward user can see exactly one network. If a terminal is attached and multiple networks are visible, the CLI shows a numbered picker and accepts either the menu number or the network ID. Noninteractive runs should pass `--network-id` explicitly. - -`--setup-id` can be omitted when the network has exactly one eligible AWS setup. -If the network has multiple AWS setups: -- interactive terminal: a setup picker is shown and accepts menu numbers or case-insensitive setup IDs. -- non-interactive: `--setup-id` must be provided (repeat for multiple setups) or the command exits with a selection error. - -The selected setup IDs are shown in `selected_setup_ids` in JSON/human output. - -If the network has multiple AWS setups and only one should be synchronized, scope the run with `--setup-id`: - -```bash -./bin/awssync \ - --setup-id AWS_SETUP_ID + --max-removals 5 \ + --max-removal-percent 2 \ + --apply --yes ``` -Repeat `--setup-id` to sync more than one setup. - -Example output: - -```json -{ - "host": "https://fwd.app", - "network_id": "NETWORK_ID", - "query_override": false, - "output": "/path/to/aws_sync_payload.json", - "manual_output": "/path/to/aws_sync_manual_payload.json", - "payload_sha256": "91f9c6...", - "manual_payload_sha256": "f5b9d4...", - "manual_payloads": { - "collect_aws": [ - { - "accountId": "111111111111", - "accountName": "acct-a", - "roleArn": "arn:aws:iam::111111111111:role/ForwardRole", - "externalId": "Org:12345", - "enabled": true - } - ] - }, - "apply": false, - "fetched_item_count": 25, - "planned_setup_count": 2, - "patched_setup_count": 0, - "skipped_setup_count": 0, - "planned_setups": [ - { - "setup_id": "collect_aws", - "role_name": "ForwardRole", - "org_id": 12345, - "external_id_configured": true, - "proxy_server_id": "proxy-1", - "regions": ["us-east-1", "us-west-2"], - "configured_account_count": 20, - "nqe_account_row_count": 21, - "nqe_candidate_row_count": 1, - "nqe_org_unit_row_count": 0, - "organization_discovery_signal": "visible_candidates", - "planned_payload_account_count": 21, - "added_accounts": [ - {"account_id": "222222222222", "account_name": "new-account"} - ], - "unchanged_account_count": 19, - "patched": false - } - ] -} -``` +Never remove an account only because collection fails. If it remains visible in Organizations, repair its role, trust policy, External ID, or collection permissions. -Manual payload example: - -```json -{ - "collect_aws": [ - { - "accountId": "111111111111", - "accountName": "acct-a", - "roleArn": "arn:aws:iam::111111111111:role/ForwardRole", - "externalId": "Org:12345", - "enabled": true - } - ] -} -``` +## Customer-Defined External ID -`discover-org` manual upload example: - -```json -[ - { - "id": "111111111111", - "name": "acct-a", - "roleArn": "arn:aws:iam::111111111111:role/ForwardRole", - "externalId": "Org:12345", - "errorMsg": null - } -] -``` - - -Check snapshot state directly from the tool: +Changing an External ID is a separate, one-time workflow and works with an existing IAM-user/access-key setup. First review the Forward payload without changing anything: ```bash -./bin/awssync status +./bin/awssync external-id \ + --setup-id AWS-PROD \ + --value customer-defined-value \ + --output aws_external_id_payload.json \ + --format human ``` -Wait for a snapshot to finish processing: +Update the target-role trust policies to require the identical `sts:ExternalId`, test a representative account, then apply the Forward change: ```bash -./bin/awssync wait \ - --snapshot-id SNAPSHOT_ID +./bin/awssync external-id \ + --setup-id AWS-PROD \ + --value customer-defined-value \ + --output aws_external_id_payload.json \ + --apply --yes ``` -Run as a webhook receiver for Forward `SNAPSHOT_READY` events: +Later syncs preserve the value. To roll back, first relax the AWS trust policies, verify role assumption, then run the same command with `--clear` instead of `--value`. -```bash -./bin/awssync serve-webhook \ - --listen :8080 \ - --path /forward/snapshot-ready \ - --webhook-basic-username awssync \ - --webhook-basic-password RECEIVER_SHARED_SECRET \ - --apply \ - --yes -``` +## Onboarding and GovCloud -Webhook mode requires `networkId` and `snapshotId` in the incoming JSON body. The receiver pins the NQE run to that exact snapshot so newly processed snapshots do not race with the webhook event that triggered the sync. +| Environment | Inventory source | Recommended command or workflow | +| --- | --- | --- | +| New commercial AWS Organization | AWS Organizations | Forward Terraform provider; `discover-org` is the CLI/manual fallback | +| Existing setup with complete Organizations data in Forward | Current Forward snapshot/NQE | `preflight`, dry plan, then guarded apply | +| No Organizations access | Reviewed account manifest | `onboard-accounts` or `sync-accounts` | +| GovCloud with complete Organizations data collected by Forward | Current Forward snapshot/NQE | Regular workflow, after preflight confirms evidence | +| Standalone or incomplete GovCloud inventory | Reviewed `aws-us-gov` manifest | `onboard-accounts` or `sync-accounts` | -Create the Forward webhook with API access: +Manifest-based sync treats the reviewed file as authoritative, but still blocks removals unless the operator explicitly allows and bounds them. GovCloud role ARNs retain the `arn:aws-us-gov` partition and mixed-partition plans are rejected. -```bash -./bin/awssync configure-webhook \ - --webhook-url https://awssync.example.com/forward/snapshot-ready \ - --webhook-basic-username awssync \ - --webhook-basic-password RECEIVER_SHARED_SECRET \ - --test-webhook -``` +## Automation -`configure-webhook` creates the webhook when missing and updates the same named webhook when it already exists. To scope webhook-triggered syncs to one or more AWS setups, pass `--setup-id`; the setup IDs are added to the receiver URL and shown in run/preflight output. To create one webhook per setup, also pass `--webhook-per-setup`. +For scheduled jobs, run preflight and the dry plan without removal flags. This allows normal additions while unexpected removals stop for review. Archive the human/JSON summary, payload SHA-256, snapshot ID, selected setup IDs, and applied payload. -```bash -./bin/awssync configure-webhook \ - --webhook-url https://awssync.example.com/forward/snapshot-ready \ - --setup-id AWS \ - --setup-id AWS-SANDBOX \ - --webhook-per-setup -``` +For event-driven operation, `serve-webhook` accepts Forward `SNAPSHOT_READY` events and serializes sync jobs through a bounded queue. Install it behind TLS, configure Basic authentication, and use `configure-webhook` to create or update the Forward webhook. -If Forward is SaaS, the webhook URL must be reachable from Forward SaaS over the internet. A localhost, RFC1918, or private URL will not work unless the receiver is exposed through an approved public endpoint, reverse proxy, or tunnel. For on-prem Forward, the URL only needs to be reachable from the Forward app server. +Do not pass Forward or AWS secrets as command-line arguments in shared process environments. Prefer protected environment injection or a service manager secret facility. Generated files are `0600`, but they may still contain sensitive static-key material and must be retained or deleted according to the customer's credential policy. -### Webhook Service Install +## Documentation -For ongoing webhook use, run `serve-webhook` as a service on a host that can reach Forward and that Forward can reach on the webhook URL. +| Guide | Use it for | +| --- | --- | +| [Quick start](docs/quick-start.md) | Copy/paste commands, setup selection, and troubleshooting | +| [AWS account sync procedure](docs/aws-account-sync-procedure.md) | Complete prerequisites, IAM, automation, and validation runbook | +| [GovCloud workflow](docs/govcloud-workflow.md) | Organizations and standalone-account GovCloud decisions | +| [Architecture and flowcharts](docs/architecture-flow.md) | Full data flow, permissions, credential modes, and security boundaries | +| [Terraform examples](examples/terraform/README.md) | AWS discovery role and collection-role StackSets | -Use an environment file for Forward credentials and receiver settings: +## Development ```bash -FWD_HOST=https://fwd.app -FWD_USER=you@example.com -FWD_PASS=secret -AWSSYNC_WEBHOOK_BASIC_USERNAME=awssync -AWSSYNC_WEBHOOK_BASIC_PASSWORD=receiver-shared-secret +make ci ``` -Linux systemd example: - -```ini -[Unit] -Description=Forward AWS account sync webhook receiver -After=network-online.target -Wants=network-online.target - -[Service] -Type=simple -EnvironmentFile=/etc/awssync/awssync.env -ExecStart=/usr/local/bin/awssync serve-webhook --listen 0.0.0.0:8080 --apply --yes -Restart=on-failure -RestartSec=10 -User=awssync -Group=awssync - -[Install] -WantedBy=multi-user.target -``` - -For temporary SaaS testing, a short-lived tunnel such as `trycloudflare` can expose the receiver. Do not use account-less tunnels for production. - -## Notes - -- The default query is the Forward platform source query for AWS account discovery. -- `--query-id` is an optional override for support/debug workflows. Multi-setup sync requires the query to return `Cloud Setup ID`. -- `--query-setup-param` sends the single selected `--setup-id` into a parameterized saved query. Use it only when the saved query declares that String parameter. -- Forward webhook configuration uses Basic Auth credentials; `serve-webhook` supports the same Basic Auth model. -- Payloads are always written to disk before any PATCH occurs. +`make ci` checks formatting, runs `go vet`, unit tests, the race detector, `govulncheck`, and a reproducible local build. Pull-request and release workflows run with read-only repository permissions except for the release publishing job, which receives only the permissions needed to upload assets and provenance. diff --git a/SECURITY.md b/SECURITY.md new file mode 100644 index 0000000..9ea7876 --- /dev/null +++ b/SECURITY.md @@ -0,0 +1,11 @@ +# Security Policy + +## Supported Versions + +Security fixes are made on the latest released version. Upgrade to the newest release before reporting behavior that may already be fixed. + +## Reporting a Vulnerability + +Do not open a public issue for a suspected vulnerability or include credentials, tenant data, or generated payloads in GitHub. Contact Forward Networks Support through your existing support channel and identify the affected `aws-sync` version from `awssync --version`. + +Include only the minimum reproduction details needed. Forward Networks will coordinate a secure channel for sensitive logs or artifacts. diff --git a/cmd/awssync/main.go b/cmd/awssync/main.go index c1ab326..8547795 100644 --- a/cmd/awssync/main.go +++ b/cmd/awssync/main.go @@ -25,6 +25,12 @@ import ( "golang.org/x/term" ) +var ( + version = "dev" + commit = "unknown" + buildDate = "unknown" +) + func main() { if err := newRootCommand().Execute(); err != nil { emitError(os.Stderr, err) @@ -47,6 +53,7 @@ func newRootCommand() *cobra.Command { cmd := &cobra.Command{ Use: "awssync", Short: "Sync AWS cloud account setup payloads in Forward Networks", + Version: fmt.Sprintf("%s (commit %s, built %s)", version, commit, buildDate), SilenceUsage: true, SilenceErrors: true, RunE: func(cmd *cobra.Command, _ []string) error { diff --git a/cmd/awssync/main_test.go b/cmd/awssync/main_test.go index 48f02d1..e32ff00 100644 --- a/cmd/awssync/main_test.go +++ b/cmd/awssync/main_test.go @@ -5,6 +5,7 @@ import ( "context" "encoding/json" "errors" + "fmt" "io" "net/http" "net/http/httptest" @@ -20,6 +21,14 @@ import ( "github.com/spf13/viper" ) +func TestRootCommandIncludesBuildMetadataInVersion(t *testing.T) { + cmd := newRootCommand() + want := fmt.Sprintf("%s (commit %s, built %s)", version, commit, buildDate) + if cmd.Version != want { + t.Fatalf("unexpected version %q; want %q", cmd.Version, want) + } +} + func TestRootCommandHonorsLocalSnapshotAndOutputFlags(t *testing.T) { var seenNQEQuery string server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { diff --git a/docs/architecture-flow.md b/docs/architecture-flow.md index a8c0a62..71f9969 100644 --- a/docs/architecture-flow.md +++ b/docs/architecture-flow.md @@ -401,6 +401,9 @@ flowchart LR ## Key security properties +- Generated payload and audit artifacts are atomically replaced with mode `0600`; static-key outputs still require secret-grade retention and disposal. +- API retry behavior is operation-aware: reads and full-state PATCHes retry bounded transient failures, while create POSTs remain single-attempt to avoid duplicate side effects. + - Existing setup sync and webhook sync do not connect to AWS; they use Forward NQE data. - `discover-org` connects to AWS Organizations only for initial onboarding. It does not write the discovery credentials to Forward. - `onboard-accounts` and `sync-accounts` do not connect to AWS; they use a locally supplied, explicitly reviewed account manifest. diff --git a/docs/aws-account-sync-procedure.md b/docs/aws-account-sync-procedure.md index 16cb0e7..a17b5a2 100644 --- a/docs/aws-account-sync-procedure.md +++ b/docs/aws-account-sync-procedure.md @@ -120,6 +120,7 @@ Expected result: the query returns AWS account IDs, names, and setup identifiers ```bash make build +./bin/awssync --version ``` The binary is written to: @@ -537,6 +538,10 @@ If a new account appears in the Forward setup but fails collection, the most lik ## Ongoing Automation +Generated payload, manual, and applied-audit files are written atomically with owner-only `0600` permissions. Treat them as secrets when a workflow uses static AWS access keys, and configure retention accordingly. + +The client retries transient `429`, `502`, `503`, and `504` failures only for idempotent reads, NQE reads, and full-state PATCH operations. It does not retry cloud-account or webhook creation POSTs. After an ambiguous create failure, inspect Forward for the requested object before retrying manually. + Run `awssync` on a schedule or after AWS account lifecycle events. The recommended automation policy is to allow routine additions while keeping removals review-gated. Run scheduled automation without `--allow-removals`; a plan containing removals will stop before changing Forward. Treat any nonzero exit as an alert requiring operator review. Retain the JSON plan, its `payload_sha256`, and the `.applied.json` audit copy from successful applies according to the customer's audit policy. After an operator verifies the account lifecycle in AWS and reviews `removed_accounts`, apply the reviewed plan with explicit removal approval and narrow `--max-removals` and `--max-removal-percent` ceilings. diff --git a/docs/quick-start.md b/docs/quick-start.md index 18268f4..e15c34c 100644 --- a/docs/quick-start.md +++ b/docs/quick-start.md @@ -73,6 +73,8 @@ If you need a manual fallback format for UI drag-and-drop, also review `aws_sync Stop if removed accounts are unexpected. +Generated payload, manual, and applied-audit files are atomically replaced with owner-only `0600` permissions. They can still contain sensitive credential material in static-key onboarding workflows, so store and dispose of them according to the customer's credential policy. + ## Add an External ID to an Existing IAM User Setup This is a one-time change separate from the AWS Organizations setup checklist. To add a customer-defined External ID while keeping the existing IAM user/access-key credentials, use the dedicated command. It reads the existing setup directly, so it does not need NQE account discovery or a new snapshot: @@ -219,6 +221,8 @@ Use AWS Organizations visibility together with the member account's `sts:AssumeR ## Webhook Option +Forward API reads, NQE queries, and full-state PATCH operations use bounded retries for transient `429`, `502`, `503`, and `504` responses. Account creation and webhook creation are not retried automatically because a repeated POST could create a duplicate. If a create request returns an ambiguous transport error, inspect current Forward state before running it again. + For event-driven sync, run the receiver: ```bash diff --git a/internal/api/client.go b/internal/api/client.go index 9a95cc8..4da8d1f 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -11,18 +11,27 @@ import ( "net/http" "net/url" "path" + "strconv" "strings" "time" ) const PageLimit = 1000 +const ( + defaultMaxAttempts = 3 + defaultRetryDelay = 200 * time.Millisecond + maxRetryDelay = 5 * time.Second +) + type Client struct { - baseURL *url.URL - apiPrefix string - username string - password string - httpClient *http.Client + baseURL *url.URL + apiPrefix string + username string + password string + httpClient *http.Client + maxAttempts int + retryDelay time.Duration } type NQEResponse struct { @@ -210,6 +219,8 @@ func NewClient(host, apiPrefix, username, password string, insecure bool, timeou Timeout: timeout, Transport: transport, }, + maxAttempts: defaultMaxAttempts, + retryDelay: defaultRetryDelay, }, nil } @@ -255,7 +266,7 @@ func (c *Client) QueryAWSAccounts( if strings.TrimSpace(snapshotID) != "" { endpointPath += fmt.Sprintf("&snapshotId=%s", url.QueryEscape(snapshotID)) } - if err := c.doJSON(ctx, http.MethodPost, endpointPath, payload, &response); err != nil { + if err := c.doJSONRetryable(ctx, http.MethodPost, endpointPath, payload, &response); err != nil { return nil, err } allItems = append(allItems, filterItemsBySetupID(response.Items, setupIDs)...) @@ -389,52 +400,122 @@ func (c *Client) TestNewWebhook(ctx context.Context, webhook Webhook) (*WebhookT } func (c *Client) doJSON(ctx context.Context, method, endpointPath string, requestBody any, out any) error { + retryable := method == http.MethodGet || method == http.MethodPatch + return c.doJSONWithRetry(ctx, method, endpointPath, requestBody, out, retryable) +} + +func (c *Client) doJSONRetryable(ctx context.Context, method, endpointPath string, requestBody any, out any) error { + return c.doJSONWithRetry(ctx, method, endpointPath, requestBody, out, true) +} + +func (c *Client) doJSONWithRetry(ctx context.Context, method, endpointPath string, requestBody any, out any, retryable bool) error { endpoint, err := c.resolve(endpointPath) if err != nil { return err } - var body io.Reader + var encoded []byte if requestBody != nil { - encoded, err := json.Marshal(requestBody) + encoded, err = json.Marshal(requestBody) if err != nil { return fmt.Errorf("encode request body: %w", err) } - body = bytes.NewReader(encoded) } - req, err := http.NewRequestWithContext(ctx, method, endpoint.String(), body) - if err != nil { - return fmt.Errorf("build request: %w", err) + attempts := 1 + if retryable && c.maxAttempts > 1 { + attempts = c.maxAttempts } - if requestBody != nil { - req.Header.Set("Content-Type", "application/json") + for attempt := 1; attempt <= attempts; attempt++ { + var body io.Reader + if requestBody != nil { + body = bytes.NewReader(encoded) + } + req, err := http.NewRequestWithContext(ctx, method, endpoint.String(), body) + if err != nil { + return fmt.Errorf("build request: %w", err) + } + if requestBody != nil { + req.Header.Set("Content-Type", "application/json") + } + req.Header.Set("Accept", "application/json") + req.SetBasicAuth(c.username, c.password) + + resp, err := c.httpClient.Do(req) + if err != nil { + if !retryable || attempt == attempts || ctx.Err() != nil { + return fmt.Errorf("perform request: %w", err) + } + if err := waitForRetry(ctx, retryDelay(c.retryDelay, attempt, "")); err != nil { + return err + } + continue + } + respBody, readErr := io.ReadAll(resp.Body) + closeErr := resp.Body.Close() + if readErr != nil { + return fmt.Errorf("read response body: %w", readErr) + } + if closeErr != nil { + return fmt.Errorf("close response body: %w", closeErr) + } + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + httpErr := &HTTPError{ + Method: method, + Path: endpoint.Path, + StatusCode: resp.StatusCode, + Body: strings.TrimSpace(string(respBody)), + } + if !retryable || attempt == attempts || !isRetryableStatus(resp.StatusCode) { + return httpErr + } + if err := waitForRetry(ctx, retryDelay(c.retryDelay, attempt, resp.Header.Get("Retry-After"))); err != nil { + return err + } + continue + } + if out == nil || len(respBody) == 0 { + return nil + } + if err := json.Unmarshal(respBody, out); err != nil { + return fmt.Errorf("decode response body: %w", err) + } + return nil } - req.Header.Set("Accept", "application/json") - req.SetBasicAuth(c.username, c.password) + return fmt.Errorf("perform request: retry attempts exhausted") +} - resp, err := c.httpClient.Do(req) - if err != nil { - return fmt.Errorf("perform request: %w", err) +func isRetryableStatus(statusCode int) bool { + switch statusCode { + case http.StatusTooManyRequests, http.StatusBadGateway, http.StatusServiceUnavailable, http.StatusGatewayTimeout: + return true + default: + return false } - defer resp.Body.Close() - respBody, err := io.ReadAll(resp.Body) - if err != nil { - return fmt.Errorf("read response body: %w", err) - } - if resp.StatusCode < 200 || resp.StatusCode >= 300 { - return &HTTPError{ - Method: method, - Path: endpoint.Path, - StatusCode: resp.StatusCode, - Body: strings.TrimSpace(string(respBody)), +} + +func retryDelay(base time.Duration, attempt int, retryAfter string) time.Duration { + if seconds, err := strconv.Atoi(strings.TrimSpace(retryAfter)); err == nil && seconds >= 0 { + return min(time.Duration(seconds)*time.Second, maxRetryDelay) + } + if when, err := http.ParseTime(strings.TrimSpace(retryAfter)); err == nil { + if delay := time.Until(when); delay > 0 { + return min(delay, maxRetryDelay) } } - if out == nil || len(respBody) == 0 { - return nil + if base <= 0 { + base = defaultRetryDelay } - if err := json.Unmarshal(respBody, out); err != nil { - return fmt.Errorf("decode response body: %w", err) + return min(base*time.Duration(1<<(attempt-1)), maxRetryDelay) +} + +func waitForRetry(ctx context.Context, delay time.Duration) error { + timer := time.NewTimer(delay) + defer timer.Stop() + select { + case <-ctx.Done(): + return fmt.Errorf("wait to retry request: %w", ctx.Err()) + case <-timer.C: + return nil } - return nil } func (c *Client) resolve(endpointPath string) (*url.URL, error) { diff --git a/internal/api/client_test.go b/internal/api/client_test.go index 0b5e94b..9e1790b 100644 --- a/internal/api/client_test.go +++ b/internal/api/client_test.go @@ -3,6 +3,7 @@ package api import ( "context" "encoding/json" + "errors" "net/http" "net/http/httptest" "testing" @@ -248,3 +249,116 @@ func TestDuplicateWebhookErrorDetection(t *testing.T) { t.Fatalf("expected duplicate webhook error, got %v", err) } } + +func TestIdempotentRequestsRetryTransientFailures(t *testing.T) { + tests := []struct { + name string + call func(context.Context, *Client) error + }{ + { + name: "get", + call: func(ctx context.Context, client *Client) error { + _, err := client.Networks(ctx) + return err + }, + }, + { + name: "patch", + call: func(ctx context.Context, client *Client) error { + return client.PatchCloudAccount(ctx, "network-1", "setup-1", map[string]any{"name": "setup-1"}) + }, + }, + { + name: "nqe read post", + call: func(ctx context.Context, client *Client) error { + _, err := client.QueryAWSAccounts(ctx, "network-1", "snapshot-1", "", "query-1", nil, nil) + return err + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + attempts := 0 + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + attempts++ + if attempts < 3 { + w.Header().Set("Retry-After", "0") + w.WriteHeader(http.StatusServiceUnavailable) + return + } + w.Header().Set("Content-Type", "application/json") + switch r.URL.Path { + case "/api/networks": + _, _ = w.Write([]byte(`[]`)) + case "/api/nqe": + _, _ = w.Write([]byte(`{"items":[]}`)) + default: + w.WriteHeader(http.StatusNoContent) + } + })) + defer server.Close() + + client, err := NewClient(server.URL, "/api", "alice", "secret", true, time.Second) + if err != nil { + t.Fatalf("NewClient() error = %v", err) + } + client.retryDelay = time.Millisecond + if err := tt.call(context.Background(), client); err != nil { + t.Fatalf("request error = %v", err) + } + if attempts != 3 { + t.Fatalf("expected 3 attempts, got %d", attempts) + } + }) + } +} + +func TestCreateCloudAccountDoesNotRetry(t *testing.T) { + attempts := 0 + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + attempts++ + w.WriteHeader(http.StatusServiceUnavailable) + })) + defer server.Close() + + client, err := NewClient(server.URL, "/api", "alice", "secret", true, time.Second) + if err != nil { + t.Fatalf("NewClient() error = %v", err) + } + client.retryDelay = time.Millisecond + err = client.CreateCloudAccount(context.Background(), "network-1", map[string]any{"name": "setup-1"}) + if !IsHTTPStatus(err, http.StatusServiceUnavailable) { + t.Fatalf("expected 503 error, got %v", err) + } + if attempts != 1 { + t.Fatalf("expected one attempt, got %d", attempts) + } +} + +func TestRetryWaitHonorsContextCancellation(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Retry-After", "60") + w.WriteHeader(http.StatusTooManyRequests) + cancel() + })) + defer server.Close() + + client, err := NewClient(server.URL, "/api", "alice", "secret", true, time.Second) + if err != nil { + t.Fatalf("NewClient() error = %v", err) + } + _, err = client.Networks(ctx) + if !errors.Is(err, context.Canceled) { + t.Fatalf("expected context cancellation, got %v", err) + } +} + +func TestRetryDelayIsBounded(t *testing.T) { + if got := retryDelay(time.Second, 10, ""); got != maxRetryDelay { + t.Fatalf("exponential delay = %s; want %s", got, maxRetryDelay) + } + if got := retryDelay(time.Second, 1, "600"); got != maxRetryDelay { + t.Fatalf("Retry-After delay = %s; want %s", got, maxRetryDelay) + } +} diff --git a/internal/app/run.go b/internal/app/run.go index 23e7a4b..41b29cb 100644 --- a/internal/app/run.go +++ b/internal/app/run.go @@ -1544,7 +1544,7 @@ func writeAuditPayloads(path string, payloads auditPayloads) (string, error) { if err != nil { return "", fmt.Errorf("encode audit payloads: %w", err) } - if err := os.WriteFile(path, data, 0o644); err != nil { + if err := writeFileAtomic0600(path, data); err != nil { return "", fmt.Errorf("write audit payloads: %w", err) } return fmt.Sprintf("%x", sha256.Sum256(data)), nil @@ -1566,7 +1566,7 @@ func writeJSONPayload(path string, payload any) (string, string, error) { if err != nil { return "", "", fmt.Errorf("encode output payload: %w", err) } - if err := os.WriteFile(outputPath, data, 0o644); err != nil { + if err := writeFileAtomic0600(outputPath, data); err != nil { return "", "", fmt.Errorf("write output payload: %w", err) } return outputPath, fmt.Sprintf("%x", sha256.Sum256(data)), nil @@ -1588,7 +1588,7 @@ func writeManualAccountData(path string, accounts []ManualAccountData) (string, if err != nil { return "", "", fmt.Errorf("encode manual account data: %w", err) } - if err := os.WriteFile(outputPath, data, 0o644); err != nil { + if err := writeFileAtomic0600(outputPath, data); err != nil { return "", "", fmt.Errorf("write manual account data: %w", err) } return outputPath, fmt.Sprintf("%x", sha256.Sum256(data)), nil @@ -1610,12 +1610,46 @@ func writeManualPayloads(path string, payloads map[string][]api.AssumeRoleInfo) if err != nil { return "", "", fmt.Errorf("encode manual payloads: %w", err) } - if err := os.WriteFile(outputPath, data, 0o644); err != nil { + if err := writeFileAtomic0600(outputPath, data); err != nil { return "", "", fmt.Errorf("write manual payloads: %w", err) } return outputPath, fmt.Sprintf("%x", sha256.Sum256(data)), nil } +func writeFileAtomic0600(path string, data []byte) (err error) { + dir := filepath.Dir(path) + if err := os.MkdirAll(dir, 0o755); err != nil { + return err + } + temp, err := os.CreateTemp(dir, "."+filepath.Base(path)+".tmp-*") + if err != nil { + return err + } + tempPath := temp.Name() + defer func() { + _ = temp.Close() + if err != nil { + _ = os.Remove(tempPath) + } + }() + if err = temp.Chmod(0o600); err != nil { + return err + } + if _, err = temp.Write(data); err != nil { + return err + } + if err = temp.Sync(); err != nil { + return err + } + if err = temp.Close(); err != nil { + return err + } + if err = os.Rename(tempPath, path); err != nil { + return err + } + return nil +} + func auditPath(outputPath string) string { ext := filepath.Ext(outputPath) if ext == "" { diff --git a/internal/app/run_test.go b/internal/app/run_test.go index 0c5fd02..a55ff87 100644 --- a/internal/app/run_test.go +++ b/internal/app/run_test.go @@ -1101,3 +1101,40 @@ func TestRunFallsBackToSingleSetupWhenQueryLacksSetupID(t *testing.T) { t.Fatalf("unexpected plan: %#v", plan) } } + +func TestWriteJSONPayloadIsAtomicAndOwnerOnly(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "payload.json") + if err := os.WriteFile(path, []byte("old"), 0o644); err != nil { + t.Fatalf("seed payload: %v", err) + } + + outputPath, _, err := writeJSONPayload(path, map[string]string{"password": "sensitive"}) + if err != nil { + t.Fatalf("writeJSONPayload() error = %v", err) + } + if outputPath != path { + t.Fatalf("unexpected output path %q", outputPath) + } + info, err := os.Stat(path) + if err != nil { + t.Fatalf("stat payload: %v", err) + } + if got := info.Mode().Perm(); got != 0o600 { + t.Fatalf("expected mode 0600, got %04o", got) + } + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read payload: %v", err) + } + if !strings.Contains(string(data), `"password": "sensitive"`) { + t.Fatalf("unexpected payload contents: %s", data) + } + temps, err := filepath.Glob(filepath.Join(dir, ".payload.json.tmp-*")) + if err != nil { + t.Fatalf("glob temporary files: %v", err) + } + if len(temps) != 0 { + t.Fatalf("temporary files were not cleaned up: %#v", temps) + } +}