fix: validate RepositoryVulnerabilityAlert to getVulnerability

[sugarshin]

Hi, there. This pull-request is related to #11911 and solves it.

Now, I facing this problem too. The problem of #11911 is caused by the Renovate does not null-check for it. As GitHub documentation, securityVulnerability is nullable. but, I think fixing it to be greatly affected, such as proper TypeScript type definitions.

And, I think this line of logging with error and exiting with non-zero status code related to the above problem is not appropriate because the try-catch here is a process to parse the vulnerability alerts for just logging. It means not affected to getVulnerabilityAlerts() return values. And is not related to the original execution behavior of the Renovate.

So I think logger.warn is better.

Changes:

Validate for securityVulnerability to skip processing parse RepositoryVulnerabilityAlert entity for logger.

Context:

• Error processing vulnerabity alerts #11911

Documentation (please check one with an [x])

• I have updated the documentation, or
• No documentation update is required

How I've tested my work (please tick one)

I have verified these changes via:

• Code inspection only, or
• Newly added/modified unit tests, or
• No unit tests but ran on a real repository, or
• Both unit tests + ran on a real repository

[CLAassistant]

All committers have signed the CLA.

[JamieMagee]

Can you open an issue to discuss this issue? Using logger.error doesn't make Renovate exit with a non-zero exit code. You can argue that it should be warning because it's a recoverable error, but I don't think that this change alone will solve the issue you're seeing.

[sugarshin]

@JamieMagee Thank you for your reply.

logger.error doesn't make Renovate exit with a non-zero exit code

No, logger.error does make Renovate exit with a non-zero status code. Could you check following?

• renovate/lib/workers/global/index.ts

  Lines 144 to 151 in 8e09d29

  	const loggerErrors = getProblems().filter((p) => p.level >= ERROR);
  	if (loggerErrors.length) {
  	logger.info(
  	{ loggerErrors },
  	'Renovate is exiting with a non-zero code due to the following logged errors'
  	);
  	return 1;
  	}

• renovate/lib/renovate.ts

  Line 16 in 8e09d29

  process.exitCode = await globalWorker.start();

[JamieMagee]

My mistake! Okay, then this may well be the fix. Can you still give me a bit more information about your setup? The original issue you linked was running on GHES and for the npm package hoek. What sort of setup is causing this error for you?

[sugarshin]

@JamieMagee Thank you! I'm using Renovate bot with self-hosting.

The global config is the following:

module.exports = {
  endpoint: 'https://<my-company-ghe-domain>/api/v3/',
  token: process.env.GHE_TOKEN,
  platform: 'github',
  logLevel: 'debug',
  onboardingConfig: {
    extends: ['config:base'],
  },
  cacheDir: __dirname + '/.renovate_cache',
  dryRun: !!process.env.RENOVATE_DRYRUN,
  autodiscover: false,
  packageRules: [
    {
      matchDatasources: ['docker'],
      registryUrls: ['<masked>']
    }
  ],
  repositories: [
    // repositories
  ],
};

The config of the repository where this problem occurs is the following:

{
  "$schema": "https://docs.renovatebot.com/renovate-schema.json",
  "extends": [
    "config:base"
  ],
  "ignorePaths": [
    "**/node_modules/**"
  ],
  "packageRules": [
    {
      "matchDepTypes": ["major"],
      "enabled": false
    },
    {
      "matchDepTypes": ["devDependencies"],
      "addLabels": ["devDependencies"]
    }
  ],
  "labels": ["renovate"]
}

The screenshot of error log:

[sugarshin]

And, I executed with LOG_LEVEL=trace to test. The log content of the following line:

• renovate/lib/platform/github/index.ts

  Line 1707 in 8e09d29

  logger.trace({ alerts }, 'GitHub vulnerability details');

{
    "name": "renovate",
    "hostname": "<masked>",
    "pid": 709,
    "level": 10,
    "logContext": "MjvOWnPA6xLTJre-KECJU",
    "repository": "<masked>",
    "alerts": [
        {
            "dismissReason": null,
            "vulnerableManifestFilename": "package-lock.json",
            "vulnerableManifestPath": "package-lock.json",
            "vulnerableRequirements": "= 2.2.1",
            "securityAdvisory": null,
            "securityVulnerability": null
        },
        ...
    ],
    "msg": "GitHub vulnerability details",
    "time": "2022-01-24T08:10:32.377Z",
    "v": 0
}

As you can see, the "alerts" includes items that "securityVulnerability" is null.

[rarkins]

Seems like a faulty alert?

I'd prefer to fix the validation (eg so that we can still process the non faulty ones) than simply lower the log severity

[sugarshin]

@rarkins Thank you for your comment.

I'd prefer to fix the validation (eg so that we can still process the non faulty ones)

Ok, I agree with you. I will fix it later.

[JamieMagee]

@sugarshin Can you give me the package name for the malformed vulnerability alert? Maybe I can follow-up internally about it.

[sugarshin]

@JamieMagee Thank you. I couldn't determine which package, but I have several candidates:

app-root-path@2.2.1
ansi-styles@2.2.1
cli-width@2.2.1
readdirp@2.2.1
md5@2.2.1
underscore.string@2.2.1
tapable@2.2.1

Could you check it?

[sugarshin]

@rarkins I fixed it, so could you check it?

[viceice]

Otherwise LGTM

[viceice]

I think this will also fix #13796

[renovate-release]

🎉 This PR is included in version 31.53.2 🎉

The release is available on:

• GitHub release
• 31.53.2

Your semantic-release bot 📦🚀