From 0138e891efd7c8fc4198fc78d227c08768284d76 Mon Sep 17 00:00:00 2001 From: Alex English Date: Wed, 9 Sep 2026 21:26:07 -0700 Subject: [PATCH] Add -ProjectTag to the AWS coverage script Audits the project tag over the resource types the tag standard names, classifying each as conforming, shared, nonconforming or missing and validating values against the standard's list. Separate from the managed-by sweep rather than a second dimension on it: the two cover different resource types and a gap means different things. A missing managed-by tag is a hygiene problem; a missing or wrong project tag denies access, because the policies conditioning on it fail closed. --- scripts/README.md | 81 ++++++ scripts/aws-terraform-coverage.ps1 | 389 ++++++++++++++++++++++++++++- 2 files changed, 469 insertions(+), 1 deletion(-) diff --git a/scripts/README.md b/scripts/README.md index 39ec71a..3cb4279 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -43,6 +43,7 @@ This is a **tag sweep, not a Terraform state diff**. Read ./aws-terraform-coverage.ps1 -AwsProfile hfla-incubator # pick a named profile ./aws-terraform-coverage.ps1 -CsvPath coverage.csv # full classified list as CSV ./aws-terraform-coverage.ps1 -Region us-west-2 # narrow the sweep +./aws-terraform-coverage.ps1 -ProjectTag # audit the project tag instead ``` A full run makes several hundred read-only API calls and takes a few minutes. @@ -103,6 +104,81 @@ under-reports. something stamped a provenance tag that is neither repo's and is not `exempt`. Worth investigating when it shows up. +### Auditing the `project` tag instead (`-ProjectTag`) + +`-ProjectTag` runs a **different sweep against a different tag**, and answers a +different question: + +| | Default run | `-ProjectTag` | +|---|---|---| +| Tag read | `managed-by` | `project` | +| Scope | ~30 resource types, the whole account | the 8 resource types the tag standard names | +| Question | does anything manage this? | which project owns this? | +| A gap means | a hygiene problem | **an outage** | + +```powershell +./aws-terraform-coverage.ps1 -ProjectTag # summary and every nonconforming value +./aws-terraform-coverage.ps1 -ProjectTag -ListArns # plus every ARN missing the tag +./aws-terraform-coverage.ps1 -ProjectTag -CsvPath project-tags.csv +``` + +The standard is +[DR-Machine-to-machine-IAM-scoping](https://github.com/hackforla/devops/wiki/DR-Machine-to-machine-IAM-scoping) +on the devops wiki. It defines the tag value as the Hack for LA **project** name +— `vrms`, `home-unite-us`, `people-depot`, `civic-tech-jobs`, `civictechindex` — +never an application, environment or repository name, plus `shared` for +infrastructure belonging to no single project. + +**Why this matters more than the `managed-by` report.** The machine roles in +[hackforla/incubator#206](https://github.com/hackforla/incubator/issues/206) are +granted access on the basis of this tag matching, and **tag-based access control +fails closed**. A resource whose tag is missing or wrong is not over-shared; it +becomes unreachable. Run this and read it immediately before any policy change +that conditions on the tag. + +Four outcomes per resource: + +- **`conforming`** — carries a value the standard recognises. +- **`shared`** — carries `shared`, i.e. deliberately belongs to no one project. +- **`nonconforming`** — carries a value that is *not* in the standard. **This is + the dangerous bucket, not `missing`.** Such a resource looks correctly tagged + in the console and still fails to match a policy, so it is always listed in + full rather than summarised. +- **`missing`** — carries no `project` tag at all. + +Two things the report deliberately does *not* treat as equivalent: + +- **On the three machine IAM role families the tag is inventory only.** Policies + name the project literally, interpolated by Terraform, rather than + self-referencing `${aws:PrincipalTag/project}`, so a missing tag on a *role* + degrades reporting and cost allocation but breaks no access. A missing tag on + a *resource* does break access. +- **A gap is not always fixable in a module.** Several untagged resources are + not in Terraform at all, so tagging them means a hand-applied change or + waiting on their import ticket. Check which before proposing where the fix + goes. + +The role families are matched by name, because nothing else distinguishes them. +Note the naming is inverted: `incubator-prod-ecs-task-role` is the shared +**execution** role, while the per-container **task** roles are the +`ecs-container-*` ones. + +#### A trap worth knowing before you re-check a result by hand + +`aws ssm list-tags-for-resource` takes the parameter name as its resource id, +leading slash included, and works fine. But **Git Bash rewrites any argument +beginning with `/` into a Windows path**, so the identical command run there +fails with `InvalidResourceId` — and a sweep that discards stderr reads that as +an untagged parameter. + +That is exactly what produced the "0 of 37 SSM parameters carry a `project` tag" +figure in +[hackforla/incubator#197](https://github.com/hackforla/incubator/issues/197) and +in the decision record. It was wrong: every parameter was tagged the whole time. +This script is unaffected because it is PowerShell, but anyone spot-checking one +of its results from Git Bash will hit it. Drop the leading slash there, or use +PowerShell. + ### Why it does not just use the Resource Groups Tagging API The obvious implementation is one `resourcegroupstaggingapi get-resources` call @@ -199,3 +275,8 @@ created first. Open a follow-on issue if the report proves worth automating. added `managed-by = terraform-incubator`, for exactly this reporting purpose. - [hackforla/devops-security#172](https://github.com/hackforla/devops-security/issues/172) — the devops-security half of the same tagging work. +- [hackforla/incubator#197](https://github.com/hackforla/incubator/issues/197) — + added `-ProjectTag`, and the audit it exists to make repeatable. +- [hackforla/incubator#206](https://github.com/hackforla/incubator/issues/206) — + the epic whose policies condition on the `project` tag, and the reason a gap in + that tag is an outage rather than a reporting defect. diff --git a/scripts/aws-terraform-coverage.ps1 b/scripts/aws-terraform-coverage.ps1 index 2b3a6bb..b8b1017 100644 --- a/scripts/aws-terraform-coverage.ps1 +++ b/scripts/aws-terraform-coverage.ps1 @@ -44,8 +44,16 @@ Account this script expects to be pointed at. A mismatch warns but does not stop, so the script can be reused against another account deliberately. +.PARAMETER ProjectTag + Audit the `project` tag instead of `managed-by`, over the resource types the + tag standard names rather than the whole account. Reports each resource as + conforming, shared, nonconforming or missing. Run this before any policy + change that conditions on the tag: those policies fail closed, so a missing + or wrong tag is an outage rather than an over-grant. + .PARAMETER ListArns List every unmanaged and unmanageable ARN, not just the per-service counts. + Under -ProjectTag, lists every ARN missing the tag. .PARAMETER CsvPath Also write the full classified resource list to this path as CSV. @@ -58,6 +66,9 @@ .EXAMPLE ./aws-terraform-coverage.ps1 -CsvPath coverage.csv + +.EXAMPLE + ./aws-terraform-coverage.ps1 -ProjectTag -ListArns #> [CmdletBinding()] param( @@ -65,7 +76,8 @@ param( [string]$AwsProfile, [string]$ExpectedAccountId = '035866691871', [switch]$ListArns, - [string]$CsvPath + [string]$CsvPath, + [switch]$ProjectTag ) $ErrorActionPreference = 'Stop' @@ -76,6 +88,24 @@ $script:DevOpsSecurityValue = 'terraform-devops-security' $script:ExemptValue = 'exempt' $script:ReadOnlyVerbPattern = '^(describe|list|get)-' +# -ProjectTag only. The recognised values come from the tag standard, and a +# value outside this list is a defect rather than a new project: adding one here +# without amending the standard defeats the point of checking against it. +$script:ProjectTagKey = 'project' +$script:SharedProjectValue = 'shared' +$script:KnownProjects = @('vrms', 'home-unite-us', 'people-depot', + 'civic-tech-jobs', 'civictechindex') + +# The three machine role families the standard names. Matched on name because +# nothing else distinguishes them, and the shared execution role's name is +# actively misleading -- incubator-prod-ecs-task-role is the execution role, +# while the per-container task roles are the ecs-container-* ones. +$script:MachineRolePatterns = [ordered]@{ + 'incubator-cicd-*' = 'iam-role-cicd' + 'ecs-container-*' = 'iam-role-ecs-task' + 'incubator-prod-ecs-task-role' = 'iam-role-ecs-execution' +} + # --------------------------------------------------------------------------- # AWS CLI plumbing # --------------------------------------------------------------------------- @@ -1131,6 +1161,358 @@ function Write-CoverageReport { Write-Host '' } +# --------------------------------------------------------------------------- +# Project tag audit (-ProjectTag) +# +# A separate sweep from the managed-by report above, over exactly the resource +# types the tag standard names -- see DR-Machine-to-machine-IAM-scoping on the +# devops wiki. It is deliberately not a second dimension bolted onto every +# collector: the standard lists eight resource types, the managed-by sweep +# covers roughly thirty, and reporting a project tag for a KMS key or a route +# table would invent gaps the standard never asked for. +# +# The two sweeps also answer different questions. managed-by asks "does anything +# manage this", where absence is a hygiene problem. project asks "which project +# owns this", where absence breaks access, because the policies that condition +# on this tag fail closed. +# --------------------------------------------------------------------------- + +$script:ProjectResources = New-Object System.Collections.ArrayList + +function Get-ProjectTagFromCall { + <# As Get-ManagedByFromCall, but reads the project tag rather than managed-by. #> + param( + [Parameter(Mandatory)][string[]]$Arguments, + [string]$TagsPath = 'Tags', + [switch]$AsMap, + [switch]$Tolerant + ) + + if ($Tolerant) { $response = Invoke-AwsCli -Arguments $Arguments -Tolerant } + else { $response = Invoke-AwsCli -Arguments $Arguments } + if ($null -eq $response) { return $null } + + $tags = Select-Nested -InputObject $response -Path $TagsPath + if ($AsMap) { return Get-TagValueFromMap -Map $tags -Name $script:ProjectTagKey } + return Get-TagValueFromPairs -Pairs $tags -Name $script:ProjectTagKey +} + +function Add-ProjectResource { + <# + Records one resource against the tag standard. Four outcomes, and the + interesting one is nonconforming: a resource carrying a project value + that is not in the standard's list is worse than an untagged one, + because it looks correct in a console and still denies access. + #> + param( + [Parameter(Mandatory)][string]$Arn, + [string]$Project, + [Parameter(Mandatory)][string]$Kind, + [string]$ResourceRegion = 'global', + [string]$Note + ) + + if ([string]::IsNullOrWhiteSpace($Arn)) { return } + + if ([string]::IsNullOrWhiteSpace($Project)) { $status = 'missing' } + elseif ($Project -eq $script:SharedProjectValue) { $status = 'shared' } + elseif ($script:KnownProjects -contains $Project) { $status = 'conforming' } + else { $status = 'nonconforming' } + + $null = $script:ProjectResources.Add([pscustomobject]@{ + Arn = $Arn + Kind = $Kind + Region = $ResourceRegion + Project = $Project + Status = $status + Note = $Note + }) +} + +function Add-ProjectEcrResources { + param([string]$SweepRegion) + $count = 0 + foreach ($repository in @((Invoke-AwsCli -Arguments @('ecr', 'describe-repositories', '--region', $SweepRegion)).repositories)) { + if ($null -eq $repository) { continue } + $project = Get-ProjectTagFromCall -TagsPath 'tags' -Arguments @( + 'ecr', 'list-tags-for-resource', '--resource-arn', $repository.repositoryArn, '--region', $SweepRegion) + Add-ProjectResource -Arn $repository.repositoryArn -Project $project ` + -Kind 'ecr-repository' -ResourceRegion $SweepRegion + $count++ + } + return $count +} + +function Add-ProjectEcsResources { + param([string]$SweepRegion) + $count = 0 + + foreach ($clusterArn in @((Invoke-AwsCli -Arguments @('ecs', 'list-clusters', '--region', $SweepRegion)).clusterArns)) { + if ([string]::IsNullOrWhiteSpace($clusterArn)) { continue } + $serviceArns = @((Invoke-AwsCli -Arguments @('ecs', 'list-services', '--cluster', $clusterArn, + '--region', $SweepRegion)).serviceArns) + # describe-services accepts at most 10 services per call. + foreach ($chunk in (Split-IntoChunks -Items $serviceArns -Size 10)) { + $described = (Invoke-AwsCli -Arguments (@('ecs', 'describe-services', '--cluster', $clusterArn, + '--include', 'TAGS', '--region', $SweepRegion, '--services') + $chunk)).services + foreach ($service in @($described)) { + if ($null -eq $service) { continue } + Add-ProjectResource -Arn $service.serviceArn ` + -Project (Get-TagValueFromPairs -Pairs $service.tags -Name $script:ProjectTagKey) ` + -Kind 'ecs-service' -ResourceRegion $SweepRegion + $count++ + } + } + } + + # Current revision per family only, matching the managed-by sweep: older + # revisions are immutable deploy artifacts and cannot be retagged usefully. + foreach ($family in @((Invoke-AwsCli -Arguments @('ecs', 'list-task-definition-families', + '--status', 'ACTIVE', '--region', $SweepRegion)).families)) { + if ([string]::IsNullOrWhiteSpace($family)) { continue } + $described = Invoke-AwsCli -Tolerant -Arguments @('ecs', 'describe-task-definition', + '--task-definition', $family, + '--include', 'TAGS', '--region', $SweepRegion) + if ($null -eq $described) { continue } + Add-ProjectResource -Arn $described.taskDefinition.taskDefinitionArn ` + -Project (Get-TagValueFromPairs -Pairs $described.tags -Name $script:ProjectTagKey) ` + -Kind 'ecs-task-definition' -ResourceRegion $SweepRegion + $count++ + } + + return $count +} + +function Add-ProjectSsmResources { + param([string]$SweepRegion) + $count = 0 + foreach ($parameter in @((Invoke-AwsCli -Arguments @('ssm', 'describe-parameters', '--region', $SweepRegion)).Parameters)) { + if ($null -eq $parameter) { continue } + # list-tags-for-resource takes the parameter name as the resource id and + # accepts the leading slash. Note for anyone re-checking a result by hand + # from Git Bash rather than from PowerShell: that shell rewrites any + # argument beginning with "/" into a Windows path, so the same call there + # fails with InvalidResourceId and reads as an untagged parameter. That is + # what produced the "0 of 37 parameters are tagged" figure in + # hackforla/incubator#197, which was wrong -- all of them are tagged. + $project = Get-ProjectTagFromCall -TagsPath 'TagList' -Tolerant -Arguments @( + 'ssm', 'list-tags-for-resource', '--resource-type', 'Parameter', + '--resource-id', $parameter.Name, '--region', $SweepRegion) + $arn = 'arn:aws:ssm:{0}:{1}:parameter{2}' -f $SweepRegion, $script:AccountId, ($parameter.Name -replace '^/?', '/') + Add-ProjectResource -Arn $arn -Project $project -Kind 'ssm-parameter' -ResourceRegion $SweepRegion + $count++ + } + return $count +} + +function Add-ProjectLogGroupResources { + param([string]$SweepRegion) + $count = 0 + foreach ($logGroup in @((Invoke-AwsCli -Arguments @('logs', 'describe-log-groups', '--region', $SweepRegion)).logGroups)) { + if ($null -eq $logGroup) { continue } + # describe-log-groups returns a trailing ":*" that list-tags-for-resource rejects. + $arn = $logGroup.arn -replace ':\*$', '' + $project = Get-ProjectTagFromCall -AsMap -TagsPath 'tags' -Tolerant -Arguments @( + 'logs', 'list-tags-for-resource', '--resource-arn', $arn, '--region', $SweepRegion) + $note = $null + if ($logGroup.logGroupName -like '/aws/lambda/*') { + $note = 'Created by Lambda on first invocation, outside Terraform' + } + Add-ProjectResource -Arn $arn -Project $project -Kind 'log-group' ` + -ResourceRegion $SweepRegion -Note $note + $count++ + } + return $count +} + +function Add-ProjectCognitoResources { + param([string]$SweepRegion) + $count = 0 + $pools = (Invoke-AwsCli -Arguments @('cognito-idp', 'list-user-pools', '--max-results', '60', '--region', $SweepRegion)).UserPools + foreach ($pool in @($pools)) { + if ($null -eq $pool) { continue } + $arn = 'arn:aws:cognito-idp:{0}:{1}:userpool/{2}' -f $SweepRegion, $script:AccountId, $pool.Id + # Cognito returns tags as a dictionary, not as Key/Value pairs. + $project = Get-ProjectTagFromCall -AsMap -Arguments @( + 'cognito-idp', 'list-tags-for-resource', '--resource-arn', $arn, '--region', $SweepRegion) + Add-ProjectResource -Arn $arn -Project $project -Kind 'cognito-user-pool' ` + -ResourceRegion $SweepRegion -Note $pool.Name + $count++ + } + return $count +} + +function Add-ProjectS3Resources { + Write-Step 'S3 buckets' + $count = 0 + foreach ($bucket in @((Invoke-AwsCli -Arguments @('s3api', 'list-buckets')).Buckets)) { + if ($null -eq $bucket) { continue } + # A bucket with no tag set at all makes this call fail, which means the + # same thing here as an empty tag set. + $project = Get-ProjectTagFromCall -Tolerant -TagsPath 'TagSet' ` + -Arguments @('s3api', 'get-bucket-tagging', '--bucket', $bucket.Name) + Add-ProjectResource -Arn "arn:aws:s3:::$($bucket.Name)" -Project $project -Kind 's3-bucket' + $count++ + } + Write-Host $count + return $count +} + +function Add-ProjectIamRoleResources { + <# + Only the three machine role families the standard names, not every role + in the account. Per the standard this tag is inventory rather than + authorization for these, so a gap here is a hygiene defect and not an + outage -- the report says so rather than leaving the reader to guess. + #> + Write-Step 'machine IAM roles' + $count = 0 + foreach ($role in @((Invoke-AwsCli -Arguments @('iam', 'list-roles')).Roles)) { + if ($null -eq $role) { continue } + if ($role.Path -like '/aws-service-role/*' -or $role.Path -like '/aws-reserved/*') { continue } + + $kind = $null + foreach ($pattern in $script:MachineRolePatterns.Keys) { + if ($role.RoleName -like $pattern) { $kind = $script:MachineRolePatterns[$pattern]; break } + } + if (-not $kind) { continue } + + $project = Get-ProjectTagFromCall -Arguments @('iam', 'list-role-tags', '--role-name', $role.RoleName) + Add-ProjectResource -Arn $role.Arn -Project $project -Kind $kind -Note 'inventory only, not authorization' + $count++ + } + Write-Host $count + return $count +} + +function Write-ProjectTagReport { + $all = @($script:ProjectResources) + + $conforming = @($all | Where-Object { $_.Status -eq 'conforming' }) + $shared = @($all | Where-Object { $_.Status -eq 'shared' }) + $nonconforming = @($all | Where-Object { $_.Status -eq 'nonconforming' }) + $missing = @($all | Where-Object { $_.Status -eq 'missing' }) + + Write-Section 'Project tag summary' + $classified = $conforming.Count + $shared.Count + $percent = 0 + if ($all.Count -gt 0) { $percent = [math]::Round(100 * $classified / $all.Count, 1) } + + @( + [pscustomobject]@{ Status = 'conforming'; Resources = $conforming.Count } + [pscustomobject]@{ Status = 'shared'; Resources = $shared.Count } + [pscustomobject]@{ Status = 'nonconforming'; Resources = $nonconforming.Count } + [pscustomobject]@{ Status = 'missing'; Resources = $missing.Count } + ) | Format-Table -AutoSize | Out-String | Write-Host + + Write-Host (" {0} of {1} resources carry a project value the standard recognises ({2}%)." -f $classified, $all.Count, $percent) + Write-Host (" Recognised values: {0}, plus '{1}' for resources belonging to no single project." -f + ($script:KnownProjects -join ', '), $script:SharedProjectValue) + + Write-Section 'By resource type' + $all | Group-Object Kind | Sort-Object Name | ForEach-Object { + $rows = @($_.Group) + [pscustomobject]@{ + Kind = $_.Name + Total = $rows.Count + Conforming = @($rows | Where-Object { $_.Status -eq 'conforming' }).Count + Shared = @($rows | Where-Object { $_.Status -eq 'shared' }).Count + Nonconforming = @($rows | Where-Object { $_.Status -eq 'nonconforming' }).Count + Missing = @($rows | Where-Object { $_.Status -eq 'missing' }).Count + } + } | Format-Table -AutoSize | Out-String | Write-Host + + # Always listed in full, however long. A wrong value denies access to a + # resource that looks tagged, so this is the section that predicts an outage. + Write-Section 'Nonconforming - project value is not in the standard' + if ($nonconforming.Count -eq 0) { Write-Host ' none' } + else { + $nonconforming | Group-Object Project | Sort-Object Count -Descending | + Select-Object @{ n = 'Value'; e = { $_.Name } }, Count | + Format-Table -AutoSize | Out-String | Write-Host + $nonconforming | Sort-Object Kind, Arn | ForEach-Object { + Write-Host (' {0} [{1}]' -f $_.Arn, $_.Project) + } + } + + Write-Section 'Missing by resource type' + if ($missing.Count -eq 0) { Write-Host ' none' } + else { + $missing | Group-Object Kind | Sort-Object Count -Descending | + Select-Object @{ n = 'Kind'; e = { $_.Name } }, Count | + Format-Table -AutoSize | Out-String | Write-Host + if (-not $ListArns) { Write-Host ' Re-run with -ListArns for the individual ARNs.' } + } + + if ($ListArns -and $missing.Count -gt 0) { + Write-Section 'Missing ARNs' + $missing | Sort-Object Kind, Arn | ForEach-Object { + if ($_.Note) { Write-Host (' {0} ({1})' -f $_.Arn, $_.Note) } + else { Write-Host " $($_.Arn)" } + } + } + + Write-Section 'Reading this report' + $notes = @( + @('A missing or wrong project tag denies access rather than over-granting,', + 'because the policies that condition on it fail closed. Run this and read', + 'it immediately before any policy change that conditions on the tag.'), + @('nonconforming is the dangerous bucket, not missing. A resource tagged', + 'with an unrecognised value looks correct in the console and still fails', + 'to match the policy.'), + @('On the three machine IAM role families the tag is inventory and cost', + 'allocation only -- policies name the project literally rather than', + 'self-referencing the principal tag -- so a gap there is a hygiene', + 'defect and not an outage.'), + @('Resources outside Terraform can only be tagged by hand, so a gap here', + 'is not always fixable in a module. Check whether the resource is', + 'managed before proposing where the fix goes.'), + @('This sweep covers the resource types the standard names and nothing', + 'else. It is not an inventory of the account -- run without -ProjectTag', + 'for that.') + ) + foreach ($note in $notes) { + Write-Host " - $($note[0])" + for ($index = 1; $index -lt $note.Count; $index++) { Write-Host " $($note[$index])" } + } + Write-Host '' +} + +function Invoke-ProjectTagAudit { + Write-Host '' + Write-Host 'Global resources' + $null = Add-ProjectS3Resources + $null = Add-ProjectIamRoleResources + + foreach ($sweepRegion in $Region) { + Write-Host '' + Write-Host "Regional resources ($sweepRegion)" + + $collectors = @( + @{ Label = 'ECR repositories'; Function = 'Add-ProjectEcrResources' } + @{ Label = 'ECS services and task defs'; Function = 'Add-ProjectEcsResources' } + @{ Label = 'SSM parameters'; Function = 'Add-ProjectSsmResources' } + @{ Label = 'CloudWatch log groups'; Function = 'Add-ProjectLogGroupResources' } + @{ Label = 'Cognito user pools'; Function = 'Add-ProjectCognitoResources' } + ) + + foreach ($collector in $collectors) { + Write-Step $collector.Label + $count = & $collector.Function -SweepRegion $sweepRegion + Write-Host $count + } + } + + Write-ProjectTagReport + + if ($CsvPath) { + $script:ProjectResources | Sort-Object Status, Kind, Arn | + Export-Csv -Path $CsvPath -NoTypeInformation -Encoding utf8 + Write-Host "Full classified list written to $CsvPath" + Write-Host '' + } +} + # --------------------------------------------------------------------------- # Main # --------------------------------------------------------------------------- @@ -1152,6 +1534,11 @@ if ($ExpectedAccountId -and $script:AccountId -ne $ExpectedAccountId) { 'Continuing anyway.') -f $ExpectedAccountId, $script:AccountId) } +if ($ProjectTag) { + Invoke-ProjectTagAudit + return +} + Write-Host '' Write-Host 'Global resources' $null = Add-IamResources