sbp commented on code in PR #430:
URL:
https://github.com/apache/tooling-trusted-releases/pull/430#discussion_r2635355177
##########
atr/get/sbom.py:
##########
@@ -107,31 +105,49 @@ async def report(session: web.Committer, project: str,
version: str, file_path:
block, project, version, file_path, task_result, vulnerabilities,
osv_tasks, is_release_candidate
)
- block.h2["Outdated tool"]
- outdated = None
- if task_result.outdated:
- outdated =
sbom.models.maven.OutdatedAdapter.validate_python(json.loads(task_result.outdated))
- if outdated:
- if outdated.kind == "tool":
- block.p[
- f"""The CycloneDX Maven Plugin is outdated. The used version is
- {outdated.used_version} and the available version is
- {outdated.available_version}."""
- ]
- else:
- block.p[
- f"""There was a problem with the SBOM detected when trying to
- determine if the CycloneDX Maven Plugin is outdated:
- {outdated.kind.upper()}."""
- ]
- else:
- block.p["No outdated tool found."]
+ _outdated_tool_section(block, task_result)
_cyclonedx_cli_errors(block, task_result)
return await template.blank("SBOM report", content=block.collect())
+def _outdated_tool_section(block: htm.Block, task_result:
results.SBOMToolScore):
+ block.h2["Outdated tools"]
+ if task_result.outdated:
+ outdated = []
+ if isinstance(task_result.outdated, str):
+ # Older version, only checked one tool
+ outdated =
[sbom.models.tool.OutdatedAdapter.validate_python(json.loads(task_result.outdated))]
+ elif isinstance(task_result.outdated, list):
+ # Newer version, checked multiple tools
+ outdated =
[sbom.models.tool.OutdatedAdapter.validate_python(json.loads(o)) for o in
task_result.outdated]
+ if len(outdated) == 0:
+ block.p["No outdated tools found."]
+ for result in outdated:
+ if result.kind == "tool":
+ if "Apache Trusted Releases" in result.name:
+ block.p[
+ f"""The last version of ATR used on this SBOM was
+ {result.used_version} but ATR is currently version
+ {result.available_version}."""
+ ]
+ else:
+ block.p[
+ f"""The {result.name} is outdated. The used version is
+ {result.used_version} and the available version is
+ {result.available_version}."""
+ ]
+ else:
+ block.p[
+ f"""There was a problem with the SBOM detected when trying
to
+ determine if the {result.name} is outdated:
Review Comment:
Not all of the classes in the `Outdated` union have a `.name` property. The
type checker didn't find this because it couldn't work out the type of
`outdated`. We should probably add an annotation to where `outdated` is created
above.
##########
atr/tasks/sbom.py:
##########
@@ -145,31 +144,29 @@ async def osv_scan(args: FileArgs) -> results.Results |
None:
components = [results.OSVComponent(purl=v.ref,
vulnerabilities=v.vulnerabilities) for v in vulnerabilities]
new_full_path: str | None = None
- if patch_ops:
- sbom.utilities.record_task("osv-scan", args.revision_number,
bundle.doc, patch_ops)
- patch_data = sbom.utilities.patch_to_data(patch_ops)
- merged = bundle.doc.patch(yyjson.Document(patch_data))
- description = "SBOM vulnerability scan through web interface"
- async with storage.write(args.asf_uid) as write:
- wacp = await
write.as_project_committee_participant(args.project_name)
- async with wacp.revision.create_and_manage(
- args.project_name, args.version_name, args.asf_uid or
"unknown", description=description
- ) as creating:
- new_full_path = os.path.join(str(creating.interim_path),
args.file_path)
- # Write to the new revision
- log.info(f"Writing updated SBOM to {new_full_path}")
- await aiofiles.os.remove(new_full_path)
- async with aiofiles.open(new_full_path, "w", encoding="utf-8")
as f:
- await f.write(merged.dumps())
-
- if creating.new is None:
- raise RuntimeError("Internal error: New revision not found")
+ new_version, merged = sbom.utilities.apply_patch("osv-scan",
args.revision_number, bundle, patch_ops)
Review Comment:
The deleted block in this diff shows that it was guarded by the conditional
`if patch_ops`, which means that we didn't create a new version if there were
no ops. The new version always creates a new version, even if there were no
vulnerabilities. Is this intentional? What are the advantages if so?
##########
atr/tasks/sbom.py:
##########
@@ -221,20 +218,22 @@ async def score_tool(args: FileArgs) -> results.Results |
None:
if not (full_path.endswith(".cdx.json") and os.path.isfile(full_path)):
raise SBOMScoringError("SBOM file does not exist", {"file_path":
args.file_path})
bundle = sbom.utilities.path_to_bundle(pathlib.Path(full_path))
- properties = sbom.utilities.get_atr_props_from_bundle(bundle)
+ version, properties = sbom.utilities.get_props_from_bundle(bundle)
warnings, errors = sbom.conformance.ntia_2021_issues(bundle.bom)
- outdated = sbom.maven.plugin_outdated_version(bundle.bom)
+ # TODO: Could update the ATR version with a constant showing last change
to the augment/scan tools
+ outdated = sbom.tool.plugin_outdated_version(bundle.bom)
vulnerabilities = sbom.osv.vulns_from_bundle(bundle)
cli_errors = sbom.cyclonedx.validate_cli(bundle)
return results.SBOMToolScore(
kind="sbom_tool_score",
project_name=args.project_name,
version_name=args.version_name,
revision_number=args.revision_number,
+ bom_version=version,
file_path=args.file_path,
warnings=[w.model_dump_json() for w in warnings],
errors=[e.model_dump_json() for e in errors],
- outdated=outdated.model_dump_json() if outdated else None,
+ outdated=[o.model_dump_json() for o in outdated] if outdated else None,
Review Comment:
Why do we allow both `[]` and `None` in these cases, but not for warnings
and errors? Is there something that we use `None` for? Is this related to our
discussion about supplying default values? (I haven't traced the full logic
here, sorry!)
##########
atr/sbom/tool.py:
##########
@@ -0,0 +1,108 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from __future__ import annotations
+
+import datetime
+from typing import TYPE_CHECKING, Any, Final
+
+from . import maven, models
+
+if TYPE_CHECKING:
+ from collections.abc import Callable
+
+try:
Review Comment:
There's very similar code to this in `utilities.py` too, with just one
variable name difference. Perhaps there could be a single function for it.
##########
atr/sbom/tool.py:
##########
@@ -0,0 +1,108 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from __future__ import annotations
+
+import datetime
+from typing import TYPE_CHECKING, Any, Final
+
+from . import maven, models
+
+if TYPE_CHECKING:
+ from collections.abc import Callable
+
+try:
+ from atr import metadata
+
+ atr_version = metadata.version
+except ImportError:
+ metadata = None
+ atr_version = "cli"
+
+_KNOWN_TOOLS: Final[dict[str, tuple[str, str, Callable[[str], str | None]]]] =
{
+ # name in file: ( canonical name, friendly name, version callable )
+ "cyclonedx-maven-plugin": ("cyclonedx-maven-plugin", "CycloneDX Maven
Plugin", maven.version_as_of),
+ "cyclonedx maven plugin": ("cyclonedx-maven-plugin", "CycloneDX Maven
Plugin", maven.version_as_of),
+ "apache trusted releases": ("apache trusted releases", "Apache Trusted
Releases platform", lambda _: atr_version),
+}
+
+
+def plugin_outdated_version(bom_value: models.bom.Bom) ->
list[models.tool.Outdated] | None:
+ if bom_value.metadata is None:
+ return [models.tool.OutdatedMissingMetadata()]
+ timestamp = bom_value.metadata.timestamp
+ if timestamp is None:
+ # This quite often isn't available
+ # We could use the file mtime, but that's extremely heuristic
+ # return OutdatedMissingTimestamp()
+ timestamp =
datetime.datetime.now(datetime.UTC).strftime("%Y-%m-%dT%H:%M:%SZ")
+ tools: list[Any] = []
+ tools_value = bom_value.metadata.tools
+ if isinstance(tools_value, list):
+ tools = tools_value
+ elif tools_value:
+ tools = tools_value.components or []
+ services = tools_value.services or []
+ tools.extend(services)
+ errors = []
+ for tool in tools:
+ name_or_description = (tool.name or tool.description or "").lower()
+ if name_or_description not in _KNOWN_TOOLS:
+ continue
+ if tool.version is None:
+
errors.append(models.tool.OutdatedMissingVersion(name=name_or_description))
Review Comment:
If `tool.version` is `None` then it gets sent to `outdated_version_core` as
`None`, but the second argument to that is `str`. Indeed it then goes on to
call `version_parse` which does `version_str.lstrip("v").split(".")`, so the
whole thing will crash. Probably you need a `continue` here to move on to the
next tool?
##########
atr/sbom/tool.py:
##########
@@ -0,0 +1,108 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from __future__ import annotations
+
+import datetime
+from typing import TYPE_CHECKING, Any, Final
+
+from . import maven, models
+
+if TYPE_CHECKING:
+ from collections.abc import Callable
+
+try:
+ from atr import metadata
+
+ atr_version = metadata.version
+except ImportError:
+ metadata = None
+ atr_version = "cli"
+
+_KNOWN_TOOLS: Final[dict[str, tuple[str, str, Callable[[str], str | None]]]] =
{
+ # name in file: ( canonical name, friendly name, version callable )
+ "cyclonedx-maven-plugin": ("cyclonedx-maven-plugin", "CycloneDX Maven
Plugin", maven.version_as_of),
+ "cyclonedx maven plugin": ("cyclonedx-maven-plugin", "CycloneDX Maven
Plugin", maven.version_as_of),
+ "apache trusted releases": ("apache trusted releases", "Apache Trusted
Releases platform", lambda _: atr_version),
+}
+
+
+def plugin_outdated_version(bom_value: models.bom.Bom) ->
list[models.tool.Outdated] | None:
+ if bom_value.metadata is None:
+ return [models.tool.OutdatedMissingMetadata()]
+ timestamp = bom_value.metadata.timestamp
+ if timestamp is None:
+ # This quite often isn't available
+ # We could use the file mtime, but that's extremely heuristic
+ # return OutdatedMissingTimestamp()
+ timestamp =
datetime.datetime.now(datetime.UTC).strftime("%Y-%m-%dT%H:%M:%SZ")
+ tools: list[Any] = []
+ tools_value = bom_value.metadata.tools
+ if isinstance(tools_value, list):
+ tools = tools_value
+ elif tools_value:
+ tools = tools_value.components or []
+ services = tools_value.services or []
+ tools.extend(services)
+ errors = []
+ for tool in tools:
+ name_or_description = (tool.name or tool.description or "").lower()
+ if name_or_description not in _KNOWN_TOOLS:
+ continue
+ if tool.version is None:
+
errors.append(models.tool.OutdatedMissingVersion(name=name_or_description))
+ _, name, version_func = _KNOWN_TOOLS[name_or_description]
+ available_version = outdated_version_core(timestamp, tool.version,
version_func)
+ if available_version is not None:
+ errors.append(
+ models.tool.OutdatedTool(
+ name=name,
+ used_version=tool.version,
+ available_version=available_version,
+ )
+ )
+ return errors
+
+
+def outdated_version_core(isotime: str, version: str, version_as_of:
Callable[[str], str | None]) -> str | None:
+ expected_version = version_as_of(isotime)
+ if expected_version is None:
+ return None
+ if version == expected_version:
+ return None
+ expected_version_comparable = version_parse(expected_version)
+ version_comparable = version_parse(version)
+ if expected_version_comparable is None or version_comparable is None:
+ # Couldn't parse the version
+ return None
+ # If the version used is less than the version available
+ if version_comparable[0] < expected_version_comparable[0]:
+ # Then note the version available
+ return expected_version
+ # Otherwise, the user is using the latest version
+ return None
+
+
+def version_parse(version_str: str) -> tuple[tuple[int, int, int], str] | None:
Review Comment:
I think this was previously just for parsing Maven versions? If so, those
had a fixed format, but if we're parsing more sorts of version numbers now,
there may be some which are incompatible with this function.
##########
atr/sbom/utilities.py:
##########
@@ -164,3 +195,35 @@ def _map_severity(severity: str) -> str:
if sev == "moderate":
return "medium"
return "unknown"
+
+
+def _record_task(task: str, revision: str, doc: yyjson.Document, patch_ops:
models.patch.Patch) -> models.patch.Patch:
+ properties: list[dict[str, str]] | None = get_pointer(doc, "/properties")
Review Comment:
Are all of these types definitely over `dict[str, str]`?
##########
atr/sbom/utilities.py:
##########
@@ -52,8 +80,7 @@ async def bundle_to_vuln_patch(
from .osv import vuln_patch
# TODO: May not need session (copied from ntia patch)
Review Comment:
This PR removes the session, so this comment can be removed too.
##########
atr/sbom/tool.py:
##########
@@ -0,0 +1,108 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from __future__ import annotations
+
+import datetime
+from typing import TYPE_CHECKING, Any, Final
+
+from . import maven, models
+
+if TYPE_CHECKING:
+ from collections.abc import Callable
+
+try:
+ from atr import metadata
+
+ atr_version = metadata.version
+except ImportError:
+ metadata = None
+ atr_version = "cli"
+
+_KNOWN_TOOLS: Final[dict[str, tuple[str, str, Callable[[str], str | None]]]] =
{
Review Comment:
Maybe better to use a Pydantic model, dataclass, or namedtuple etc. for the
values here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
