feat(ci): add Aviator remediation action with automated PR creation (SSC & FoD)#1037
feat(ci): add Aviator remediation action with automated PR creation (SSC & FoD)#1037SangameshV wants to merge 46 commits into
Conversation
…ed the same from generic actions
…add debug logging
There was a problem hiding this comment.
Pull request overview
This PR adds an automated Aviator remediation workflow to the built-in SSC and FoD CI action zips, including applying remediations, committing changes, pushing a branch, and creating a PR/MR through the detected CI provider (GitHub/GitLab).
Changes:
- Bump action schema version to 2.9.0.
- Add new SSC/FoD actions (
apply-remediations,create-pr) and wire them into the existing SSC/FoDci.yamlpipelines. - Extend CI helper APIs (GitHub/GitLab) and add new
#git.*SpEL functions (JGit-backed) to support git status/branch/commit/push operations from actions.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| gradle.properties | Bumps action schema version to 2.9.0. |
| fcli-core/fcli-ssc/src/main/resources/com/fortify/cli/ssc/actions/zip/ci.yaml | Adds remediation + PR creation steps to SSC pipeline and fixes Aviator wait argument wiring. |
| fcli-core/fcli-ssc/src/main/resources/com/fortify/cli/ssc/actions/zip/apply-remediations.yaml | New SSC action that applies Aviator remediations to the local source tree. |
| fcli-core/fcli-ssc/src/main/resources/com/fortify/cli/ssc/actions/zip/create-pr.yaml | New SSC action that commits/pushes changes and creates a PR/MR. |
| fcli-core/fcli-fod/src/main/resources/com/fortify/cli/fod/actions/zip/ci.yaml | Adds remediation + PR creation steps to FoD pipeline. |
| fcli-core/fcli-fod/src/main/resources/com/fortify/cli/fod/actions/zip/apply-remediations.yaml | New FoD action that applies Aviator remediations to the local source tree. |
| fcli-core/fcli-fod/src/main/resources/com/fortify/cli/fod/actions/zip/create-pr.yaml | New FoD action that commits/pushes changes and creates a PR/MR. |
| fcli-core/fcli-common-ci/src/main/java/com/fortify/cli/common/ci/gitlab/GitLabProject.java | Adds GitLab merge request creation API call. |
| fcli-core/fcli-common-ci/src/main/java/com/fortify/cli/common/ci/github/GitHubRepo.java | Adds GitHub pull request creation API call. |
| fcli-core/fcli-common-action/src/main/java/com/fortify/cli/common/action/helper/ci/gitlab/ActionGitLabProject.java | Exposes GitLab MR creation as a SpEL function for actions. |
| fcli-core/fcli-common-action/src/main/java/com/fortify/cli/common/action/helper/ci/github/ActionGitHubRepo.java | Exposes GitHub PR creation as a SpEL function for actions. |
| fcli-core/fcli-common-action/src/main/java/com/fortify/cli/common/action/runner/ActionRunnerContextLocal.java | Registers new git SpEL variable for action execution. |
| fcli-core/fcli-common-action/src/main/java/com/fortify/cli/common/action/helper/git/ActionGitSpelFunctions.java | New JGit-backed SpEL functions for repo inspection + git operations used by create-pr. |
| fcli-core/fcli-app/src/main/resources/META-INF/native-image/fcli/fcli-app/jgit/reflect-config.json | Updates native-image reflection configuration for JGit/LFS-related classes. |
| if (git == null) { | ||
| throw new FcliSimpleException("Not a git repository: " + sourceDir); | ||
| } | ||
| git.add().setUpdate(true).addFilepattern(".").call(); |
There was a problem hiding this comment.
The current implementation of addAll() using setUpdate(true) is intentional. It ensures that only modified or deleted files are staged, while excluding newly created files. This was done to prevent unrelated files generated during CI action runs from being accidentally committed.
This behavior was explicitly introduced as part of commit 9c7a5bc to limit commits only to actual remediation changes.
Refer to the below screenshot showing previously committed unwanted files.

