Skip to content

feat: CPLYTM-1285 - add AMPEL plugin for complyctl#380

Draft
marcusburghardt wants to merge 13 commits intocomplytime:mainfrom
marcusburghardt:001-ampel-branch-scan
Draft

feat: CPLYTM-1285 - add AMPEL plugin for complyctl#380
marcusburghardt wants to merge 13 commits intocomplytime:mainfrom
marcusburghardt:001-ampel-branch-scan

Conversation

@marcusburghardt
Copy link
Contributor

@marcusburghardt marcusburghardt commented Feb 16, 2026 •
edited
Loading

Summary

Implement a complyctl plugin that translates OSCAL assessment plan rules into AMPEL policy files, scans GitHub/GitLab repositories for branch protection compliance using snappy and ampel verify, and produces per-repository result files mapped to the standardized PVPResult format.

Plugin packages:

  • config: Plugin configuration with path resolution
  • convert: OSCAL-to-AMPEL policy conversion with CEL expressions
  • scan: snappy + ampel verify CLI orchestration
  • results: Output parsing, sanitization, and PVP mapping
  • targets: Target repository config loading and validation
  • toolcheck: Required tool presence validation
  • server: policy.Provider interface implementation

Includes specification artifacts (spec, plan, tasks, data model, research, contracts) and 69 unit tests at 86.4% coverage.

Related Issues

  • New feature so we can gradually move from consuming Ampel directly to using it as complyctl plugin where we can benefit from the standardized formats and user interface offered by complyctl.
  • https://issues.redhat.com/browse/CPLYTM-1285

Review Hints

  • This PR was assisted by SpecKit and ClaudeCode Opus 4.6.
  • Checking the spec files is the best start to review this PR.
  • The initial code was mainly created by code agents and manually reviewed to apply improvements
  • I ensured all the context in the README.md, which is the best starting point to review this PR.
  • For technical tests, there also a PR in complytime-demos that was developed in parallel. I recommend to use it:

@marcusburghardt marcusburghardt changed the title feat(plugin): Add AMPEL branch protection scanning plugin feat: Add AMPEL branch protection scanning plugin Feb 17, 2026
@marcusburghardt marcusburghardt mentioned this pull request Feb 17, 2026
Open
@marcusburghardt marcusburghardt changed the title feat: Add AMPEL branch protection scanning plugin feat: add AMPEL plugin for complyctl Feb 17, 2026
@marcusburghardt marcusburghardt changed the title feat: add AMPEL plugin for complyctl feat: CPLYTM-1285 - add AMPEL plugin for complyctl Feb 17, 2026
gvauter
gvauter requested changes Feb 17, 2026
View reviewed changes
Copy link
Member

@gvauter gvauter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall LGTM, left a few small comments on some duplicate logic and one potential config conflict.

cmd/ampel-plugin/config/config.go
// DefaultPolicyDir is the default directory name for granular AMPEL policy source files.
DefaultPolicyDir = "policy"
// GeneratedPolicyDir is the workspace subdirectory for generated policy artifacts.
GeneratedPolicyDir = "policy"
Copy link
Member

@gvauter gvauter Feb 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One nit here as this could become fragile. If the output filename changes or additional artifacts get written, the loader will try to parse them. Perhaps use a distinct directory for generated output (e.g., GeneratedPolicyDir = "generated") to separate these cleanly.

Copy link
Contributor Author

@marcusburghardt marcusburghardt Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hum, actually DefaultPolicyDir should point to the workspace or wherever the granular policies are cached. The GeneratedPolicyDir defines where the Ampel will find the policy to be used during the scan. I will keep this open and review when rebasing with #381 . Thanks

@marcusburghardt marcusburghardt mentioned this pull request Feb 24, 2026
Open
marcusburghardt and others added 13 commits February 27, 2026 15:19
@marcusburghardt @claude
feat(plugin): Add AMPEL branch protection scanning plugin
b6f0f36 
Implement a complyctl plugin that translates OSCAL assessment plan rules
into AMPEL policy files, scans GitHub/GitLab repositories for branch
protection compliance using snappy and ampel verify, and produces
per-repository result files mapped to the standardized PVPResult format.

Plugin packages:
- config: Plugin configuration with path resolution
- convert: OSCAL-to-AMPEL policy conversion with CEL expressions
- scan: snappy + ampel verify CLI orchestration
- results: Output parsing, sanitization, and PVP mapping
- targets: Target repository config loading and validation
- toolcheck: Required tool presence validation
- server: policy.Provider interface implementation

