Repository navigation
Report unredacted AWS account IDs and EC2 instance IDs in PRs - #119
Merged
Merged
Conversation
Reads the lines a pull request adds and reports unredacted AWS account IDs (twelve digits, no digit either side) and EC2 instance IDs (`i-` plus eight or seventeen hexadecimal characters, case-insensitive). Read-only: it runs `git diff` and nothing else, and its only write is appending to $GITHUB_STEP_SUMMARY. Added lines only. `main` already carries real-looking identifiers in committed eval results and example ARNs, so a whole-file scan would fail pull requests for content their authors never wrote. The diff is read with --find-renames, without which a pure rename reports every line as added and would flag identifiers a pull request only moved. A matched identifier has an optional trailing `_<digits>` stripped before the allowlist comparison, which is the skill evaluation tool's redaction suffix. A \b-anchored pattern would not match the suffixed form at all, since `_` is a word character, so the suffix is consumed by the pattern and normalized away. Four suppressors: the allowlist file's own added lines are skipped; an entry in .github/aws-identifier-allowlist.json, which must carry a reason; a twelve-digit run that is a bare JSON value rather than a quoted string, in a .json/.jsonl path, since AWS always writes an account ID as a string; and an `aws-id-ok: <reason>` comment on the line, in the file types that have a comment syntax. The marker is checked after the patterns, so a line that merely names it and holds no identifier is left alone. The allowlist parser fails closed: bad JSON, a missing key, or a blank reason grants nothing and exits 2. Exit 0 clean (a pull request adding no lines included), 1 on findings, 2 on a broken allowlist or an unreadable diff. `--self-check` asserts every allowlist entry is suppressed bare and `_N`-suffixed. Verified against real history (origin/main~1...origin/main reports the four real-looking account IDs the last merge added to eval fixtures and nothing else) and against 15 diff fixtures held in /tmp: known-bad account and instance IDs fail; all 12 seed values pass bare and suffixed; `322122547200` as a bare JSON value passes while the same digits quoted fail; a marked line passes; removed lines, context lines, the `+++` header and a pure rename produce nothing; 60 findings print 50 and a remainder count; four malformed allowlists each exit 2.
Runs the scanner on every pull request and reports a status. Strictly
read-only: it checks out into the runner's throwaway workspace, runs `git fetch`
and the scanner, and writes only into the artifact directory, $GITHUB_OUTPUT and
$GITHUB_STEP_SUMMARY. No `git` write command, no pull request mutation,
`permissions: contents: read` and nothing more.
The job id and its display name are the identical string `scan-aws-identifiers`,
because the display name is the status check context branch protection matches
on and contexts are global across the repository.
Three deliberate omissions, each commented in the file. No `paths:` filter: a
required check that never runs reports pending forever and would permanently
block pull requests touching nothing it names, so the job always runs and the
scanner exits 0 when nothing was added. No `if:` guard on the job: a skipped job
reports "skipped", which branch protection can treat as satisfied, so a guard
keyed on anything a contributor controls would be a merge bypass. And
`continue-on-error: true` comes before the artifact upload, with the job failed
by a later step, because otherwise the artifact never uploads in the failing
case — exactly the case that needs a label.
`labeled` is in the trigger list so a maintainer can force a run on an
already-open pull request; GitHub cannot filter `labeled` by label name. A label
applied by GITHUB_TOKEN raises no `labeled` event, so the companion labeler
cannot re-trigger this check.
Every event-payload value goes through `env:` and is quoted on use; no `${{ }}`
appears in a `run:` line. Verified by parsing the file with PyYAML: single job
key equal to its `name:`, permissions read-only, no `paths:`, no `if:`.
A red check does not block merging until an admin adds the context
`scan-aws-identifiers` to the required status checks for `main`.
Applies `needs-id-redact` when "Scan for AWS identifiers" fails and removes it
once the check passes, so a pull request is never left carrying a stale label.
A separate workflow_run workflow because GITHUB_TOKEN is read-only for
pull_request events from forks whatever the `permissions:` block says, and this
repository is overwhelmingly fork-driven — labeling from the check job itself
would silently do nothing for most pull requests. The pull request number comes
from the check's artifact, since `workflow_run.pull_requests` is empty for fork
pull requests, which is what the `actions: read` permission is for.
The repository's default workflow permission is read-only, which is the
recommended baseline and must stay that way, so `pull-requests: write` is raised
on this one job. It writes labels and nothing else: no checkout, no branch, no
commit.
A label written by GITHUB_TOKEN raises no `labeled` event, so this workflow
cannot re-trigger the check it reports on.
Removal is a checked lookup rather than `--remove-label || true`. Suppressing
the absent-label error also suppresses every other error, including a
missing-GH_REPO failure, and the step then reports success while doing nothing —
the bug label-skill-evals-result.yml was fixed for. This file starts from the
fix, so an absent label stays benign while a real API failure fails the job.
This workflow cannot be smoke-tested on the pull request that introduces it:
workflow_run only fires for a file already on the default branch. Verifying it
is a post-merge step, noted in the file's header.
Verified by parsing with PyYAML: `workflows:` is byte-equal to the gate's
`name:`, the artifact name and both filenames match on both sides, and the job
permissions are exactly {contents: read, actions: read, pull-requests: write}.
CONTRIBUTING.md gets a top-level "Scanning for AWS Identifiers" section, placed after Custom Agents and before Reporting Bugs so it sits alongside the skill-evals gate material without being nested under Skills — the check applies to every file in the repository, not only to skills. It covers the two patterns, the added-lines-only scope and why (`main` already carries real-looking identifiers, and a whole-file scan would fail contributors for content they did not write), the three suppression routes with an example of each, the bare-JSON-number exclusion, how to run the scanner and `--self-check` locally, what exit 1 and exit 2 mean, and the `needs-id-redact` label being applied and removed by automation alone. Every identifier in the examples is an allowlisted value, except the `322122547200` byte count, which carries an `aws-id-ok: <reason>` marker so the section passes the check it describes. The same material goes into .kiro/steering/project-conventions.md as a short "AWS Identifiers" section after Disallowed Content, grouped with the other repository-wide content constraints. That file already covers the skill-evals gate at comparable depth, which is the condition for touching it, so this stays to six bullets. .claude/CLAUDE.md is a tracked near-duplicate of it and gets the same section with the shallower relative link, so the pair does not drift. The documentation also states that pre-existing identifiers on `main` are a separate follow-up change and not to be cleaned up opportunistically, since the check never reports them. Verified: the staged tree self-scans clean — 1113 added lines, 0 findings, 0 warnings.
The scan step left `git fetch` to errexit, so an unreachable remote killed the step before it wrote its exit code. The empty value read downstream became OUTCOME=fail, and the labeling workflow then applied needs-id-redact to a pull request that contained no identifiers at all. Exit code 2, which the scanner uses for a malformed allowlist or a git failure, collapsed the same way, discarding the distinction the scanner takes care to maintain. The step now captures the fetch exit code instead of dying on it, and the published outcome carries three values rather than two: pass, fail, error. The labeling workflow leaves the label untouched on error, because needs-id-redact is a claim about a pull request's content and a broken scan is in no position to make it. The job still fails on every non-zero exit, so a scan that could not run can never be read as a clean one. The scanner's --self-check runs in the gate as well. It covers the `_N` redaction-suffix handling, which is the one part whose breakage is silent: a regression there makes identifiers stop being detected rather than start firing falsely, so a passing check would be the only symptom.
A UUID ends in twelve hexadecimal characters, so about one in 250 ends in twelve digits. The account-ID pattern required only that no digit sit on either side, and a hyphen is not a digit, so the final group of a UUID was reported as an account. Eleven UUIDs in committed eval output match that shape today. Measuring the fix over the whole tree turned up a second class the same check had been reporting: a twelve-digit run inside a longer hexadecimal token. A network interface id is built from a hex blob, and the twelve digits sitting in the middle of one name no account at all -- yet 24 lines of eval output were reported that way. Widening the boundary from "no digit" to "no letter or digit" covers it, along with every other resource id built the same way: subnet, vol, sg, snap, ami. Both guards only remove matches, verified by replaying the old and new patterns over every text file in the repository: 8694 matches before, 8645 after, and not one match that the new pattern finds and the old one missed. The 49 it drops are all UUID tails or hex resource ids. The UUID guard spells out the full 8-4-4-4 hex prefix rather than rejecting a preceding hyphen, so an account ID that merely follows a hyphen is still reported. Every element of it is a fixed repetition, which is what lets Python's re accept it as a lookbehind. Eleven boundary cases now run in --self-check, which the gate workflow executes on every pull request. They are checked against the pattern directly rather than through the allowlist, because a guard that grows too broad fails silently: identifiers stop being reported and a passing check is the only symptom.
The twelve-digit shape rule was wrong more often than right. Of 1867 twelve-digit runs on the open pull requests' added lines, 1008 (54%) name no account, in four shapes: a YYYYMMDDHHMM datestamp used as a resource-name suffix (916 hits), the fractional digits of a decimal (57), the integer part of a decimal (7), and a zero-padded counter (28). The bare-JSON-number heuristic could not catch the decimal ones, because the eval journals store serialized AWS API responses as strings inside the outer JSON, so those digits really are inside a JSON string. The scan now runs in two passes. Pass one gathers evidence and reports nothing, from three sources: the account field of an ARN (the fifth colon-separated field, per the AWS reference); a twelve-digit object key in the tool result of a tool_summary block, in a journal_records.json file; and the value of an aws_account_id field, in a journal_records.json file. The two journal sources are fields of the DevOps Agent journal schema, read by walking the record tree and parsing the JSON-inside-JSON as it goes, since the escaping depth varies from one record to the next. Pass one reads every changed file whole, so an ARN in a part of a file the pull request never touched still proves an account that an added line names bare. Pass two reports, on added lines only: an ARN whose account field holds a non-allowlisted twelve-digit value, reported as the whole ARN so the remedy is to remove the ARN rather than blank one field and leave the rest of it in place; every occurrence of an account pass one proved, in any file type; and EC2 instance IDs, unchanged. An ARN with no account field is not reported. Of 121 such ARNs on the open pull requests' added lines, none names a private resource — 97 are templates or explicit examples, 21 are placeholders or AWS-published service quota codes, 3 are truncated — and there is no way to tell a real bucket ARN from an example one, since anybody may own a bucket by any name. Removed with the shape rule: the UUID-tail guard, the alphanumeric boundary as a load-bearing guard, and the unquoted-JSON-number exclusion. The two boundary lookarounds stay on the occurrence search as cheap insurance and are commented as such. Verified: - --self-check exits 0: 13 evidence cases (the three ARN resource forms, a non-aws partition, an account-less ARN, the literal aws in the account field, the redaction suffix, both journal sources including the nested and escaped tool_summary shape, and a journal that will not parse), 16 reporting cases (the whole-ARN finding, prose occurrences, and every lookalike shape staying silent), and all 12 allowlist entries suppressed bare and _N-suffixed. - The branch passes its own scan against origin/main with no findings. - All 33 open pull requests were scanned. Five report findings and 28 are clean. Of the five, one is instance IDs alone; three carry ARNs naming real accounts; one is caught solely by a tool_summary object key in an eval journal, which the previous rule also caught but only by shape. None of the four removed shapes is reported anywhere. - One pull request stays clean that the old rule would have failed: its account appears only as prose in a benchmark file with no ARN, which is the documented gap this design accepts. - Whole main tree, every line of all 860 tracked files: 9016 findings against the old rule's 9102. Account findings fall from 8218 across 18 distinct values to 8132 across 3, each of the 3 proved by an ARN. Of the 15 values no longer reported, 4 are not accounts at all (two are divisors in a SQL snippet) and the rest are named only in prose or in a field this scan does not read. Instance findings are identical, 884 across 4 distinct ids; running the previous scanner over the same added lines of the two pull requests that carry instance IDs produced a byte-identical list. - Four local eval directories outside the repository were scanned, with gitignored paths skipped. The account each one carries in CloudFormation stack ARNs in _metadata.json is still found, now through the ARN rule. One account named only in agent prose and in an aws-iam-authenticator string is missed, which is the same accepted gap. - The scanner still writes nothing. Its subprocess calls are git diff and git show, both read-only, and its only file write appends to $GITHUB_STEP_SUMMARY. Neither workflow YAML nor the allowlist changed; this is a scanner-internal change. CONTRIBUTING.md, the Kiro steering file and .claude/CLAUDE.md are rewritten to describe the ARN-and-proven-value model, the documentation convention of writing an example ARN with an allowlisted placeholder account, and the known prose gap.
Review of the evidence-based account ID detection found two blocking gaps and
two inaccurate comments.
ARN_RE pinned the region to [a-z0-9-]* and the account to [0-9A-Za-z_-]*, so an
ARN with * in either field failed to parse at all and its twelve-digit account
field was never read — neither as evidence nor as a finding. That shape is how
an IAM policy scopes a resource across regions, and this repository ships seven
policy JSON files. A policy file naming a real account proved nothing and was
reported as nothing. Every field before the resource now accepts *, so such an
ARN parses and is judged on its account field: twelve digits are proved and
reported whole, and a wildcard or a template expression stays silent because it
names no account, not because the parse failed.
.agents/ is now gitignored. The directory this change created holds working
notes that quote real account IDs as prose, in files that are not journals and
carry no ARN — the one gap this scan documents as uncatchable, so a commit of
them would pass clean.
Two comments are reworded: JOURNAL_ACCOUNT_FIELD_RE runs on every journal
alongside the tree walk rather than only when the walk fails, and the extraction
case claiming ARNs are read in any file type now carries an ARN in its fixture.
CONTRIBUTING.md's ARN source row names the wildcard case, for the contributor
writing a policy reference file.
Verification, all re-run after the fix:
- --self-check exits 0: 15 evidence cases, 18 reporting cases (two of each are
new and cover the wildcard fields), 12 allowlist entries bare and _N-suffixed,
0 failures.
- The branch passes its own scan against origin/main, 0 findings, exit 0; the
working tree was scanned the same way through --diff-file before committing.
- All 33 open pull requests: the same five fail and 28 pass as before the fix,
finding for finding, so the wider parse costs no new findings on them. None of
the four removed false-positive classes is reported anywhere.
- The whole main tree: 9016 findings, unchanged, across the same three
ARN-proven accounts and four instance IDs. Every wildcard-region ARN on main
holds * or ${AWS::AccountId} in its account field, so none of them reports.
- The four local eval directories report exactly what they reported before.
- A fixture shaped like a skill's references/iam-policy.json, two statements
with wildcard regions naming one account: 0 proved and 0 findings before the
fix, 1 proved and 2 whole-ARN findings after.
- The scanner still performs no repository write: git diff and git show only,
and one append to $GITHUB_STEP_SUMMARY.
The ARN pattern lines up five colon-separated fields to find the account in
the fifth. A CloudFormation template expression carries a colon pair of its
own, so a region written as ${AWS::Region} left the fields unalignable, the
whole match failed, and the account standing next to it was never read --
neither proved as an account nor reported as a finding.
The field that matters there is not the templated one. A SAM template names
a published Lambda layer as arn:aws:lambda:${AWS::Region}:<a literal
account>:layer:X, so the account beside the expression is a literal twelve
digits, and a failed parse made it invisible. Every field before the
resource now accepts a ${...} expression as one unit, braces and inner
colons included, matched ahead of the plainer character classes so those
classes never stop at a colon inside the braces.
A template expression in the account field itself still yields nothing, but
now because the field is not twelve digits rather than because the ARN did
not parse -- which is the distinction the comment beside the pattern had
claimed and did not have. That comment is corrected, and the placeholder
syntaxes still not covered in a field before the resource are named there
and in the contributor documentation: {Region}, <region> and %REGION% all
fail to parse, so an account beside one of those goes unreported.
Four lines in this repository's SAM templates were affected, all naming the
AWS-owned Lambda Web Adapter account, which is allowlisted -- so they
proved nothing before and report nothing now, and no finding changes.
Verified against every open pull request: the same five report and the same
twenty-eight are clean, finding for finding. The self-check gains four
cases, two proving the account beside a templated region is read and two
proving a templated account field stays silent.
The skill evaluation tool writes _metadata.json at the root of each functional version directory. It records how the run was invoked -- region, iteration count, local skill directory -- and the CloudFormation stacks the tool created, each named by a full stack ARN. Those stack ARNs carry the account the evaluation ran in, which makes this the one file in an eval run that names a real account and keeps it. The tool's redaction pass replaces account IDs in the agent's own output, but _metadata.json comes from the harness rather than from the agent, so the redaction never reaches it. Of the two copies committed today, one names a real account and the other a documentation placeholder; every local run inspected names the account its evaluation used. Nothing reads the file after the run. The evals validation check lists it among the files it deliberately does not look at, so ignoring it changes no other check. The pattern is spelled out as **/evals/functional/*/_metadata.json rather than **/_metadata.json so the name stays committable elsewhere under skills/, matching how the sibling patterns for outputs/ are scoped. Verified that a fresh _metadata.json under a version directory is ignored while benchmark.json and evals.json beside it stay committable. Also ignores .worktrees/. An agent working on a branch in parallel with the checkout creates a worktree there, and each one is a full second copy of the tree, so committing one would nest the repository inside itself. Git keeps a worktree's administrative files under .git/worktrees instead, so ignoring the directory loses nothing. This does not untrack the two _metadata.json files already committed -- ignoring a path has no effect on a path git is already tracking. Removing them is a separate change.
Until now the three contributor documents described the pull request check and nothing else: what it reports, what it deliberately stays quiet about, and how to resolve a finding. Read together they implied the tooling is the control, so a contributor whose pull request goes green had no reason to look further. The tooling is not the control, and two of its limits are the reason. The skill evaluation tool's redaction pass rewrites the agent's own output and only that -- a hand-written evals.json prompt is never touched, nor is the _metadata.json the harness writes at the root of each functional version directory, and a third-party account inside a returned API payload has been seen passing through unredacted while the operating account in the same sentence was replaced. The check, by design, reports an account ID only where something proves it is one, so a green check means nothing was proved rather than that nothing is there. CONTRIBUTING.md gains a "Redacting AWS Identifiers" section, placed ahead of the one describing the check so the contributor's own duty is read first. It gives the replacement for each kind of identifier -- a documentation example account, the example instance ID, an ARN rewritten whole rather than blanked in one field, and a generic stand-in for any other real resource name -- then lists the four places real values arrive, and closes with why neither piece of tooling is enough. The Kiro steering file and .claude/CLAUDE.md carry the same instruction in the short form those two documents use. Agent output is named as three files per run, not one: journal_records.json, benchmark.json, and each scenario's functional-tests-results.json, which quotes the output again as the evidence for every assertion it judged. The same identifier usually appears in more than one, so fixing the journal and assuming the rest followed leaves the value behind. Of the eval result files committed today, instance IDs and account-bearing ARNs appear in functional-tests-results.json across two skills. Also corrects a stale line in both short documents, which still described a single known gap. There are two: an account ID written only as prose in a file that is not a journal, and an ARN whose fields cannot be lined up because a field before the resource uses a placeholder syntax the check does not parse. CONTRIBUTING.md was already updated when the second gap was found. No behaviour changes. Every example uses an allowlisted documentation placeholder, and the three files pass the check they describe.
Every allowlist entry carries a reason, and reviewing that reason is the only control on what the allowlist waives. Three of them described the wrong thing, or only half of it. i-0123456789abcdef0 was described as the AWS documentation example instance ID. It is not: AWS documents i-1234567890abcdef0, both on the EC2 resource ID page and in its API examples. The entry is still worth keeping, and its shape is reason enough -- a counting run through the hexadecimal digits. The reason now adds that the value is used as an example instance ID throughout this repository, which is an observation about where it already appears rather than a convention anyone settled on. i-1234567890abcdef0 and 012345678901 were each described only as the skill evaluation tool's redaction placeholder, which both are. Both are also values AWS documents, so an author copying an example from AWS lands on them, and a reader of either reason alone would think the entry narrower than it is. The account ID appears on the AWS account identifiers page and in the Account Management reference; the instance ID on the EC2 resource ID page. No value is added or removed, so nothing changes about what the check reports. Verified that the file still parses and the scanner's self-check passes, which it would not if a reason were left blank.
Scanning every open pull request for identifiers that are plainly fabricated but absent from the allowlist turned up exactly two, both in a hand-written eval fixture: i-0a1b2c3d4e5f60011 and i-0a1b2c3d4e5f60022. They are the ascending digits interleaved with letters that i-0a1b2c3d4e5f67890 already uses, ending in a repeated pair instead. Reporting them would fail a pull request over values that name nothing. That is the failure worth avoiding most while this check is new, because an author who meets it has nothing to redact and no way to tell the check is wrong rather than themselves. The account side needed no such additions. Every plainly fabricated account ID found in the tree or on an added line was already allowlisted, so these two are the whole gap. With them in, four open pull requests report and twenty-nine are clean, down from five reporting. The four that remain all name real identifiers.
The three contributor documents described the second evidence source as an object key in the tool result of a tool_summary block. That skips the step a reader needs. The tool result holds a text field, the text field holds JSON as a string, and the account ID is a key of that JSON -- which is the path the scanner's own docstring spells out as tool_summary > tool_result > text. Without naming the text field, the description reads as though the key sits directly in the tool result, so anyone checking the behaviour against a real journal looks in the wrong place. The example now shows the field holding its escaped string rather than the parsed object. Wording only. No change to what is read or reported.
ams-thakkar
approved these changes
Oct 6, 2026
ams-thakkar
left a comment
ams-thakkar
left a comment
Contributor
There was a problem hiding this comment.
Approving at f579b1e. I tested this adversarially rather than reading it, because a check built entirely out of suppressions fails silently when a guard grows too broad — which is the same reason you built --self-check, and it's the right instinct.
It catches real leaks in this repository today. I swept the scanner across all 34 open pull requests. Three fail, and all three are genuine: #109, #115 and #117 between them carry four distinct real AWS account IDs — all in _metadata.json stack ARNs and journal records, none of them placeholders. I am not naming the values here; I will raise them with each author directly. The other 31 pass. That is a 3/34 hit rate with no false positive I could manufacture, which is the number that makes this safe to turn on as a required check.
More pointedly: main already carries an unredacted real account ID in one committed _metadata.json under skills/aws-backup-coverage-review/evals/. It is not allowlisted and it is not a documentation example. I fed that exact file through the scanner as an added file and it reported every ARN. I approved and merged the pull request that introduced it, and nothing in the repository would have told me. That is the strongest argument for this change that I can offer.
What I verified
Against my own corpus, not yours. All five ARN template forms parse and report, including the two I expected to break on internal colons — arn:aws:s3:${AWS::Region}:… and arn:${AWS::Partition}:s3:${AWS::Region}:… — plus a * wildcard region and a plain region. Both instance-ID lengths are caught, 8-hex and 17-hex. An account proved by an ARN is then reported where it appears bare on another line. Both journal sources fire: an object key inside the text of a tool_summary tool result, and an aws_account_id field. A bare twelve-digit run with nothing proving it is correctly ignored — the documented gap, behaving as documented.
No false positive I could construct was reported: a YYYYMMDDHHMM resource-name datestamp, the fractional digits of a decimal, a twelve-digit run inside an eni- ID, a zero-padded counter, a UUID tail, arn:aws:iam::aws:policy/…, and an account-less arn:aws:s3:::bucket. Those first three are exactly the shapes that burned me by hand earlier this week, so I was looking for them.
The allowlist fails closed on all three malformed cases — bad JSON, missing key, blank reason — each exiting 2. In the blank-reason case the account was still reported, so a typo genuinely grants nothing. aws-id-ok: <reason> suppresses silently; a bare aws-id-ok suppresses but warns with the file and line. Exit codes are distinct and correct: 0 clean, 1 found, 2 could-not-run.
The workflow pair mirrors validate-skill-evals closely enough that I have no concerns about it as a required check. No if: guard on the gate job, so it reports a result on every pull request rather than going pending forever on ones it does not care about. contents: read on the scan. The labeler on workflow_run with pull-requests: write scoped to just the labeling job, which is what makes it work for fork pull requests whose own token is read-only. Exit 2 leaves the label exactly as it was, with a notice explaining why — the right call, since the label is a claim about content and an incomplete scan cannot make one.
Nits
None blocking.
- A proved account's hyphenated spelling survives. Given an ARN that proves an account on one line, the scanner reports that ARN and the same account written bare elsewhere, but not the same digits written in the console's
NNNN-NNNN-NNNNgrouping — the form the console displays and people paste. The realistic failure is an author redacting both flagged copies, the hyphenated one remaining, and the check going green. Since pass two already matches the literal value of a proved account, also matching its 4-4-4 grouping would close it without weakening the evidence rule — the value is already proven by then, so there is no new guesswork. - Gitignoring
_metadata.jsontrades away the best provenance signal we have. Your reasoning for ignoring it is sound and the three leaks above are all in that file. But those CloudFormation stack ARNs are the only artifact that proves a skill's functional evals genuinely ran rather than being hand-written — I built a provenance checker on exactly that signal this week, and across 30 skills onmainit is what separates the 3 with real tool output from the 27 without. Having the eval tool redact the account in_metadata.json, the way it already redacts journals to111122223333, would keep both properties. Worth raising with whoever owns the harness rather than solving here. - Related: the ignore rule does not untrack the three
_metadata.jsonfiles already committed onmain, one of which holds the live value above. Needs a separate cleanup commit. _metadata.jsonalso records the author's local filesystem path,/Users/<alias>/…, inskill_dir. Out of scope for an identifier scanner, but the same harness-side redaction pass would cover it.
The measurement behind the design is what sells it — 1008 of 1867 twelve-digit runs naming no account, broken down by shape, is why the two-pass structure is obviously right rather than merely defensible. Reporting the whole ARN instead of the account field, because blanking one field leaves enough to rebuild it, is the kind of detail that only comes from thinking about how the redaction actually gets done.
3 of 6 tasks
kaiserschmarm
added a commit
to kaiserschmarm/tools-for-devops-agents
that referenced
this pull request
Oct 7, 2026
The with_skill journal for the config-cost-negative-trigger scenario committed a real, non-placeholder AWS account ID in four places (two inside escaped-JSON tool payloads, two in assistant prose). Replace it with the eval tool's standard redaction placeholder 111122223333, which is already on .github/aws-identifier-allowlist.json, so the AWS identifier scan (added in aws#119) passes and no real account reaches public history. Verified the value appears in no other tracked file. It remains only in three untracked, now-gitignored evals/functional/v1/_metadata.json files (**/evals/functional/*/_metadata.json), which never enter the PR or history. Addresses feedback item 1.
ams-thakkar
pushed a commit
that referenced
this pull request
Oct 7, 2026
#115) * chore: Scaffold security-service cost optimization skills Add initial directory structure and placeholder files for three new cost-optimization skills (CloudTrail, Config, GuardDuty) and a unified security cost optimizer custom agent. Content to be filled in subsequently. * feat(skills): Add security services cost optimization tools Add three read-only cost optimization skills and a unified custom agent for AWS security and governance services. - cloudtrail-cost-optimization: duplicate management-event trails, Read events, KMS/RDS noise, broad/duplicate data events, Lake, S3 lifecycle - config-cost-optimization: continuous-vs-daily recording frequency, over-broad allSupported, duplicate global-resource recording, CI drivers, redundant rules/conformance packs, S3 lifecycle - guardduty-cost-optimization: per-plan spend attribution from AWS/GuardDuty usage metrics, Runtime Monitoring/VPC Flow Log offset, free-trial projection, framed as cost-vs-security tradeoffs - unified-security-cost-optimizer agent routes to the three skills and produces a consolidated report Update llms.txt catalog and add least-privilege IAM policies (ce:GetCostAndUsage, s3:GetBucketLifecycleConfiguration, athena reads) to the skill-policies CloudFormation template. * fix(skills): Correct conformance-pack and CloudTrail dedup logic Address two flaws surfaced by the security-cost-optimization report for account 650728049843, plus fold in the refactor of the CloudTrail skill into progressive-disclosure references and the updated evals schema. config-cost-optimization: - Treat overlapping conformance packs (PCI + NIST) as intentional dual attestation, not waste. Compliance is tracked per pack, and shared rules map to different framework controls. Recommend consolidation only when separate per-framework reporting is confirmed unnecessary; otherwise report the overlap as INFO with a cost ceiling. - Require matching source identifier and parameters before calling a standalone rule a pack duplicate; flag stricter thresholds as distinct. cloudtrail-cost-optimization: - Add a dedup-vs-filtering interaction rule: after de-duplicating to one management-event trail, the survivor is the free first copy, so KMS/RDS exclusion on it saves ~$0 and removes coverage. Filtering applies only to a retained paid copy; never stack the two savings. - Refactor SKILL.md into progressive-disclosure references (billing-model, opportunities, api-inventory) and an assets report template. - Update evals.json to the structured {skill_name, evals[]} schema. Bump both skills to 1.1.0 with CHANGELOG entries. * feat(skills): Add functional evals and finalize progressive-disclosure refactor for config & guardduty cost skills Complete the config-cost-optimization and guardduty-cost-optimization refactor to progressive disclosure and add passing eval suites. Both skills: - SKILL.md is now a slim checkbox-checklist workflow; detailed material moved into references/ (billing-model, data-collection, opportunities) and assets/report-template.md, matching the cloudtrail skill structure. - best-practices evals now score 100/100 (was config 88, guardduty 94), fixing BP-03, BP-12, BP-16 and the BP-17 validation-loop warning. - evals.json migrated to the current schema and given a file-independent, uplift-oriented functional suite (6 scenarios each incl. a negative trigger); structure + best-practices + functional results committed. guardduty (1.1.1): sharpened the Runtime Monitoring / VPC Flow Log offset check so the agent-gap case explains the "worst of both worlds" (offset does not apply, so flow-log charges and the plan are both paid with no runtime coverage). config stays 1.2.0. validate_skill_evals.py passes for both skills. * feat(skills): Migrate cloudtrail cost eval suite to file-independent scenarios and commit results Bring cloudtrail-cost-optimization in line with the config and guardduty cost skills: migrate evals.json to the current schema with a file-independent, uplift-oriented functional suite and commit the eval results. - evals.json: inlined duplicate-reasoning and artifact-naming (no longer depend on files/*-context.json, which the eval harness does not deliver), added a dedup-vs-exclusion interaction scenario exercising the §4.1/§4.3 rule, and a negative-trigger case. - Functional run: 6 scenarios x 3 iterations, 33/33 clean, expected_output 3/3 on every triggering scenario, two skill_uplift scenarios, negative trigger correctly suppressed. Best-practices 100/100; structure passing. - CHANGELOG 1.1.1 documents the progressive-disclosure refactor and evals migration. validate_skill_evals.py passes for cloudtrail-cost-optimization. * chore(skills): Trim eval results to benchmark + iteration-1 for cost skills Keep only benchmark.json (and functional evals.json/_metadata.json) plus a single iteration-1 per test type for cloudtrail-, config-, and guardduty-cost-optimization; drop iteration-2 and iteration-3. The roll-up scores are preserved and validate_skill_evals.py still passes (a complete version needs only one iteration with results). * chore(skills): Stop tracking functional eval _metadata.json (account-linked) The functional _metadata.json files embed environment-specific, account-linked data (CloudFormation stack ARNs with the eval AWS account ID, agent-space IDs, and local filesystem paths) and are not required by validate-skill-evals.yml. Remove the three committed copies from tracking and add a skills/.gitignore rule so future eval runs do not re-add them. Eval validation still passes. * fix(security): Treat ingested data as untrusted and require evidence for coverage cuts Addresses H1 from peer review on the unified-security-cost-optimizer PR. The three cost-optimization skills ingest attacker-influenceable data (S3 bucket names, resource tags, Cost Explorer usage-type strings, CloudWatch metric dimensions, GuardDuty finding statistics, resource identifiers) and their output is advice to reduce CloudTrail, Config, and GuardDuty coverage. Nothing previously told the agent that data was untrusted, so a crafted tag or name could steer it into recommending security monitoring be turned off. - SYSTEM_PROMPT and all three SKILL.md "Safety and Boundaries" sections now state that all ingested resource, usage, finding, and cost data is untrusted and must never be followed as instructions. - Every coverage-reducing recommendation must now state its security impact in plain language and cite the specific evidence (metric, API field, resource) it rests on, so a human can verify independently before acting. Enforced via each skill's Step 5 validation check and new Security Impact / Evidence columns in the three report templates and the consolidated report. - Version bumps: cloudtrail 1.1.1->1.2.0, config 1.2.0->1.3.0, guardduty 1.1.1->1.2.0, agent 1.0.0->1.1.0, with CHANGELOG entries. * fix(config-cost-optimization): Document Athena prerequisites and gate the IAM Addresses M1 from peer review on the unified-security-cost-optimizer PR. The skill advertised an Athena CI-driver attribution path and the template granted athena:StartQueryExecution/GetQueryExecution/GetQueryResults, but the path could not work on a default DevOps Agent setup: - the DevOps Agent role has no s3:PutObject, so Athena must use a workgroup configured with Athena-managed query results; - AIDevOpsAgentAccessPolicy grants no s3:GetObject (only s3:ListBucket on AWSLogs/ prefixes), so the query engine cannot read the Config S3 data; - querying Config data via Athena also needs Glue Data Catalog reads. On a default setup the path fails with AccessDenied. Changes: - SKILL.md Step 3, references/data-collection.md, and README now spell out the workgroup, s3:GetObject, and Glue requirements, and instruct the agent to fall back to GetDiscoveredResourceCounts (labeled approximate) instead of attempting an Athena query it cannot complete. - CloudFormation: moved the Athena actions out of the always-on base policy into a new off-by-default EnableConfigAthenaCiAnalysis add-on granting the complete working set (athena query/results/workgroup, glue catalog reads, s3:GetObject scoped via ConfigDataBucketArn). The base Config policy now grants only what works out of the box; SkillPolicySummary reflects the split. - Version bump: config-cost-optimization 1.3.0 -> 1.4.0. * fix(evals): Redact real AWS account ID from config-cost eval journal The with_skill journal for the config-cost-negative-trigger scenario committed a real, non-placeholder AWS account ID in four places (two inside escaped-JSON tool payloads, two in assistant prose). Replace it with the eval tool's standard redaction placeholder 111122223333, which is already on .github/aws-identifier-allowlist.json, so the AWS identifier scan (added in #119) passes and no real account reaches public history. Verified the value appears in no other tracked file. It remains only in three untracked, now-gitignored evals/functional/v1/_metadata.json files (**/evals/functional/*/_metadata.json), which never enter the PR or history. Addresses feedback item 1. * fix(docs): Use absolute GitHub URL for cloudformation template links The strict docs build (mkdocs build --strict) fails on five warnings, all the same broken link to ../../cloudformation/devops-agent-skill-policies.yaml. The docs hook copies each skill/agent README verbatim into the docs tree (docs/skills/<id>.md), so a relative path to cloudformation/ — which is not part of the docs tree — can never resolve, unlike a skill-to-skill link. On merge this would turn the docs deploy red (main has been at zero warnings since #122). Replace the relative links with the absolute form already used by three merged skills (aws-backup-coverage-review, msk-operations, database-migration-service-expertise): https://github.com/aws/tools-for-devops-agent/blob/main/cloudformation/devops-agent-skill-policies.yaml Fixed the five README/agent links that trigger the warnings, plus the same link in config-cost-optimization/references/data-collection.md for consistency (it is not copied into the docs tree, so it does not warn, but matching the absolute form keeps the cross-tree link robust). Verified locally with `mkdocs build --strict` (mkdocs-material==9.6.14, the CI pin): exit 0, zero WARNING lines. Addresses feedback item 2. * fix(iam): Remove three no-op per-skill IAM gates The EnableCloudTrailCostOptimization, EnableConfigCostOptimization and EnableGuardDutyCostOptimization CloudFormation gates granted nothing, and one of their two actions was not a real IAM action. Verified against the live AIDevOpsAgentAccessPolicy default version v11 (iam:GetPolicyVersion): - ce:GetCostAndUsage is already granted by the managed policy via ce:Get* on * with no condition. - s3:GetBucketLifecycleConfiguration is not a valid IAM action (it is the API name); cfn-lint flagged it as W3037. The real action, s3:GetLifecycleConfiguration, is also already granted on * by the managed policy. So all three gates added no permission in any combination and their descriptions were wrong on both counts. Deleted the three parameters, their conditions, and their policy resources, and updated the three skill READMEs and the agent README to state no additional IAM is required (the pattern aiml-gpu-training-cluster-investigation uses). This also clears the two W3037 warnings the branch introduced; cfn-lint is now 4x W2001 only, matching main. EnableConfigAthenaCiAnalysis is kept — it is genuinely load-bearing (s3:GetObject and the three athena query actions are not in the managed policy). Its condition was rewired to no longer reference the removed EnableConfigCostOptimization parameter, and its docs now note that the Glue reads and athena:GetWorkGroup it also uses are already covered by the managed policy. Addresses feedback item 3.
Contributor
|
fixed and added commit to #109 . 1/ Replaced eight AWS account IDs that was in the functional eval run artifacts under evals/functional/ with the standard placeholder 123456789012. 2/Also stopped adding the per run _metadata.json to commit(Agent Space IDs, stack ARNs, environment account IDs), and added a gitignore entry for functional/*/_metadata.json. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds a read-only pull request check that reports unredacted AWS account IDs and EC2 instance IDs on the lines a pull request adds, plus a companion workflow that applies and removes a
needs-id-redactlabel from the result. The intent is to make it a required status check onmainafter merge, so the manual steps at the end matter.GitHub secret scanning does not cover account IDs or instance IDs, since neither is a credential. Nothing in the repository catches them today.
An account ID is reported only where something proves it is one. Twelve digits on their own are not evidence. Measured across the 33 open pull requests, 54% of the twelve-digit runs on their added lines named no account at all, in five shapes: a
YYYYMMDDHHMMdatestamp used as a resource-name suffix, the fractional digits of a decimal, the integer part of a decimal, a zero-padded counter, and a twelve-digit run inside a longer hexadecimal resource ID such as a network interface ID. So the check runs in two passes.Pass one gathers evidence and reports nothing. Three places put a value where only an account ID sits:
textfield of atool_summaryblock's tool result, in ajournal_records.jsonfiletextholds JSON as a string, and that JSON is keyed by account ID, so the key itself is the account:"text": "{\"123456789012\": {\"DBInstances\": []}}"aws_account_idfield, in ajournal_records.jsonfile"aws_account_id": "123456789012"The two journal sources are fields of the DevOps Agent journal schema, which is why they are read only in a file of that name. Both belong to the tool DevOps Agent uses to run an AWS API call or a
kubectlcommand, and they are two ways that tool represents the account ID the call was made in, which is why the check reads both rather than picking one. That is what makes them worth reading: an account the tool was pointed at is an account somebody really used, so the value is an account ID by construction rather than by its shape.Other spellings an API response might use,
AccountIdandaccountIdamong them, are deliberately not read. They are whatever shape a payload happened to have, so building on them means the check drifts whenever a response changes, and nothing about the position says the value is an account rather than a number that looks like one.Pass one reads every changed file whole, not just the added lines, so an account ID written bare on an added line is still caught when the ARN that proves it sits in an untouched part of the same file.
Two things exempt a value from being reported, and both are visible in the diff under review.
The first is an allowlist,
.github/aws-identifier-allowlist.json, added by this pull request. It holds account IDs and instance IDs that are safe to publish, each with a reason: the AWS documentation examples, the skill evaluation tool's redaction placeholders, two AWS-owned accounts that appear legitimately in SAM templates and in a container image reference, and several plainly fabricated instance IDs. An allowlisted account ID is dropped from the proven set, which silences both the bare occurrences of it and any ARN that names it. The file fails closed: invalid JSON, a missing key or a blank reason grants nothing at all and fails the check, so a typo cannot quietly waive a leak.The second is a per-line marker. A line carrying an
aws-id-ok: <reason>comment is skipped, which covers the one-off case where an allowlist entry would be too broad. It works only in file types that have comments, and JSON has none, so the allowlist is the route for JSON content. A bareaws-id-okwith no reason still suppresses the line but produces a warning naming the file and line, so an unexplained suppression is visible rather than silent.Pass two reports, on added lines only: an ARN whose account field holds a twelve-digit value the allowlist does not cover, reported as the whole ARN; every occurrence of an account ID pass one proved, in any file type; and an EC2 instance ID, which needs no evidence because its prefix and length are the proof.
Reporting the whole ARN rather than its account field is deliberate. Blanking one field leaves the region, the service and the resource name in place, and a surviving copy of the account ID elsewhere in the file would be enough to rebuild the ARN.
The scanner checks itself before it checks the pull request. It takes a
--self-checkflag that scans nothing and instead runs the scanner's own cases: over the evidence pass, over the reporting pass, and over every allowlist entry both bare and with a redaction suffix, exiting non-zero if any case fails. The gate runs it ahead of the diff scan on every pull request. It is there because every guard in this scanner works by suppressing something, and a guard that grows too broad fails silently -- identifiers stop being reported, and a check that still passes is the only symptom.The check reports three outcomes, not two. Nothing found, which exits 0; identifiers found, which exits 1; and the scan could not run, which exits 2 -- a malformed allowlist, a failed fetch of the base branch, a failed self-check. The job fails on exit 1 and exit 2 alike, so a broken scan is never mistaken for a clean one. The label follows a different split: exit 0 removes
needs-id-redact, exit 1 applies it, and exit 2 leaves it exactly as it was, because the label is a claim about a pull request's content and a scan that did not complete is in no position to make it.Only the lines a pull request adds are reported on.
mainalready carries real identifiers in committed eval output and example ARNs, so reporting on whole files would fail contributors for content they never wrote. Renames are detected, so moving such a file reports nothing.An ARN with no account field is not reported. For example,
arn:aws:s3:::my-internal-bucketnames no account, and there is no reliable way to tell a real bucket from an example one, since anybody may own that name. Of the 121 account-less ARNs on open pull request added lines, none names a private resource: 97 are templates or explicit examples, 21 are placeholders or AWS-published service quota codes, 3 are truncated. Redacting a real one is the author's job and catching it is a reviewer's.Three documentation files gain a Redacting AWS Identifiers section, placed ahead of the one describing the check, because neither the skill evaluation tool's redaction pass nor this check covers everything, and the author's own duty should be read first.
Case-by-case behavior
needs-id-redactapplied, whole ARN reported123456789012or111122223333arn:aws:iam::aws:policy/...{Region},<region>or%REGION%*wildcard and a CloudFormation${...}expression both parseaws-id-ok: <reason>aws-id-okwith no reasonTesting
The two journal evidence sources. Six cases in
--self-checkpin them, so they are reproducible from this pull request by running the scanner: thetool_summarykey at the nesting depth a real journal uses, with its inner JSON escaped as a string; the same twelve-digit key under a block that is not atool_summary, proving nothing;aws_account_idin a parsed tool input and again in agent prose; a journal too truncated to parse, falling back to the field pattern; and abenchmark.jsoncarrying both an ARN and anaws_account_id, where only the ARN counts because the journal fields are read in a journal alone.One open pull request exercises the
tool_summarypath on real recorded output: its account is proved by that key and by nothing else, since no ARN in it names the account, and the check reports all four occurrences including the one in agent prose.Both paths were then exercised against a wider local corpus -- every
journal_records.jsonfile in one skill's eval history, 1039 files across 190 runs. Those files are not in this repository, so the counts below are not reproducible from this pull request. All 1039 parse end to end, 547 yield an account through thetool_summarykey, 305 throughaws_account_id, and 829 yield at least one.Neither source would do on its own. Across six consecutive runs, two are reachable only through the
tool_summarykey and two only throughaws_account_id-- the same scenario recorded one way in one run and the other way in another. A raw-text match onaws_account_idruns alongside the tree walk as a fallback; it found the field in four files the walk did not reach, at a nesting depth past the walk's limit, and it can only ever add what the walk would have added.Only two distinct accounts come out of all 1039 files, because the corpus is one skill repeating a handful of scenarios: a real account in 799 files, and the allowlisted redaction placeholder in 30, which is the skill evaluation tool's redaction pass showing up in the newer runs.
Against every open pull request. Four report and twenty-nine are clean. The four all name real identifiers: stack ARNs in two, real instance IDs and accounts in one, and an account proved by a
tool_summarykey in one. None of the five false-positive shapes is reported anywhere -- checked by name in the two pull requests that carry them.Placeholders cross-checked against the allowlist. Scanned
mainand every open pull request for obvious placeholders absent from the allowlist. Two were found, both plainly fabricated instance IDs in a hand-written eval fixture, and both are now allowlisted so no author meets a finding with nothing to redact. No account placeholder was missing.Fixture suite, 14 cases. A real account ID and both instance ID lengths fail; every allowlist entry passes bare and
_N-suffixed; a UUID ending in twelve digits passes; a network interface ID containing a twelve-digit run passes; a hyphen-prefixed account ID still fails; a removed line passes; a pure rename passes; an empty diff passes; two malformed allowlists exit 2.The self-check detects a guard that has grown too broad, rather than passing regardless. 17 evidence cases, 20 reporting cases and 14 allowlist entries pass as they stand. Loosening the ARN pattern on purpose, so that an account-less ARN starts yielding a bogus account, makes two of them fail and name what they expected -- which is the failure mode the cases exist to catch.
Forced-failure tests of the gate's own shell. Ran the scan step's
run:block withgitandpython3stubbed to force each path. A clean scan recordspass, findings recordfail, and a config error, a failed base-branch fetch or a failed self-check all recorderror, which leaves the label untouched. Before this, a failed fetch died under errexit before writing its exit code, and the empty value read downstream becamefail-- labelling a pull request that contained nothing.This pull request passes its own check, 2043 added lines and no findings, which is why every example in it uses an allowlisted placeholder.
Both workflows parse, every
run:block passesbash -n, and the job id and display name are identical, since the display name is the status check context.Read-only, verified by inspection. The scanner's only subprocess calls are
git diffandgit show; its only file write appends to$GITHUB_STEP_SUMMARY. The gate runs undercontents: readand writes a remote-tracking ref in its own clone,$GITHUB_OUTPUT, and a one-day artifact. The labeler does no checkout and writes nothing but labels.Verified against the fix in #105
That pull request found the evals labeler had never applied or removed a label:
ghresolves its repository from git remotes, the job does no checkout, so every call failed before reaching the API -- and blanket error suppression on the pass branch reported success while doing nothing. This labeler was written from the fixed version, and the fix was re-tested here rather than assumed. Running the calls from a directory that is not a git repository fails withoutGH_REPOand succeeds with it. Both branches of the removal conditional behave against live pull requests: a label the pull request carries returnstrue, one it does not returnsfalse, a pull request with no labels at all returnsfalserather than erroring, and a failed lookup exits non-zero so the job fails instead of reporting "label absent".What could not be tested before merge
workflow_runruns its definition from the default branch, so the labeling workflow cannot fire on the pull request that introduces it. Confirmed on this pull request: the gate ran and reported, whilegh run listfor the labeling workflow answers that no workflow of that name exists. Untested until then: the artifact download across runs, the per-jobpull-requests: writeelevation, and the three label writes. None of them can affect pass or fail -- the gate runs in the other workflow and needs no write access. The producing half is verified: the gate's log shows the recorded values and the artifact uploads.Manual steps
The first step can be done at any time. The rest wait for the merge, because
pull_requestresolves the workflow from each pull request's merge commit, so until the gate is onmainit applies to this pull request alone, andworkflow_runresolves its definition from the default branch, so the labeling workflow does not exist as far as GitHub is concerned.Create the
needs-id-redactlabel. Before or after merging, either is fine. It does not exist yet. The labeling workflow creates it on its first failing run, but that call is wrapped in|| trueso that it tolerates the label already existing -- which means a creation that fails for any other reason is swallowed, and the only symptom is the--add-labelthat follows it failing. Creating the label deliberately removes that dependency, and makes the labeling check in step 2 a test of the add and remove calls alone:gh label create needs-id-redact \ --description "Unredacted AWS account IDs or EC2 instance IDs found in this pull request" \ --color D93F0BThe description and colour match what the workflow would have used, so the label looks the same either way.
Confirm the labeling workflow works. After merging, this is its first possible run. Pick a pull request the check fails, confirm
needs-id-redactappears, then confirm it is removed after a passing run:gh run list --workflow "Label AWS identifier scan result" --limit 5Sweep every open pull request, not only the ones the check would fail. A required check with no reported status blocks a pull request identically to a failure, minus the log:
Use
ci-sweeprather thanneeds-id-redact. The labeling workflow owns the latter, so applying it by hand makes the workflow's own behaviour unobservable -- you could not tell whether the label is there because the check failed or because somebody put it there.Add
scan-aws-identifiersto the required status checks formain. Either route works.Through the API, appending to the list that already holds
validate-skill-evals:gh api -X POST \ repos/aws/tools-for-devops-agent/branches/main/protection/required_status_checks/contexts \ -f 'contexts[]=scan-aws-identifiers'Or in Settings, Branches, by editing the rule for
mainand addingscan-aws-identifiersto the status checks that must pass. The picker only offers a context that has reported recently, which this one has -- the gate ran on this pull request and reported under that name.Do this after the sweep in step 3, not before. Branch protection evaluates statuses already sitting on each head commit, so it is retroactive: sweeping first means every failing pull request blocks immediately with its log attached, while flipping the setting first leaves them blocked with nothing to show their authors.
Tell the authors of the failing pull requests. Actions failure mail goes only to whoever triggered the run, so a maintainer sweeping an idle pull request gets the mail and its author gets nothing. For the one carrying a large eval run, the remedy is a re-run with the current redaction rather than hand-editing.
Identifiers already on
mainare untouched by this change, since the check reads added lines only. Cleaning them up is a separate pull request: two tracked_metadata.jsonfiles, real account IDs in five skills' eval output, instance IDs in one skill's functional run, and one in two MCP source comments.Type of change
License confirmation