There was a problem hiding this comment.
Although I think current Aviator remediation only changes existing files, what if a future version can also create new files, for example adding some validator class that's than called from updated code. If we don't include new files, the PR would contain broken code.
In my manual comments, I added a suggestion to use git stash/unstash, although given your screenshot, that might not work well either; no idea what would happen if you stash/unstash the fcli.log file that current fcli invocation is actively writing to.
I don't mind keeping it simple for now with just including changed files, but we should at least add a comment explaining this; why we don't add new files (build output that shouldn't be committed, fcli state data, ...), and how this can causes issues if future Aviator versions do create new files.
There was a problem hiding this comment.
Adding fcli.log to .gitignore makes sense and avoids committing that file. However, using git stash/unstash in CI introduces complexity and potential inconsistencies, especially with untracked files and conflict handling.
For now, it’s safer to keep the current approach of committing only modified files and explicitly document this behavior. We can revisit this if future Aviator versions require inclusion of newly created files, potentially with a controlled/whitelisted inclusion strategy instead of relying on stash.
There was a problem hiding this comment.
Although I understand the use case, the method name addAll doesn't properly reflect actual functionality (which only adds changed files, doesn't add new files). At least method name should be updated to reflect actual behavior, or possibly do some refactoring to make this configurable by the caller.
| // GitLab CI | ||
| token = EnvHelper.env("CI_JOB_TOKEN"); | ||
| if (StringUtils.isNotBlank(token)) { | ||
| return new UsernamePasswordCredentialsProvider("gitlab-ci-token", token); | ||
| } |
| var remoteUrl = repo.getConfig().getString("remote", remote, "url"); | ||
| if (remoteUrl != null && !remoteUrl.endsWith(".git")) { | ||
| remoteUrl = remoteUrl + ".git"; | ||
| repo.getConfig().setString("remote", remote, "url", remoteUrl); | ||
| repo.getConfig().save(); | ||
| } |
| - if: ${#isBlank(gitRepoInfo.repository.remoteUrl)} | ||
| throw: "Git repository has no remote URL configured. Cannot create PR without remote repository access." | ||
| - if: ${gitRepoInfo!=null} | ||
| log.info: "Git repository: workspace=${gitRepoInfo.repository.workspaceDir}, remote=${gitRepoInfo.repository.remoteUrl?:'not configured'}, branch=${gitRepoInfo.branch.short_?:'detached HEAD'}" |
| - var.set: | ||
| githubToken: "${#env('GITHUB_TOKEN')?:#env('GH_TOKEN')}" | ||
| gitlabToken: "${#env('CI_JOB_TOKEN')}" | ||
| azureToken: "${#env('SYSTEM_ACCESSTOKEN')}" | ||
| bitbucketToken: "${#env('BITBUCKET_TOKEN')}" |
| - var.set: | ||
| githubToken: "${#env('GITHUB_TOKEN')?:#env('GH_TOKEN')}" | ||
| gitlabToken: "${#env('CI_JOB_TOKEN')}" | ||
| azureToken: "${#env('SYSTEM_ACCESSTOKEN')}" | ||
| bitbucketToken: "${#env('BITBUCKET_TOKEN')}" |
|
|
||
| - var.set: | ||
| sourceDir: ${global.ci.sourceDir?:'.'} | ||
| artifactSelector: "${#isNotBlank(cli.aviatorArtifact) ? '--artifact-id '+cli.aviatorArtifact : '--av '+av+' --latest'}" |
| - `SOURCE_DIR` — Source directory where changes are detected (default: CI workspace or current directory) | ||
| - `PR_TITLE` — PR/MR title (default: "fix: Fortify auto-remediation fixes [generated by fcli]") | ||
| - `PR_BODY` — PR/MR body (default: auto-generated description) | ||
| - `PR_BRANCH_PREFIX` — Branch name prefix (default: "fcli/remediation") | ||
| - `PR_COMMIT_MESSAGE` — Commit message (default: "fix: apply Fortify auto-remediation fixes [generated by fcli]") |
| - `SOURCE_DIR` — Source directory where changes are detected (default: CI workspace or current directory) | ||
| - `PR_TITLE` — PR/MR title (default: "fix: Fortify auto-remediation fixes [generated by fcli]") | ||
| - `PR_BODY` — PR/MR body (default: auto-generated description) | ||
| - `PR_BRANCH_PREFIX` — Branch name prefix (default: "fcli/remediation") | ||
| - `PR_COMMIT_MESSAGE` — Commit message (default: "fix: apply Fortify auto-remediation fixes [generated by fcli]") |
rsenden
left a comment
There was a problem hiding this comment.
Note that some of the comments posted on FoD files may also apply to SSC, or vice versa.
Other than individual comments, I'm wondering whether we can improve architecture to be more logical and more user-friendly, which would supersede most of the individual comments.
Please investigate whether something like the following in the ci.yaml files would work:
AVIATOR_REMEDIATE:
cmd: ${#fcliCmd(<SSC/FoD apply-remediations cmd>)}
skip.if-reason:
<same as before, possibly enhanced for FoD to check remediations are available>
on.success:
- var.set:
createPrResult: ${#<CI-specific SpEL function>(subject, body, ...)}
- log.info: Aviator remediations ${global.ci.prTerminology} created: ${createPrResult.prLink}
I.e.:
- We'd get rid of
apply-remedations.yamlandcreate-pr.yamlactions - Functionality from
create-pr.yamlwould move to Java code, exposed through SpEL function(s) - Running apply-remediations and creating the PR/MR consolidated in a single
ci.yamlstep
Main questions:
- How to easily call a CI-specific SpEL function, like
#github.repo().raisePR(...)vs#gitlab.project().raiseMR(...)in a generic way, without hardcoding knowledge about supported CI systems and what functions they expose, directly inci.yaml. Maybe just add araisePR(...)method onActionCiSpelFunctions(or have a separate SpelFunctions class that handles cross-CI PR logic), which callsdetect()and then calls the appropriate CI-specific SpEL function on the detected CI system? - How to handle add/commit/push? Probably easiest to do this from Java code, such that SSC/FoD
ci.yamlfiles just need to call a single SpEL function
Also something to consider; what if the build that was executed by scancentral package caused any changes in current git state (like adding generated code in non-ignored paths). We'd want the PR to only include code that was added/removed/updated by the apply-remediations command, so possibly we should stash any changes before running apply-remediations, and unstash after pushing the Aviator changes/raising the PR?
| @Reflectable | ||
| @SpelFunctionPrefix("git.") | ||
| @Slf4j | ||
| public class ActionGitSpelFunctions { |
There was a problem hiding this comment.
Some of the methods in this class are very long; please refactor using private helper methods. Also, please make sure that proper imports are used instead of fully qualified class names.
| } | ||
| } | ||
|
|
||
| private CredentialsProvider detectCredentialsProvider() { |
There was a problem hiding this comment.
This tightly binds generic Git functionality to individual CI systems. When we add support for a CI system, we'd expect having to create corresponding CI-specific classes, but it may be easily overlooked that this method needs to be updated as well.
Can we refactor this, for example by having some getGitCredentialsProvider method in each of the CI implementations, and then either have the fcli action yaml explicitly retrieve credentials from currently active CI environment and pass this to ActionGitSpelFunctions methods, or have ActionGitSpelFunctions somehow retrieve this from the currently active CI environment implementation?
Please check what architecture would fit best here, for example in which of the CI-specific classes the getGitCredentialsProvider method should live (if we choose to have such a method), and how ActionGitSpelFunctions can gain access to that.
Also, what if for example workflow is running in ADO, but repo is on GitHub? I guess in that case, SYSTEM_ACCESSTOKEN wouldn't give access to the GitHub repo, so would be good to do some research as to how other (non-Fortify) pipeline integrations handle such scenarios. For now, we can consider this an edge case, but it might affect optimal architecture for the above, so good to have a look at this.
| on.success: | ||
| - var.set: { postScan.skipReason: } # Reset postScan.skipReason to allow post-scan tasks to run | ||
|
|
||
| APPLY_REMEDIATIONS: |
There was a problem hiding this comment.
For consistency with other Aviator-related steps, probably better to rename steps and env vars to something like AVIATOR_REMEDIATE.
|
|
||
|
|
||
| on.success: | ||
| - var.set: |
There was a problem hiding this comment.
Why do we need to explicitly retrieve artifact id from the output, rather than just relying on fcli --store option and using ::aviator_audit:: (like we did before) instead of ${aviator.artifactId}
| skip.if-reason: | ||
| - ${#actionCmdSkipFromEnvReason('APPLY_REMEDIATIONS', true)} # Skip unless DO_APPLY_REMEDIATIONS==true or APPLY_REMEDIATIONS_ACTION/EXTRA_OPTS defined | ||
| - ${postScan.skipReason} # Skip if no scans were run | ||
| - ${#env('DO_AVIATOR_AUDIT')!='true'?'Aviator audit not enabled (DO_AVIATOR_AUDIT!=true), no remediations available':''} # Skip if Aviator audit was not enabled |
There was a problem hiding this comment.
Any more reliable way to check whether Aviator Audit was run? Maybe check FoD whether Aviator audit was run, or, even better, check whether remediations are available?
…ing to print the read results of called fcli action
…changes and create-pr actions.
…ctive results storing
There was a problem hiding this comment.
To start with, note that there have been some general changes/improvements to CI-related environment classes/SpEL functions in dev/v3.x, which might (or might not) be useful for improving the code in this PR.
I have some doubts about current implementation:
- Looks like most of the earlier review comments haven't yet been addressed, like long methods, names of steps in
ci.yaml, CI token handling, proper separation between generic git-related code and CI-specific credential providers, ... Note though that implementation might change based on the below, so best to handle those first. ciaction usually calls a single sub-action/fcli command to perform a specific task, but for this use case, it now needs to call three separate cmds/actions; first apply-remediations, then git-push-changes, then create-pr.- Apparently, JGit doesn't natively integrate with CI-provided credentials, and according to AI, there are some other edge cases that require special attention in JGit compared to plain
gitinvocations from CI pipelines. Also, looks like quite a lot of code is needed to accomplish tasks that would only require simple agitcommand if we'd do this from command line.
Please research feasibility of the following:
- Only have a single
AVIATOR_REMEDIATEstep inci.yaml, without separate steps for pushing changes and raising PR. Some potential ideas:- Have a single
raise-remediation-praction infcli-actionthat runs either SSC or FoD apply-remediations command (select based on CLI arg), commits/pushes the changes, then raises PR (on supported CI systems) - Have two
raise-remediation-practions; one in fcli-fod, one in fcli-ssc; same as above, but automatically calling the appropriate apply-remediations cmd - Have CI-specific actions like
gitlab-remediations-mrandgithub-remediations-pr; either single copy infcli-actionor two copies infcli-fodandfcli-ssc. Advantages: can use CI-specific terminology (MR vs PR), users anddetect-envcan easily check availability for current CI system (and maybe fall back to a genericpush-remediationsaction for CI systems on which we don't support raising PR yet), most in line with current action structure - If needed, to reduce duplication across multiple actions, have a special-purpose SpEL function that handles add+commit+push specifically for this use case
- Have a single
- Use
ProcessBuilderfor running plaingit add/commit/push commands, instead of doing everything through JGit- Would likely greatly reduce amount of code in this PR; no need for remote/branch detection, no need for credentials handling, ...
- Likely results in better CI integration / better user experience;
gitis usually properly configured for the checked out repo, whereas we'd need to manually handle this (including edge cases) with JGit. - In general, invoking external processes should be avoided, but warranted in this case given likely better CI integration, code simplification, and we can assume that
gitis available on CI systems & dev workstations - We can either provide wrapper SpEL functions (like
add,commit, ...) for specific git operations (somewhat more secure, not exposing dangerous git cmds), or generic#git.run(<args>)SpEL function and/orrun.gitYAML instruction, somewhat similar torun.fcli(much more flexible)
Summary
Introduces a new FCLI action to automatically apply Aviator remediation fixes and create a pull request with the generated changes.
This implementation supports both SSC (Software Security Center) and FoD (Fortify on Demand) pipelines.
Background
Aviator generates remediation changes for identified issues, but there was no automated way to:
Changes
✅ New Feature: Automated Remediation Action