Skip to content

Repo hygiene: 21 ruff errors and open CodeQL findings outside the capture engines #77

Description

@imran-siddique

Found while working on the capture engines and deliberately not fixed there, to keep those pull requests reviewable. All pre-existing.

21 ruff errors at --target-version py39, in integrations/sentinel, integrations/comply54 and decisionassure. The capture engines, the core and scripts/ are clean, so a repo-wide ruff check cannot currently be a gate.

Open CodeQL findings:

  • py/stack-trace-exposure, medium, five instances in integrations/sentinel/sentinel/server.py (lines 128, 131, 299, 311, 347). Stack traces reaching an external caller. This one is worth doing first: it is a real information leak in a service, not a lint preference.
  • PinnedDependenciesID, medium, across ramen-ai-cmcp-conformance.yml, scheduled-agents-tests.yml, agentrust-codex-tests.yml, spendguard-conformance.yml, claude-code-tests.yml, codeql.yml, scorecard.yml, and integrations/sentinel/Dockerfile: unpinned pip install commands, unpinned GitHub actions, and an unpinned container base image.

The pinning findings matter more than usual here, given this repository publishes supply-chain integrity tooling. The newer workflows pin actions by SHA; the older ones do not.

Suggest splitting: one pull request for the stack-trace exposures, one for pinning, then make repo-wide ruff a required check once it is clean.

Activity

  1. kingztech2019 commented on Aug 22, 2026

    @kingztech2019
    Contributor

    I'd like to take this, starting with the stack-trace exposure fix as its own PR per the suggested split, then the pinning work as a follow-up.

    Plan for the first PR: keep the existing server-side logging (print(..., traceback.format_exc()) where it's already there, add it where it isn't) and replace what reaches the caller in all five spots with a generic message, so the exception detail stays in server logs rather than the HTTP response.

    Flagging before I start in case anyone is already on this.

  2. imran-siddique commented on Aug 23, 2026

    @imran-siddique
    MemberAuthor

    @kingztech2019 yes, go ahead, and your plan is the one I would have written. Confirming the split so scope does not drift mid-review.

    PR 1: the stack-trace exposures. Five instances of py/stack-trace-exposure in integrations/sentinel/sentinel/server.py, at lines 128, 131, 299, 311 and 347. Your approach is right: keep the server-side logging with traceback.format_exc() and add it where it is missing, then replace what reaches the caller with a generic message. The thing I care about is that the fix does not trade an information leak for an unobservable service. A caller learning nothing and an operator learning nothing are different failures, and only the first one is the bug.

    This is the one worth doing first because it is a real leak in a running service rather than a lint preference.

    PR 2: pinning. PinnedDependenciesID across ramen-ai-cmcp-conformance.yml, scheduled-agents-tests.yml, agentrust-codex-tests.yml, spendguard-conformance.yml, claude-code-tests.yml, codeql.yml, scorecard.yml, and the integrations/sentinel/Dockerfile. Unpinned pip install, unpinned actions, unpinned base image. The newer workflows already pin actions by SHA, so follow that convention rather than inventing one, and pin the Dockerfile base by digest.

    This matters more here than it would elsewhere: the repository publishes supply-chain integrity tooling, and an unpinned action in it is the kind of thing someone screenshots.

    PR 3, or the tail of PR 2: the 21 ruff errors at --target-version py39 in integrations/sentinel, integrations/comply54 and decisionassure. Capture engines, core and scripts/ are already clean.

    Then repo-wide ruff check becomes a required check, and not before, because a gate that is red on arrival gets ignored or bypassed. I will add it once the three are in rather than asking you to.

    Take them in that order and tag me on each. Thanks for #121, and for reproducing before offering both times.

  3. imran-siddique commented on Aug 27, 2026

    @imran-siddique
    MemberAuthor

    Rechecked all four findings. Three were already fixed; the fourth is now gated, and one number in the issue does not reproduce for a reason worth recording.

    Finding Status
    py/stack-trace-exposure, 5 instances in sentinel/server.py Fixed in #132. The response is a generic {"error": "Invalid input"}; only print(traceback.format_exc()) reaches stdout, which is a log rather than a caller-visible sink
    21 ruff errors in sentinel, comply54, decisionassure Fixed in #137, and now gated. See below
    Unpinned GitHub actions Fixed. All 49 actions/checkout, 42 actions/setup-python, and every third-party action are SHA-pinned
    Unpinned container base image Fixed. sentinel/Dockerfile is python:3.11-slim@sha256:9c900dea…
    Unpinned pip install in workflows Open, and arguably intended. The conformance workflows install agentrust-trace-tests unpinned precisely so they test against the current release. Pinning them would defeat what they exist to check

    The gate you asked for

    then make repo-wide ruff a required check once it is clean

    Done in #145. It runs on every PR, pins ruff==0.16.3, and selects the default set (E4, E7, E9, F).

    That scope is deliberate. It catches defects rather than preferences, and several integrations here are contributed by their vendors under CONTRIBUTING rule 5. Failing somebody's pull request on import sorting is how a self-serve submission path stops being self-serve. A wider set, if wanted, should arrive with its own fix pass rather than being switched on for the next contributor to discover.

    Nine findings had to be cleared first, and eight were introduced the same day by the WCM integrations: imports left unused when those modules moved to the SDK's artifact_digest, plus typing leftovers. They were tested and never linted, because nothing linted this repository. Exactly the gap a gate closes and a review does not.

    Why the "21" will not reproduce

    A bare ruff check on the three named directories reports 128 findings here, not 21: UP006, FA100, I001, DTZ005 and friends. Ruff resolves configuration from outside the repository when the repository carries none, so the effective rule set depends on whose machine runs it. Against the default set, those same directories are clean.

    That is the real lesson from this issue, and it is now fixed: the gate states its version and its selection explicitly, so the number means the same thing on every machine.

    Closing. The remaining pip install question is a deliberate trade-off rather than debt; happy to open a narrow issue for it if you would rather it were tracked.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions