Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: checkstyle configuration and suggested updates #7003

Merged
merged 6 commits into from
Oct 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 60 additions & 30 deletions .github/workflows/pull_requests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ jobs:
test:
name: Build and Test
permissions:
security-events: write
contents: read
runs-on: ubuntu-latest
steps:
Expand Down Expand Up @@ -43,10 +44,31 @@ jobs:
with:
# Command to be sent to SARIF Multitool
command: 'validate core/target/test-reports/Report.sarif'
- name: Upload SARIF file
uses: github/codeql-action/upload-sarif@v3
with:
sarif_file: utils/target/spotbugsSarif.json
category: spotbugs-utils
- name: Upload SARIF file
uses: github/codeql-action/upload-sarif@v3
with:
sarif_file: cli/target/spotbugsSarif.json
category: spotbugs-cli
- name: Upload SARIF file
uses: github/codeql-action/upload-sarif@v3
with:
sarif_file: ant/target/spotbugsSarif.json
category: spotbugs-ant
- name: Upload SARIF file
uses: github/codeql-action/upload-sarif@v3
with:
sarif_file: core/target/spotbugsSarif.json
category: spotbugs-core

maven:
name: Regression Test Maven Plugin
permissions:
security-events: write
contents: read
runs-on: ubuntu-latest
steps:
Expand Down Expand Up @@ -76,7 +98,7 @@ jobs:
env:
NVD_API_KEY: ${{ secrets.NVD_API_KEY }}
run: |
mvn -V -s settings.xml -pl utils,core,maven -am compile verify -DtestMavenPlugin -DreleaseTesting --no-transfer-progress --batch-mode
mvn -V -s settings.xml -pl maven -am compile verify -DtestMavenPlugin -DreleaseTesting --no-transfer-progress --batch-mode
- name: Archive IT test logs
id: archive-logs
if: always()
Expand All @@ -85,32 +107,40 @@ jobs:
name: it-test-logs
retention-days: 7
path: maven/target/it/**/build.log
# this action has been failing - so I'm disabling it; we never really found anything anyways
# audit:
# runs-on: ubuntu-latest
# permissions:
# contents: read
# pull-requests: write
# name: Audit
# steps:
# - uses: actions/checkout@v4
# - name: Check Maven Cache
# id: maven-cache
# uses: actions/cache@v4
# with:
# path: ~/.m2/repository/
# key: ${{ runner.os }}-maven-${{ hashFiles('**/pom.xml') }}
# restore-keys: |
# ${{ runner.os }}-maven-
# - name: Semgrep
# id: semgrep
# run: |
# docker run --rm -v "${PWD}:/src" returntocorp/semgrep semgrep --config "p/ci" --sarif > semgrep.sarif
# - name: Maven Site
# if: always()
# run: |
# mvn -s settings.xml package site -DskipTests=true --no-transfer-progress --batch-mode
# - name: Publish Comments
# if: always()
# run: |
# mvn se.bjurr.violations:violation-comments-to-github-maven-plugin:violation-comments --no-transfer-progress --batch-mode -DpullRequestId=${{ github.event.pull_request.number }} -DoAuth2Token=${{ secrets.GITHUB_TOKEN }}
- name: Upload SARIF file
uses: github/codeql-action/upload-sarif@v3
with:
sarif_file: maven/target/spotbugsSarif.json
category: spotbugs-maven

checkstyle:
name: Checkstyle Validation
permissions:
security-events: write
contents: read
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- name: Check Maven Cache
id: maven-cache
uses: actions/cache@v4
with:
path: ~/.m2/repository/
key: ${{ runner.os }}-maven-${{ hashFiles('**/pom.xml') }}
restore-keys: |
${{ runner.os }}-maven-
- name: Set up JDK 11
id: jdk-11
uses: actions/setup-java@v4
with:
java-version: 11
distribution: 'zulu'
- name: Checkstyle
id: checkstyle
run: |
mvn -V -s settings.xml checkstyle:checkstyle-aggregate --no-transfer-progress --batch-mode
- name: Upload SARIF file
uses: github/codeql-action/upload-sarif@v3
with:
sarif_file: target/checkstyle-result.sarif
category: checkstyle
9 changes: 5 additions & 4 deletions cli/src/main/java/org/owasp/dependencycheck/CliParser.java
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ private void addAdvancedOptions(final Options options) {
"Credentials for basic authentication to the NVD API Datafeed."))
.addOption(newOptionWithArg(ARGUMENT.NVD_API_DATAFEED_PASSWORD, "password",
"Credentials for basic authentication to the NVD API Datafeed."))
.addOption(newOptionWithArg(ARGUMENT.NVD_API_MAX_RETRY_COUNT,"count",
.addOption(newOptionWithArg(ARGUMENT.NVD_API_MAX_RETRY_COUNT, "count",
"The maximum number of retry requests for a single call to the NVD API."))
.addOption(newOptionWithArg(ARGUMENT.NVD_API_VALID_FOR_HOURS, "hours",
"The number of hours to wait before checking for new updates from the NVD."))
Expand Down Expand Up @@ -1144,16 +1144,17 @@ public static class ARGUMENT {
*/
public static final String DATA_DIRECTORY = "data";
/**
* The CLI argument name for setting the URL for the NVD API Endpoint
* The CLI argument name for setting the URL for the NVD API Endpoint.
*/
public static final String NVD_API_ENDPOINT = "nvdApiEndpoint";
/**
* The CLI argument name for setting the URL for the NVD API Key.
*/
public static final String NVD_API_KEY = "nvdApiKey";
/**
* The CLI argument name for setting the maximum number of retry requests for a single call to the NVD API.
*/
* The CLI argument name for setting the maximum number of retry
* requests for a single call to the NVD API.
*/
public static final String NVD_API_MAX_RETRY_COUNT = "nvdMaxRetryCount";
/**
* The CLI argument name for setting the number of hours to wait before
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -346,12 +346,14 @@ private List<SuppressionRule> loadSuppressionFile(final SuppressionParser parser
file = getSettings().getTempFile("suppression", "xml");
final URL url = new URL(suppressionFilePath);
try {
Downloader.getInstance().fetchFile(url, file, false, Settings.KEYS.SUPPRESSION_FILE_USER, Settings.KEYS.SUPPRESSION_FILE_PASSWORD);
Downloader.getInstance().fetchFile(url, file, false, Settings.KEYS.SUPPRESSION_FILE_USER,
Settings.KEYS.SUPPRESSION_FILE_PASSWORD);
} catch (DownloadFailedException ex) {
LOGGER.trace("Failed download suppression file - first attempt", ex);
try {
Thread.sleep(500);
Downloader.getInstance().fetchFile(url, file, true, Settings.KEYS.SUPPRESSION_FILE_USER, Settings.KEYS.SUPPRESSION_FILE_PASSWORD);
Downloader.getInstance().fetchFile(url, file, true, Settings.KEYS.SUPPRESSION_FILE_USER,
Settings.KEYS.SUPPRESSION_FILE_PASSWORD);
} catch (TooManyRequestsException ex1) {
throw new SuppressionParseException("Unable to download supression file `" + file
+ "`; received 429 - too many requests", ex1);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,15 +86,16 @@ public class CarthageAnalyzer extends AbstractFileTypeAnalyzer {
private static final FileFilter CARTHAGE_FILTER = FileFilterBuilder.newInstance().addFilenames(CARTFILE_RESOLVED).build();

/**
* The capture group #1 is the dependency type, #2 is the name, #3 is dependency version.
* The version can be a commit ref, so we can't assume it's a number
* The capture group #1 is the dependency type, #2 is the name, #3 is
* dependency version. The version can be a commit ref, so we can't assume
* it's a number
*
* Example values:
* - binary "https://dl.google.com/geosdk/GoogleMaps.json" "7.2.0"
* - git "https://gitlab.matrix.org/matrix-org/olm.git" "3.2.16"
* - github "alinradut/SwiftEntryKit" "95f4a08f41ddcf2c02e2b22789038774c8c94df5""
* - github "CocoaLumberjack/CocoaLumberjack" "3.8.5"
* - github "realm/realm-swift" "v10.44.0"
* Example values: - binary "https://dl.google.com/geosdk/GoogleMaps.json"
* "7.2.0" - git "https://gitlab.matrix.org/matrix-org/olm.git" "3.2.16" -
* github "alinradut/SwiftEntryKit"
* "95f4a08f41ddcf2c02e2b22789038774c8c94df5"" - github
* "CocoaLumberjack/CocoaLumberjack" "3.8.5" - github "realm/realm-swift"
* "v10.44.0"
Comment on lines -92 to +98
Copy link
Collaborator

Choose a reason for hiding this comment

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

@jeremylong Might be good to still post-merge review and correct occurrences like this in the delta that transformed 'nicely coded, but horribly rendered by Javadoc' to 'horribly coded, and horribly rendered by Javadoc' javadoc comments.
They should've been converted to proper HTML <ul> + <li> to be rendered as intended by the original written code rather than reformatted to have the bulleted items all rendered as 'standard javadoc text paragraph'

Copy link
Owner Author

Choose a reason for hiding this comment

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

I love autoformatting sometimes... Thanks for pointing this out.

*/
private static final Pattern CARTFILE_RESOLVED_DEPENDENCY_PATTERN = Pattern.compile("(github|git|binary) \"([^\"]+)\" \"([^\"]+)\"");

Expand All @@ -106,9 +107,8 @@ public class CarthageAnalyzer extends AbstractFileTypeAnalyzer {
/**
* Capture group #1 is the dependency name.
*
* Example values:
* - robbiehanson/XMPPFramework
* - CocoaLumberjack/CocoaLumberjack
* Example values: - robbiehanson/XMPPFramework -
* CocoaLumberjack/CocoaLumberjack
*/
private static final Pattern CARTFILE_RESOLVED_GITHUB_DEPENDENCY = Pattern.compile("[a-zA-Z0-9-_]+/([a-zA-Z0-9\\-_\\.]+)");

Expand All @@ -120,11 +120,9 @@ public class CarthageAnalyzer extends AbstractFileTypeAnalyzer {
/**
* Capture group #1 is the dependency name.
*
* Example values:
* - https://my.domain.com/release/MyFramework.json
* - file:///some/Path/MyFramework.json
* - relative/path/MyFramework.json
* - /absolute/path/MyFramework.json
* Example values: - https://my.domain.com/release/MyFramework.json -
* file:///some/Path/MyFramework.json - relative/path/MyFramework.json -
* /absolute/path/MyFramework.json
*/
private static final Pattern CARTFILE_RESOLVED_BINARY_DEPENDENCY = Pattern.compile("([a-zA-Z0-9\\-_\\.]+).json");

Expand Down Expand Up @@ -186,6 +184,7 @@ protected void analyzeDependency(Dependency dependency, Engine engine)
* Analyzes the Cartfile.resolved and adds the evidence to the dependency.
*
* @param cartfileResolved the dependency
* @param engine a reference to the dependency-check engine
* @throws AnalysisException thrown if there is an error analyzing the
* Cartfile
*/
Expand All @@ -210,8 +209,7 @@ private void analyzeCartfileResolvedDependency(Dependency cartfileResolved, Engi
final Matcher versionMatcher = CARTFILE_VERSION_PATTERN.matcher(version);
if (versionMatcher.find()) {
version = versionMatcher.group(1);
}
else {
} else {
// this is probably a git commit reference, so we'll default to 0.0.0.
// this will probably bubble up a ton of CVEs, but serves you right for
// not using semantic versioning.
Expand All @@ -224,15 +222,13 @@ private void analyzeCartfileResolvedDependency(Dependency cartfileResolved, Engi
continue;
}
name = nameMatcher.group(1);
}
else if (type.contentEquals("github")) {
} else if (type.contentEquals("github")) {
final Matcher nameMatcher = CARTFILE_RESOLVED_GITHUB_DEPENDENCY.matcher(name);
if (!nameMatcher.find()) {
continue;
}
name = nameMatcher.group(1);
}
else if (type.contentEquals("binary")) {
} else if (type.contentEquals("binary")) {
final Matcher nameMatcher = CARTFILE_RESOLVED_BINARY_DEPENDENCY.matcher(name);
if (!nameMatcher.find()) {
continue;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,11 +123,11 @@ protected void analyzeDependency(Dependency dependency, Engine engine) throws An
final DependencyTree tree;
List<MavenDependency> deps;
try {
JsonNode jsonNode = MAPPER.readTree(dependencyFile);
JsonNode v2Version = jsonNode.path("version");
JsonNode v010Version = jsonNode.path("dependency_tree").path("version");
final JsonNode jsonNode = MAPPER.readTree(dependencyFile);
final JsonNode v2Version = jsonNode.path("version");
final JsonNode v010Version = jsonNode.path("dependency_tree").path("version");

if (v2Version.isTextual()) {
if (v2Version.isTextual()) {
final InstallFileV2 installFile = INSTALL_FILE_V2_READER.readValue(dependencyFile);
if (!Objects.equals(installFile.getAutogeneratedSentinel(), "THERE_IS_NO_DATA_ONLY_ZUUL")) {
return;
Expand Down Expand Up @@ -155,7 +155,6 @@ protected void analyzeDependency(Dependency dependency, Engine engine) throws An
return;
}


} catch (IOException e) {
System.out.println("e");
return;
Expand Down Expand Up @@ -325,11 +324,12 @@ public String getVersion() {
* {@code .dependency_tree.dependencies}.
*/
private static class MavenDependency {
public MavenDependency(String coord) {

MavenDependency(String coord) {
this.coord = coord;
}

public MavenDependency() {
MavenDependency() {
}
/**
* The standard Maven coordinate string
Expand All @@ -352,7 +352,13 @@ public String getCoord() {
* A reusable reader for {@link InstallFile}.
*/
private static final ObjectReader INSTALL_FILE_READER;
/**
* A reusable reader for {@link InstallFileV2}.
*/
private static final ObjectReader INSTALL_FILE_V2_READER;
/**
* A reusable object mapper.
*/
private static final ObjectMapper MAPPER;

static {
Expand All @@ -367,11 +373,12 @@ public String getCoord() {
* file.
*
* <p>
* At the time of writing, the latest version is 2, and the dependencies
* are stored in {@code .artifacts}.
* At the time of writing, the latest version is 2, and the dependencies are
* stored in {@code .artifacts}.
*
* <p>
* The top-level keys we care about are {@code .artifacts}. {@code .version}.
* The top-level keys we care about are {@code .artifacts}.
* {@code .version}.
*/
private static class InstallFileV2 {

Expand All @@ -382,8 +389,9 @@ private static class InstallFileV2 {
private String version;

/**
* A list of Maven dependencies made available. Note that this map is transitively closed and
* pinned to a specific version of each artifact.
* A list of Maven dependencies made available. Note that this map is
* transitively closed and pinned to a specific version of each
* artifact.
* <p>
* The key is the Maven coordinate string, less the version
* {@code group:artifact[:optional classifier][:optional packaging]}.
Expand All @@ -394,8 +402,8 @@ private static class InstallFileV2 {
private Map<String, Artifactv2> artifacts;

/**
* A sentinel value placed in the file to indicate that it is an auto-generated pinned maven
* install file.
* A sentinel value placed in the file to indicate that it is an
* auto-generated pinned maven install file.
*/
@JsonProperty("__AUTOGENERATED_FILE_DO_NOT_MODIFY_THIS_FILE_MANUALLY")
private String autogeneratedSentinel;
Expand Down Expand Up @@ -427,23 +435,23 @@ public String getAutogeneratedSentinel() {
return autogeneratedSentinel;
}
}

private static class Artifactv2 {

/**
* The version of the artifact.
*/
@JsonProperty("version")
private String version;

/**
* Returns the value of version.
*
* @return the value of version
*/
public String getVersion() {
return version;
}
}
/**
* The version of the artifact.
*/
@JsonProperty("version")
private String version;

/**
* Returns the value of version.
*
* @return the value of version
*/
public String getVersion() {
return version;
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ private List<Advisory> analyzePackage(final File lockFile, final File packageFil
final JsonObject lockJson = fetchYarnAuditJson(dependency, skipDevDependencies);
// Retrieves the contents of package-lock.json from the Dependency
final JsonObject packageJson;
try (final JsonReader packageReader = Json.createReader(Files.newInputStream(packageFile.toPath()))) {
try (JsonReader packageReader = Json.createReader(Files.newInputStream(packageFile.toPath()))) {
packageJson = packageReader.readObject();
}
// Modify the payload to meet the NPM Audit API requirements
Expand Down
Loading
Loading