feat: add AWS Config compliance-results-to-OSCAL-AR task - #2334
Conversation
Adds a new 'aws-config-result-to-oscal-ar' task + transformer (mirroring the existing tanium/osco task+transformer shape) that converts AWS Config compliance evaluation results into OSCAL Assessment Results. Input: the json shape returned by AWS Config's get-compliance-details-by-config-rule / get-compliance-details-by-resource APIs (a top-level EvaluationResults list of EvaluationResult objects). Field names verified against: https://docs.aws.amazon.com/config/latest/APIReference/API_EvaluationResult.html https://docs.aws.amazon.com/config/latest/APIReference/API_EvaluationResultQualifier.html Output: one OSCAL Result per input file, with one Observation per EvaluationResult (method TEST-AUTOMATED), an inventory-item per distinct AWS resource (deduplicated across repeated evaluations of the same resource), and compliance-type/config-rule-name/etc. carried as props. Control mapping is intentionally include-all (AWS Config rules aren't mapped to specific catalog controls out of the box), same approach OSCO takes for the analogous case. Verified against a real installed trestle (editable install, Python 3.12): - 11 transformer-level tests (tests/trestle/transforms/implementations/ aws_config_test.py) against a schema-verified fixture -- inventory dedup, subject linkage, compliance-type/annotation props, missing optional fields, full oscal_serialize_json_bytes round-trip. - 6 task-level tests (tests/trestle/tasks/aws_config_result_to_oscal_ar_test.py) exercising the real TaskBase.execute()/simulate() path with actual file I/O, output-overwrite handling, and missing-config failure modes. - Confirmed 'trestle task -l' lists the new task correctly alongside osco-result-to-oscal-ar and tanium-result-to-oscal-ar. - flake8 clean on both new source files. Two real bugs were caught and fixed by this testing before submission: InventoryItem does not have a 'status' field (that's a SystemComponent field; my first draft wrongly copied it), and OSCAL's Observation.props requires min_length=1 when present, so it must be omitted rather than passed as an empty list. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Ahmed Hassan <th3reality72@gmail.com>
398a517 to
bb85e8c
Compare
|
@AAH20 Thx for this PR. Q1: Do you have an IDF on slack and can you join our slack channel #oscal-compass-trestle-agileauthoring-c2p? Q2: Would you be able to present this enhancement at an upcoming community meeting, next one is Aug 25 at 11:00 AM ET? |
|
Here is Claude's review: This is a solid, well-scoped PR: a new aws-config-result-to-oscal-ar task + AwsConfigResultToOscalARTransformer that mirrors the existing osco/tanium Task + ResultsTransformer + private-factory pattern. The write-up is unusually thorough — verified AWS field names against the actual API docs, disclosed two real bugs caught during testing, and proactively flagged open questions (control mapping, paginated exports) rather than guessing. 17 new tests cover dedup, subject linkage, prop handling, and a full OSCAL serialization round-trip. Things worth raising in review
python This class attribute is evaluated once when the module loads, not per-call — a classic mutable/computed-default pitfall. In practice it's harmless today because transform() always passes self.get_timestamp() explicitly into _OscalResultsFactory(...), so the frozen default is never actually used on the real code path. But it's a landmine for anyone who later instantiates _OscalResultsFactory() directly (e.g. in a long-running process), since they'd silently get a stale, load-time timestamp. Worth simplifying or dropping the default.
python runs unconditionally before the per-file loop, even when self._simulate is True. So a "dry run" leaves behind an empty output directory as a side effect. The test tolerates this (assert not output_dir.exists() or not list(...)), which suggests the behavior was noticed but accepted rather than fixed — probably worth gating this behind if not self._simulate: for a true no-op simulate.
python This will throw on any non-JSON file, dotfile, or nested subdirectory in input-dir (e.g. IsADirectoryError from open()). Worth checking whether osco.py/tanium.py already guard this — if not, this task inherits the same weakness, but it's a good opportunity to add an is_file()/extension check.
What's good None of the above are blockers — they're the kind of nits a maintainer would likely raise before merge, especially #1–#3 since they're straightforward fixes. You are out of free messages until 6:40 PM |
- default_timestamp no longer frozen at import time; computed per call - simulate() no longer creates the output directory - Input iteration skips non-.json/.jsn files and subdirectories - class_ on compliance-type prop documented with a comment - Added aws-config task section to cli.md and compliance_posture.md - Added test for non-json/directory skipping Addresses degenaro's Claude review on oscal-compass#2334. Signed-off-by: Ahmed Hassan <th3reality72@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com>
|
@degenaro Thanks for the review and for sharing the Claude analysis — every point was fair. I've pushed a follow-up commit addressing the nits:
Re your questions: I'd love to join the Slack channel and present at the Aug 25 community meeting. I'll request access to |
|
@AAH20 Please fix DCO issue. |
- default_timestamp no longer frozen at import time; computed per call - simulate() no longer creates the output directory - Input iteration skips non-.json/.jsn files and subdirectories - class_ on compliance-type prop documented with a comment - Added aws-config task section to cli.md and compliance_posture.md - Added test for non-json/directory skipping Addresses degenaro's Claude review on oscal-compass#2334. Signed-off-by: Ahmed Hassan <th3reality72@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com>
65b55ec to
ba043ac
Compare
|
@AAH20 I penciled you in for Aug 25th to present your work on PR 2334. Thx! Agenda is here https://docs.google.com/document/d/1XTYM7xnWlIqd-8Nn5-qtgvgk8kH3NSmYle5yZvaS7qs/edit?tab=t.0 Meeting link is here: https://oscal-compass.dev/community/#community-meetings-and-communications |
|
@AAH20 reminder that tomorrow 11:00AM ET is our next regular community meeting. We hope to you can make a short presentation/discussion on this contribution to our code base, thx! |
|
Thanks for the invite — I'll be there for the 11:00 AM ET community meeting today. |
What this adds
A new
aws-config-result-to-oscal-artask +AwsConfigResultToOscalARTransformer, converting AWS Config compliance evaluation results into OSCAL Assessment Results — following the same Task +ResultsTransformer+ private results-factory shape as the existingtanium/oscoAR tasks (closest analogues I found and mirrored).Input: the JSON shape returned by AWS Config's
get-compliance-details-by-config-rule/get-compliance-details-by-resourceAPIs ({"EvaluationResults": [EvaluationResult, ...]}). Every field name was verified against AWS's own API reference rather than assumed:Output: one
Resultper input file, oneObservationperEvaluationResult(methods: ['TEST-AUTOMATED']), anInventoryItemper distinct AWS resource — de-duplicated when the same resource is evaluated more than once — andcompliance-type/config-rule-name/evaluation-mode/config-rule-invoked-time/result-tokencarried asprops.Control mapping is intentionally
include-all: AWS Config rules aren't mapped to specific catalog controls out of the box (that mapping is org/catalog-specific), so I followed the same approach OSCO takes for the analogous case rather than fabricate a mapping.Testing
Editable-installed the real package (Python 3.12) and ran everything against it, not just against my own code:
tests/trestle/transforms/implementations/aws_config_test.py) against a fixture (tests/data/tasks/aws-config/aws-config-sample.json) hand-built to match AWS's documented schema field-for-field — covering inventory de-duplication, subject→inventory-item linkage, compliance-type/annotation props, missing-optional-fields handling, and a fulloscal_serialize_json_bytes()round-trip.tests/trestle/tasks/aws_config_result_to_oscal_ar_test.py) exercising the realTaskBase.execute()/simulate()path with actual file I/O,output-overwritehandling, and the missing-config failure mode.execute()directly against a temp workspace and inspected the written file — genuine, valid OSCAL JSON.trestle task -l(in an initialized trestle workspace) listsaws-config-result-to-oscal-arcorrectly alongsideosco-result-to-oscal-arandtanium-result-to-oscal-ar.flake8clean on both new source files.Two real bugs caught and fixed by this testing before submission (leaving them in the commit message for transparency): my first draft gave
InventoryItemastatusfield, which doesn't exist on that model (I'd confused it withSystemComponent's field) — pydantic rejected it immediately. Second, OSCAL'sObservation.propsrequiresmin_length=1when present, so an empty list has to be omitted from the payload entirely rather than passed as[].Known gap — please advise
Same as the transformer/task pattern in
osco.py,content-typehandling for compressed/paginated AWS Config exports isn't handled — this covers the single-file JSON case. Happy to extend if there's a preferred shape for larger exports (e.g. AWS Config's S3 delivery channel snapshots, which paginate differently than the CLI's direct API output).Also open to feedback on whether
ReviewedControlsshould stayinclude-allor whether there's a preferred convention for AWS Config → OSCAL catalog control mapping I should follow instead.