Includes specification artifacts (spec, plan, tasks, data model,
research, contracts) and 69 unit tests at 86.4% coverage.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@marcusburghardt @claude
fix(plugin): Use correct snappy/ampel CLI interfaces for scan
1067a3a 
The scan commands were using incorrect CLI flags. This updates them to
match the real tool interfaces: snappy uses spec files with --var flags
for variable substitution, and ampel verify takes a positional sha256
subject hash with -p and -a flags. Attestations from snappy are now
saved to disk and their subject hash is extracted to pass to ampel.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@marcusburghardt @claude
feat(plugin): Replace CEL generation with granular AMPEL policy matching
debaef1 
Replace monolithic CEL expression generation with a granular policy
matching system. Granular per-policy-ID AMPEL files are loaded from a
configurable source directory, matched against OSCAL assessment plan
rules, and merged into a single bundle for scan. The ampel verify
output is saved as in-toto attestation files (ampel.intoto.json) so
results can be properly interpreted for assessment-results.json.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Marcus Burghardt <maburgha@redhat.com>
@marcusburghardt @claude
feat(plugin): Support configurable snappy spec files per target
fa8e887 
Allow targets to optionally declare which snappy spec files to use via a
`specs` list in the targets YAML. Each spec triggers a separate
snappy+ampel cycle, enabling multiple specs per target and external spec
files. Targets without specs fall back to the default embedded spec.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@marcusburghardt @claude
feat(plugin): Make specs mandatory and support builtin: prefix
b31fa52 
Targets without specs are now skipped with a warning instead of falling
back to DefaultSpecs(). The builtin: prefix (e.g. builtin:github/branch-rules.yaml)
is passed through to snappy as-is for its built-in spec resolution.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@marcusburghardt
chore: show ampel output in debug level
dc9dd87 
Signed-off-by: Marcus Burghardt <maburgha@redhat.com>
@marcusburghardt @claude
fix(plugin): Handle DSSE envelopes and group observations by CheckID
a42f508 
ParseAmpelOutput now unwraps DSSE-signed envelopes before parsing the
ampel result predicate, fixing silent all-fail results when ampel writes
signed attestations.

ToPVPResult now groups findings by CheckID into a single
ObservationByCheck with multiple Subjects (one per repo), matching the
OSCAL pattern and preventing last-write-wins overwrites in the
downstream oscal-sdk-go observation manager.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@marcusburghardt @claude
docs(plugin): Add ampel-plugin README with setup and usage instructions
79ca4d7 
Covers plugin structure, configuration, target file format, AMPEL
policies, generate/scan workflow, installation of snappy and ampel via
go install, GITHUB_TOKEN requirement, plugin registration, and
complytime-demos Fedora 43 VM setup.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@marcusburghardt @claude
fix(plugin): Update server test for CheckID-grouped observations
bbd6de4 
Align TestGetResults_MultipleSpecs with the ToPVPResult grouping change:
same CheckID from multiple specs now produces 1 observation with 2
subjects instead of 2 separate observations.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@marcusburghardt
chore: fix lint issues
ca5f65b 
Signed-off-by: Marcus Burghardt <maburgha@redhat.com>
@marcusburghardt @claude
docs: Update speckit files to reflect implementation changes
502a229 
Align spec, plan, tasks, data model, quickstart, research, and
contracts with the actual implementation: granular policy matching
(replacing CEL generation), per-repo spec configuration with
builtin: prefix, DSSE envelope handling, CheckID-grouped
observations for multi-target scanning, GITHUB_TOKEN requirement,
and go install for snappy/ampel. Add strategy document.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@marcusburghardt @claude
refactor(plugin): Extract DSSE handling into shared intoto package
f1ebbf8 
Move the duplicated dsseEnvelope type and DSSE unwrapping logic
from results/results.go and scan/scan.go into a new intoto
package with an exported UnwrapDSSE function. Both consumers now
call intoto.UnwrapDSSE instead of duplicating the logic.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@marcusburghardt @claude
refactor(plugin): Consolidate duplicated URL helpers into targets pac…
cb51213 
…kage

Move parseRepoURL, sanitizeRepoName, sanitizeForFilename, and
repoDisplayName into the targets package as exported functions
(ParseRepoURL, SanitizeRepoURL, RepoDisplayName) to eliminate
duplication across scan and results packages.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gvauter gvauter gvauter requested changes

@jpower432 jpower432 Awaiting requested review from jpower432

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@marcusburghardt @gvauter