feat(observability): implement AIBOMGenerator for AI-SPM supply chain compliance#5
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughIntroduces AIBOMGenerator to produce AI-BOM manifests (components, compliance, timestamp, SHA-256 manifest_hash), adds unit tests for structure and deterministic hashing, and updates CI Docker publish workflow and Dockerfile build/runtime steps. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/qwed_mcp/observability/aibom.py (1)
4-9:generate_manifestshould be a@staticmethod; addNoneguards for list parameters.The class holds no instance state, so
generate_manifestis effectively a standalone function. Marking it@staticmethod(or making it a module-level function) signals intent and avoids the implicitselfparameter.Additionally, if a caller passes
Noneforqwed_engines_usedormcp_tools_used(despite thelistannotation), the list comprehensions will raise aTypeErrorat runtime with no diagnostic message.🛠️ Proposed fix
- def generate_manifest(self, llm_model: str, qwed_engines_used: list, mcp_tools_used: list) -> dict: + `@staticmethod` + def generate_manifest(llm_model: str, qwed_engines_used: list, mcp_tools_used: list) -> dict: + if not llm_model: + raise ValueError("llm_model must be a non-empty string") + qwed_engines_used = qwed_engines_used or [] + mcp_tools_used = mcp_tools_used or []🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/qwed_mcp/observability/aibom.py` around lines 4 - 9, The generate_manifest method on class AIBOMGenerator should be converted to a `@staticmethod` (or moved to module scope) since it uses no instance state: add the `@staticmethod` decorator and remove any implicit use of self; also add None-guards for qwed_engines_used and mcp_tools_used (e.g., treat None as [] at the start of generate_manifest) so the list comprehensions over qwed_engines_used and mcp_tools_used (and usage of llm_model) won’t raise TypeError when callers pass None.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/qwed_mcp/observability/aibom.py`:
- Line 11: Replace the raw epoch float used for the "timestamp" field with an
ISO 8601 UTC string: import datetime at top of the module and set "timestamp" to
datetime.datetime.utcnow().replace(tzinfo=datetime.timezone.utc).isoformat() (or
datetime.datetime.utcnow().isoformat() + "Z") so the manifest stores a
human-readable ISO 8601 UTC timestamp instead of time.time(); update any
relevant serialization code if needed to accept the string.
- Around line 20-22: The current manifest hash uses
hashlib.sha256(str(bom).encode()) which is non-deterministic because bom already
contains a timestamp and str(dict) is an unstable serialization; fix by
computing the hash from a deterministic JSON serialization of the BOM with the
timestamp excluded: create a shallow copy of bom without the "timestamp" key (or
otherwise remove volatile fields), call json.dumps(copy, sort_keys=True,
separators=(",", ":")) and compute bom_hash =
hashlib.sha256(that_bytes).hexdigest(), then set bom["manifest_hash"] = bom_hash
while leaving the original bom["timestamp"] unchanged; update the code around
where bom_hash is created and assigned (the bom, bom_hash assignment and
manifest_hash set).
In `@tests/test_aibom.py`:
- Around line 25-38: Update the test
test_aibom_hash_is_deterministic_based_on_content to actually assert cross-call
determinism: call AIBOMGenerator().generate_manifest("claude-3-opus", [],
["search"]) twice (using the same arguments) and assert both returned
manifest_hash values are equal; use copy.deepcopy on the manifest when
recomputing the expected_hash to avoid shallow-copy pitfalls and move the
hashlib import to the top of the test file instead of importing inside the
function; reference AIBOMGenerator.generate_manifest when making these changes
so the test targets the same method twice.
---
Nitpick comments:
In `@src/qwed_mcp/observability/aibom.py`:
- Around line 4-9: The generate_manifest method on class AIBOMGenerator should
be converted to a `@staticmethod` (or moved to module scope) since it uses no
instance state: add the `@staticmethod` decorator and remove any implicit use of
self; also add None-guards for qwed_engines_used and mcp_tools_used (e.g., treat
None as [] at the start of generate_manifest) so the list comprehensions over
qwed_engines_used and mcp_tools_used (and usage of llm_model) won’t raise
TypeError when callers pass None.
…mestamps, and static type safety
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/qwed_mcp/observability/aibom.py (1)
11-15: Tighten type annotations on list parameters and return type.The parameters accept
Noneat runtime (guarded byor []on lines 14–15), but the annotations declarelist, notlist[str] | None. A strict type checker will flag callers passingNone. Similarly,-> dictcould be-> dict[str, Any].♻️ Proposed refinement
+from typing import Optional- def generate_manifest(llm_model: str, qwed_engines_used: list, mcp_tools_used: list) -> dict: + def generate_manifest( + llm_model: str, + qwed_engines_used: Optional[list[str]], + mcp_tools_used: Optional[list[str]], + ) -> dict[str, Any]:For Python 3.9+,
list[str] | Nonecan replaceOptional[list[str]]without the import.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/qwed_mcp/observability/aibom.py` around lines 11 - 15, Update the generate_manifest signature to use precise typing: change qwed_engines_used and mcp_tools_used to accept list[str] | None (or Optional[list[str]] if you prefer) and change the return type from dict to dict[str, Any]; add "from typing import Any" if needed and leave the existing runtime guards (qwed_engines_used = qwed_engines_used or [] and mcp_tools_used = mcp_tools_used or []) intact so callers can pass None safely; ensure references to the function name generate_manifest remain unchanged..github/workflows/docker-publish.yml (1)
66-75:exit-code: truewill hard-fail PRs when base-image or third-party CVEs are present.This is intentional for breaking builds on new CVEs introduced by a PR, but it will also block any PR that inherits unpatched CVEs from the base image or an upstream dependency outside the PR's control. Consider whether a
continue-on-error: truefallback or a separate required/non-required check distinction would better fit the team's workflow.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/docker-publish.yml around lines 66 - 75, The Docker Scout CVEs step ("Docker Scout CVEs" using docker/scout-action@v1) currently sets exit-code: true which will fail PRs for any CVEs inherited from base images or upstream dependencies; change the step to avoid hard-failing by either setting exit-code to false and handling failures downstream or adding continue-on-error: true (or split into a non-required nightly job vs required PR gate) so only new/PR-introduced CVEs block merges; update the step inputs around exit-code and/or add continue-on-error while keeping the command/image/only-severities/github-token settings intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/docker-publish.yml:
- Around line 63-64: There is a duplicate mapping key "github-token" in the
workflow's inputs which violates YAML and actionlint; remove the redundant entry
so only a single "github-token: ${{ secrets.GITHUB_TOKEN }}" remains in the same
map (i.e., delete the second occurrence) to avoid undefined behavior from
duplicate keys.
---
Nitpick comments:
In @.github/workflows/docker-publish.yml:
- Around line 66-75: The Docker Scout CVEs step ("Docker Scout CVEs" using
docker/scout-action@v1) currently sets exit-code: true which will fail PRs for
any CVEs inherited from base images or upstream dependencies; change the step to
avoid hard-failing by either setting exit-code to false and handling failures
downstream or adding continue-on-error: true (or split into a non-required
nightly job vs required PR gate) so only new/PR-introduced CVEs block merges;
update the step inputs around exit-code and/or add continue-on-error while
keeping the command/image/only-severities/github-token settings intact.
In `@src/qwed_mcp/observability/aibom.py`:
- Around line 11-15: Update the generate_manifest signature to use precise
typing: change qwed_engines_used and mcp_tools_used to accept list[str] | None
(or Optional[list[str]] if you prefer) and change the return type from dict to
dict[str, Any]; add "from typing import Any" if needed and leave the existing
runtime guards (qwed_engines_used = qwed_engines_used or [] and mcp_tools_used =
mcp_tools_used or []) intact so callers can pass None safely; ensure references
to the function name generate_manifest remain unchanged.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/docker-publish.ymlsrc/qwed_mcp/observability/aibom.pytests/test_aibom.py
|
🔍 Vulnerabilities of
|
| digest | sha256:55a5819be672360ee82efe0f20348814d6c2cf98e50583c4b7a8d15949333e03 |
| vulnerabilities |     |
| platform | linux/amd64 |
| size | 131 MB |
| packages | 191 |
📦 Base Image python:3.11-slim
| also known as |
|
| digest | sha256:a8e0a3090316aed0b11037aac613aef32fb1747dcc1dcb5c0f6c727a0113a07f |
| vulnerabilities |    |
Description
Description
| ||||||||||||||||||||||||
Description
|
There was a problem hiding this comment.
🧹 Nitpick comments (3)
Dockerfile (2)
16-28: Consider separating dependency install from package install for better layer caching.Currently,
pyproject.toml,README.md, andsrc/are all copied beforeuv pip install .. Any change insrc/invalidates the dependency-install cache layer. If dependencies change less frequently than source code, you can split this into two steps: install deps first (frompyproject.tomlonly), then copy and install the package.♻️ Proposed layer-caching optimization
# Copy project files COPY pyproject.toml . COPY README.md . -COPY src/ src/ # Create virtual environment and install dependencies -# We install the package in editable mode or standard mode RUN uv venv /opt/venv ENV VIRTUAL_ENV=/opt/venv ENV PATH="/opt/venv/bin:$PATH" -# Install the package (including sentry-sdk and other deps from pyproject.toml) +# Install dependencies only (cached unless pyproject.toml changes) +RUN uv pip install --no-deps -r pyproject.toml || uv pip install . + +# Now copy source and install the package +COPY src/ src/ RUN uv pip install .Note: The exact split depends on whether
uvsupports a--no-deps/--only-depsworkflow frompyproject.toml. Adjust to fit your tooling.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile` around lines 16 - 28, Split dependency installation from package installation to improve Docker layer caching: first COPY only pyproject.toml (and README.md if needed), run dependency install (e.g., use the existing RUN uv pip install -r /path/to/requirements or RUN uv pip install --no-build-isolation -e . or the tool-specific equivalent to install dependencies from pyproject.toml), then COPY src/ and run the final package install with RUN uv pip install . (or RUN uv pip install -e .) so changes to src/ do not invalidate the dependency install layer; update the Dockerfile lines referencing COPY pyproject.toml, COPY src/, RUN uv venv /opt/venv, and RUN uv pip install . accordingly.
36-36: Runtimeapt-get upgradeis good for patching, but consider pinning or logging upgraded packages.Running
apt-get upgrade -ywithout--no-install-recommendsis fine here since you're upgrading (not installing new packages). However, this makes builds non-deterministic — two builds on different days may produce different images. For audit trail purposes (especially given the AI-BOM focus of this PR), you might want to log what was upgraded or use a dated base image digest pin.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile` at line 36, The RUN line currently using "apt-get upgrade -y" makes builds non-deterministic; either pin the base image to a specific digest (use a dated base image digest) or modify the RUN command to capture what was upgraded for auditing: replace the current RUN apt-get update && apt-get upgrade -y && rm -rf /var/lib/apt/lists/* with a version that records the upgrade list (e.g., apt-get update && apt-get -y upgrade && dpkg-query -W -f='${Package} ${Version}\n' > /var/log/apt-upgraded-packages.txt && rm -rf /var/lib/apt/lists/*) so you have an audit trail, or alternatively stop upgrading in-image and instead rely on a pinned base image digest for reproducibility..github/workflows/docker-publish.yml (1)
66-76:exit-code: truecombined withcontinue-on-error: true— clarify intent with a comment.This combination means the step signals failure when critical/high CVEs are found, but the job still succeeds. The pattern is fine for a soft-gate during initial rollout, but it can confuse contributors who see a green check despite CVE findings. Consider adding an inline comment (e.g.,
# Soft-fail: report CVEs without blocking merges) so intent is obvious to future maintainers.📝 Suggested clarifying comment
- name: Docker Scout CVEs + # Soft-fail: surface critical/high CVEs in the PR without blocking the pipeline if: ${{ github.event_name == 'pull_request' }} uses: docker/scout-action@v1🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/docker-publish.yml around lines 66 - 76, The Docker Scout CVEs step currently sets exit-code: true while also using continue-on-error: true, which causes the action to report failures for critical/high CVEs but still allow the job to pass; add a clear inline comment above or next to the "Docker Scout CVEs" step (referencing the step name "Docker Scout CVEs" and the keys exit-code and continue-on-error) explaining this is an intentional soft-fail (for example: "Soft-fail: report CVEs without blocking merges during initial rollout") so future maintainers understand the intent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/docker-publish.yml:
- Around line 66-76: The Docker Scout CVEs step currently sets exit-code: true
while also using continue-on-error: true, which causes the action to report
failures for critical/high CVEs but still allow the job to pass; add a clear
inline comment above or next to the "Docker Scout CVEs" step (referencing the
step name "Docker Scout CVEs" and the keys exit-code and continue-on-error)
explaining this is an intentional soft-fail (for example: "Soft-fail: report
CVEs without blocking merges during initial rollout") so future maintainers
understand the intent.
In `@Dockerfile`:
- Around line 16-28: Split dependency installation from package installation to
improve Docker layer caching: first COPY only pyproject.toml (and README.md if
needed), run dependency install (e.g., use the existing RUN uv pip install -r
/path/to/requirements or RUN uv pip install --no-build-isolation -e . or the
tool-specific equivalent to install dependencies from pyproject.toml), then COPY
src/ and run the final package install with RUN uv pip install . (or RUN uv pip
install -e .) so changes to src/ do not invalidate the dependency install layer;
update the Dockerfile lines referencing COPY pyproject.toml, COPY src/, RUN uv
venv /opt/venv, and RUN uv pip install . accordingly.
- Line 36: The RUN line currently using "apt-get upgrade -y" makes builds
non-deterministic; either pin the base image to a specific digest (use a dated
base image digest) or modify the RUN command to capture what was upgraded for
auditing: replace the current RUN apt-get update && apt-get upgrade -y && rm -rf
/var/lib/apt/lists/* with a version that records the upgrade list (e.g., apt-get
update && apt-get -y upgrade && dpkg-query -W -f='${Package} ${Version}\n' >
/var/log/apt-upgraded-packages.txt && rm -rf /var/lib/apt/lists/*) so you have
an audit trail, or alternatively stop upgrading in-image and instead rely on a
pinned base image digest for reproducibility.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/docker-publish.ymlDockerfilesrc/qwed_mcp/observability/aibom.py
🚧 Files skipped from review as they are similar to previous changes (1)
- src/qwed_mcp/observability/aibom.py
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
1 participant
Overview
This PR implements an AI Bill of Materials (AI-BOM) generator to address critical observability and supply chain security (AI-SPM) concerns within autonomous agent ecosystems, as highlighted by Snyk.
Changes Made
manifest_hashof the execution environment to prevent post-execution tampering.Impact
Provides total visibility into the agentic supply chain, ensuring that every AI decision can be perfectly audited with a cryptographic AI-BOM manifest.
Summary by CodeRabbit
New Features
Tests
Chores