Skip to content

envoy.dependency.check: tighten TypedDict schema and add metadata validation#4442

Closed
Copilot wants to merge 3 commits into
mainfrom
copilot/fix-typeddict-schema-mismatch
Closed

envoy.dependency.check: tighten TypedDict schema and add metadata validation#4442
Copilot wants to merge 3 commits into
mainfrom
copilot/fix-typeddict-schema-mismatch

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 11, 2026

urls and sha256 were marked optional in DependencyMetadataDict (total=False) despite being accessed unconditionally, causing silent KeyErrors on malformed input. No validation ran before dependency objects were constructed.

Changes

typing.py

  • Promote urls: list[str] and sha256: str to BaseDependencyMetadataDict (required)
  • Keep cpe: str | None optional in the total=False subclass

exceptions.py

  • Add DependencyMetadataError(Exception) for schema validation failures

abstract/checker.py

  • Add REQUIRED_DEPENDENCY_METADATA_KEYS constant (module-level + class attribute for subclass override)
  • Add _validate_dependency_metadata(data) — collects all errors before raising once with an aggregated message:
    Invalid dependency metadata:
      dep_a: missing required keys: sha256
      dep_b: metadata must be a mapping, got str
    
  • dependency_metadata calls validation before returning loaded JSON
  • @runner.catches(...) on run now includes DependencyMetadataError for clean CLI exit

Tests

  • Update test_checker_dependency_metadata to assert validation is invoked
  • Update test_checker_run_catches to cover DependencyMetadataError
  • Add parametrized test_checker__validate_dependency_metadata (valid data, missing key, multiple errors, non-dict value)
  • Add test_checker_run_catches_dependency_metadata_error
  • Add tests/test_exceptions.py asserting DependencyMetadataError is an Exception subclass
Original prompt

Follow-up to the code-review notes in py/envoy.dependency.check/REVIEW.md (PR #4422). This is the report's recommended follow-up PR #3: fix the TypedDict schema mismatch and add input validation so malformed dependency metadata is reported clearly rather than producing cryptic KeyErrors deep inside the checker loop.

Background

envoy/dependency/check/typing.py currently splits the dependency metadata schema into a required base and a total=False extension:

class BaseDependencyMetadataDict(TypedDict):
    release_date: str
    version: str


class DependencyMetadataDict(BaseDependencyMetadataDict, total=False):
    cpe: str | None
    urls: list[str]
    sha256: str

But in envoy/dependency/check/abstract/dependency.py both urls and sha256 are accessed unconditionally:

@property
def release_sha(self) -> str:
    return self.metadata["sha256"]

@property
def urls(self) -> list[str]:
    return self.metadata["urls"]

So a malformed input file silently raises KeyError mid-check. Findings 7.1 and 11.2 of REVIEW.md cover this.

Scope

All edits in py/envoy.dependency.check/. Two related changes, one PR.

1. Tighten the TypedDict schema (finding 7.1)

File: envoy/dependency/check/typing.py.

  • urls and sha256 are required by the rest of the codebase — promote them to required fields.
  • cpe is genuinely optional — keep it optional.

Suggested shape:

from typing import TypedDict


class BaseDependencyMetadataDict(TypedDict):
    release_date: str
    version: str
    urls: list[str]
    sha256: str


class DependencyMetadataDict(BaseDependencyMetadataDict, total=False):
    cpe: str | None


DependenciesDict = dict[str, DependencyMetadataDict]

This keeps cpe optional via the total=False subclass pattern that the codebase already uses, while making urls and sha256 part of the required base. mypy will now flag any unconditional access to cpe (there shouldn't be any).

2. Pre-flight schema validation on the input JSON (finding 11.2)

File: envoy/dependency/check/abstract/checker.py.

dependency_metadata currently does:

@property
@abc.abstractmethod
def dependency_metadata(self) -> typing.DependenciesDict:
    """Dependency metadata (derived in Envoy's case from
    `repository_locations.bzl`)."""
    return json.loads(self.repository_locations_path.read_text())

After loading the JSON, validate that every entry contains the required keys (release_date, version, urls, sha256) before the checker constructs Dependency objects. On failure, raise a new exception type with a clear message that names the offending dependency and the missing key(s).

Implementation suggestions (use whichever fits cleanly — keep it lightweight, no new third-party deps):

  • Add a new exception in envoy/dependency/check/exceptions.py, e.g.:

    class DependencyMetadataError(Exception):
        """Raised when the loaded dependency metadata fails validation."""
  • In abstract/checker.py, add a class attribute listing the required keys (don't hard-code them inline more than once):

    REQUIRED_DEPENDENCY_METADATA_KEYS = ("release_date", "version", "urls", "sha256")
  • Run validation as part of dependency_metadata (or in a dedicated _validate_dependency_metadata helper called from there). Collect all errors per dependency and raise once with a multi-line message — surfacing every problem in one run is much friendlier than failing on the first one. Example shape:

    def _validate_dependency_metadata(self, data: dict) -> None:
        errors: list[str] = []
        for dep_id, meta in data.items():
            if not isinstance(meta, dict):
                errors.append(f"{dep_id}: metadata must be a mapping, got {type(meta).__name__}")
                continue
            missing = [k for k in self.REQUIRED_DEPENDENCY_METADATA_KEYS if k not in meta]
            if missing:
                errors.append(f"{dep_id}: missing required keys: {', '.join(missing)}")
        if errors:
            raise exceptions.DependencyMetadataError(
                "Invalid dependency metadata:\n  " + "\n  ".join(errors))
  • Wire it through the existing @runner.catches(...) on run so the user gets a clean exit message rather than a traceback. Add exceptions.DependencyMetadataError to the catches tuple alongside exceptions.GithubTokenError.

Keep the validation simple and structural (presence + basic type). Do NOT add a heavy schema library (jsonschema, pydantic, etc.) — a hand-rolled pass is sufficient and matches the codebase style.

Tests

Update / add tests under py/envoy.dependency.check/tests/:

  • tests/test_abstract_checker.py:
    • test_checker__validate_dependency_metadata — parametrised cases:
      • Valid metadata: no exception raised.
      • Missing one required key: raises DependencyMetadataError, message names the dep id and missing key.
      • Multiple deps wi...

This pull request was created from Copilot chat.

@netlify
Copy link
Copy Markdown

netlify Bot commented May 11, 2026

Deploy Preview for nifty-bassi-e26446 ready!

Name Link
🔨 Latest commit 7da8f83
🔍 Latest deploy log https://app.netlify.com/projects/nifty-bassi-e26446/deploys/6a01886c4c86f40008ee8df7
😎 Deploy Preview https://deploy-preview-4442--nifty-bassi-e26446.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Copilot AI changed the title [WIP] Fix TypedDict schema mismatch and add input validation envoy.dependency.check: tighten TypedDict schema and add metadata validation May 11, 2026
Copilot AI requested a review from phlax May 11, 2026 07:43
Copilot finished work on behalf of phlax May 11, 2026 07:43
@phlax phlax closed this May 11, 2026
@phlax phlax deleted the copilot/fix-typeddict-schema-mismatch branch May 11, 2026 10:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants