chore: add provenance file handling into run_macaron.sh

[tromai]

Closes #697

[nathanwn]

Thanks. LGTM.

[behnazh-w]

@tromai Now that #685 is merged, can you please add a test to ingest the witness provenance and check the results in both integration_tests_docker.sh and integration_tests.sh?

[behnazh-w]

Suggested change

	# Determine the provenance expectation path to be mounted into ${MACARON_WORKSPACE}/prov_files/${pf_name} where pf_name is a file name.
	# Mount the provenance file into ${MACARON_WORKSPACE}/prov_files/${pf_name} where pf_name is a file name.

[tromai]

Fixed in bf2f407

[nathanwn]

Not a comment for this PR in particular, but a general comment: We should probably rethink where the expected result files should be stored. To me, it makes more sense to store all expected files for a particular test case in the same directory instead of scattering it through multiple different directories, making it hard to understand which expected file belongs to which test.

[tromai]

I will remove the --digest CLI input from the added integration test after #708 is merged.

[tromai]

@behnazh-w @nathanwn I have updated the integration test case:

1. Remove the provided digest as input through --digest. With this move, we are hoping for the digest to be obtained from the provided witness provenance, combined with the repository path provided as input to form the main analysis target. (see 60d0cab).
2. Update the policy file to enforce more check results in the test case (see b1bc20c)

For the 1. change, however, just removing the --digest from the run command of the integration test won't make Macaron behave as we expected. This is because for this test case scenario, these values are provided as input:

• The witness provenance that only contains the commit hash
• The path to the repository on local file system.
• The PURL of the main target analysis.

This combination will be handled inside this block

macaron/src/macaron/slsa_analyzer/analyzer.py

Lines 702 to 708 in e214326

	case (_, _):
	# If both the PURL and the repository are provided, we will use the user-provided repository path to
	# create the ``Repository`` instance later on. This ``Repository`` instance is attached to the
	# software component initialized from the user-provided PURL.
	return Analyzer.AnalysisTarget(
	parsed_purl=parsed_purl, repo_path=repo_path_input, branch=input_branch, digest=input_digest
	)

However, its logic doesn't extract the commit hash from provenance, which leave Analyzer.AnalysisTarget.digest as empty. Therefore, when preparing the repository for the analysis, Macaron will try to find the commit from the given PURL here

macaron/src/macaron/slsa_analyzer/analyzer.py

Lines 819 to 825 in e214326

	if not digest and purl and purl.version:
	found_digest = find_commit(git_obj, purl)
	if not found_digest:
	logger.error(
	"Could not map the input purl string to a specific commit in the corresponding repository."
	)
	return None

which will fail because the target repository doesn't have any release on GitHub (see https://github.com/behnazh-w/example-maven-app/releases).
I have proposed a fix (see 82e241b) which make the test case work again. However, I am not sure that this fix is the best solution, and whether it's out of scope of this PR. There are other solutions:

• Create releases on the example-maven-project repository so that the commit finder can work.
• Still using --digest to provide digest for the integration test case and we don't need to fix anything in this PR. The fix for this issue can happen in a separated PR, together with the work on validating consistency of repo url and commit from multiple input sources.

Please let me know what you think.

[nicallen]

@behnazh-w @nathanwn I have updated the integration test case:
... I have proposed a fix (see 82e241b) which make the test case work again. However, I am not sure that this fix is the best solution, and whether it's out of scope of this PR. ...

I think the proposed fix is the correct behaviour, but I think that commit should be separated into its own PR and merged prior to finishing this one (since it isn't really related to this change, more a continuation of #708 that happens to be a prerequisite for this one).

[nathanwn]

I have proposed a fix (see 82e241b) which make the test case work again.

In your fix, if I understand it right, the user-provided digest takes precedence over the commit in the provenance.
If this is as expected, then the fix looks OK to me.
One thing I realized from the fix: this final case branch here is not necessary and should be removed (duplicating the first case branch)

macaron/src/macaron/slsa_analyzer/analyzer.py

Lines 725 to 729 in 82e241b

	case _:
	raise InvalidAnalysisTargetError(
	"Cannot determine the analysis target: PURL and repository path are missing."
	)

I agree with Nick that this should be in another PR. Do open it and I can approve it for you.

[tromai]

Thanks everyone! I will bring the fix commit to a difference Pull request shortly.

In your fix, if I understand it right, the user-provided digest takes precedence over the commit in the provenance.
If this is as expected, then the fix looks OK to me.

This is True, the digest provided from the user takes precedence.

One thing I realized from the fix: this final case branch here is not necessary and should be removed (duplicating the first case branch)

Yes this is True. I meant to put it there originally as a "default" case because mypy doesn't recognize that we have covered all possible cases.

[nathanwn]

I now remember why the last case branch is necessary. It is related to the issue of mypy cannot type-narrow tuples correctly (seems to be recently fixed but not yet released).
We can keep it around I guess.

[tromai]

@nicallen @nathanwn Thanks for your feedback. I have created a Pull Request for fixing the issue here - #711. After that PR is merged, I will rebase and fix any conflicts here.