fix(vex): handle 304 status code

[alegrey91]

Description

Currently, trivy deletes the content of the vex cache every time a new download happens.
This action is not always needed, since when an ETag is provided, the server may respond with 304 Not Modified, meaning the existing content at dst is still valid.
In that case, trivy should not destroy dst. Instead, we can move it aside as a backup and restore it on 304.
This particular case happens when a Vexhub repository has a small ' update_interval '.
Here's the original PR where we noticed the bug: kubewarden/sbomscanner#867
cc @fabriziosestito

Related issues

There's no open issue for it.

Remove this section if you don't have related PRs.

Checklist

• I've read the guidelines for contributing to this repository.
• I've followed the conventions in the PR title.
• I've added tests that prove my fix is effective or that my feature works.
• I've updated the documentation with the relevant information (if needed).
• I've added usage information (if the PR introduces new options)
• I've included a "before" and "after" example to the description (if the PR is a user interface change).

[knqyf263]

Could we simplify this a bit? I think os.Rename works for both files and directories on all platforms, including Windows, as long as they are on the same volume. Since backup is always in the same parent directory as dst, this should be guaranteed. We also os.RemoveAll(backup) before renaming, so the target never exists — I don't think the Windows limitation applies here.

If so, we could handle everything in download.go alone without platform-specific files:

 func Download(ctx context.Context, src, dst, pwd string, opts Options) (string, error) {
-	// go-getter doesn't allow the dst directory already exists if the src is directory.
-	_ = os.RemoveAll(dst)
+	// go-getter doesn't allow the dst directory already exists if the src is directory.
+	// We rename dst as a backup and restore it if the download is skipped or fails.
+	dst = filepath.Clean(dst)
+	cleanup, err := backupDst(dst)
+	if err != nil {
+		return "", xerrors.Errorf("failed to back up dst: %w", err)
+	}
+	defer cleanup()

// backupDst renames dst aside so that go-getter can create it fresh.
// It returns a cleanup function that restores dst on error or removes the backup on success.
func backupDst(dst string) (cleanup func(), err error) {
	backup := dst + ".backup"
	_ = os.RemoveAll(backup)

	if err := os.Rename(dst, backup); errors.Is(err, os.ErrNotExist) {
		return func() {}, nil
	} else if err != nil {
		return nil, xerrors.Errorf("failed to rename dst: %w", err)
	}

	return func() {
		if _, err := os.Stat(dst); errors.Is(err, os.ErrNotExist) {
			if err := os.Rename(backup, dst); err != nil {
				log.Warn("Failed to restore backup", log.FilePath(backup), log.Err(err))
			}
			return
		}
		if err := os.RemoveAll(backup); err != nil {
			log.Warn("Failed to remove backup", log.FilePath(backup), log.Err(err))
		}
	}, nil
}

[alegrey91]

Hello @knqyf263, I used a platform-specific files because I had failing tests on windows. I can restore it and test again. Maybe they were just flaky.

[alegrey91]

@knqyf263 fixed. Thanks for the suggestion.

[alegrey91]

@knqyf263 I've rebased the branch to main but some tests are failing. Could you please re-run them?

[alegrey91]

@knqyf263 any updates on this?

[knqyf263]

Due to the impact of the recent incident, all tokens have been revoked, and IP restrictions and rulesets have been introduced for the aquasecurity organization. As a result, the CI/CD is currently not functioning. We are working to restore it as soon as possible, so we would appreciate your patience.

[alegrey91]

Due to the impact of the recent incident, all tokens have been revoked, and IP restrictions and rulesets have been introduced for the aquasecurity organization. As a result, the CI/CD is currently not functioning. We are working to restore it as soon as possible, so we would appreciate your patience.

Apologize for that. Take your time and thanks for clarifying ;)