Skip to content

feat(ci): add Aviator remediation action with automated PR creation (SSC & FoD)#1037

Open
SangameshV wants to merge 46 commits into
dev/v3.xfrom
feat/v3.x/create-pr-fcli-action
Open

feat(ci): add Aviator remediation action with automated PR creation (SSC & FoD)#1037
SangameshV wants to merge 46 commits into
dev/v3.xfrom
feat/v3.x/create-pr-fcli-action

Conversation

@SangameshV

Copy link
Copy Markdown
Contributor

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:

  • Apply fixes to the repository
  • Commit updates
  • Push changes to a new branch
  • Create a pull request for review

Changes

✅ New Feature: Automated Remediation Action

  • Implemented FCLI action to:
    • Apply Aviator-generated remediations
    • Stage and commit updated files
    • Create a remediation branch
    • Push the branch to the remote repository
    • Open a pull request against the base branch

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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/FoD ci.yaml pipelines.
  • 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();

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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.
image

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Comment on lines +434 to +438
// GitLab CI
token = EnvHelper.env("CI_JOB_TOKEN");
if (StringUtils.isNotBlank(token)) {
return new UsernamePasswordCredentialsProvider("gitlab-ci-token", token);
}
Comment on lines +279 to +284
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'}"
Comment on lines +102 to +106
- var.set:
githubToken: "${#env('GITHUB_TOKEN')?:#env('GH_TOKEN')}"
gitlabToken: "${#env('CI_JOB_TOKEN')}"
azureToken: "${#env('SYSTEM_ACCESSTOKEN')}"
bitbucketToken: "${#env('BITBUCKET_TOKEN')}"
Comment on lines +102 to +106
- 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'}"
Comment on lines +14 to +18
- `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]")
Comment on lines +14 to +18
- `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 rsenden left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.yaml and create-pr.yaml actions
  • Functionality from create-pr.yaml would move to Java code, exposed through SpEL function(s)
  • Running apply-remediations and creating the PR/MR consolidated in a single ci.yaml step

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 in ci.yaml. Maybe just add a raisePR(...) method on ActionCiSpelFunctions (or have a separate SpelFunctions class that handles cross-CI PR logic), which calls detect() 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.yaml files 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 {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For consistency with other Aviator-related steps, probably better to rename steps and env vars to something like AVIATOR_REMEDIATE.



on.success:
- var.set:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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?

@SangameshV SangameshV requested a review from rsenden June 25, 2026 18:10

@rsenden rsenden left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.
  • ci action 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 git invocations from CI pipelines. Also, looks like quite a lot of code is needed to accomplish tasks that would only require simple a git command if we'd do this from command line.

Please research feasibility of the following:

  • Only have a single AVIATOR_REMEDIATE step in ci.yaml, without separate steps for pushing changes and raising PR. Some potential ideas:
    • Have a single raise-remediation-pr action in fcli-action that 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-pr actions; 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-mr and github-remediations-pr; either single copy in fcli-action or two copies in fcli-fod and fcli-ssc. Advantages: can use CI-specific terminology (MR vs PR), users and detect-env can easily check availability for current CI system (and maybe fall back to a generic push-remediations action 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
  • Use ProcessBuilder for running plain git 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; git is 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 git is 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/or run.git YAML instruction, somewhat similar to run.fcli (much more flexible)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants