Skip to content

fix(sieve): do not call the platform without a repository identity - #579

Merged
Marc-cn merged 1 commit into
darnitdevorg:mainfrom
Marc-cn:fix/no-remote-platform-checks
Oct 12, 2026
Merged

Marc-cn merged 1 commit into
darnitdevorg:mainfrom
Marc-cn:fix/no-remote-platform-checks

Conversation

@Marc-cn

@Marc-cn Marc-cn commented Oct 10, 2026

Copy link
Copy Markdown
Collaborator

Summary

On a checkout with no git remote, detect_owner_repo returns ("", <directory name>). The gh_api step substitutes that into its endpoint and calls the platform anyway:

! OSPS-LE-02.02: ERROR [unavailable] - Platform API call /repos//toy/license could not be measured (unavailable): HTTP 404

Two problems with that:

  1. The message reads as "GitHub says this does not exist". The real situation is that darnit has no repository to ask about. I hit this auditing a new local repository that had not been pushed yet, which is the first thing a new user does.
  2. The 404 is treated as a platform answer. If a step declares 404 in fail_on_status, the handler returns FAIL with confidence 1.0 ("which proves failure for this control") for a request that was never about the audited repository. No baseline step declares fail_on_status today, so this is a latent risk and not a wrong verdict I observed.

This PR makes gh_api_handler check before substituting: if the endpoint uses $OWNER or $REPO and the context has no value for it, it returns ERROR (class unavailable) without calling the platform:

! OSPS-LE-02.02: ERROR [unavailable] - Platform API call /repos/$OWNER/$REPO/license needs $OWNER, and this checkout has no repository identity (no git remote was found)

github_branch_protection already refuses the same way ("needs owner/repo"), so this brings the generic step in line with it. Endpoints that need neither, such as /user, are unaffected.

What this does not decide: whether platform controls on a repository with no remote should be ERROR, N/A or something else. They were ERROR before and they still are. Related: #334, #344.

Type of Change

  • Bug fix (non-breaking change fixing an issue)
  • New feature (non-breaking change adding functionality)
  • Breaking change (fix or feature causing existing functionality to change)
  • Documentation update
  • Refactoring (no functional changes)

Framework Changes Checklist

  • Updated framework spec (docs/architecture/framework-design.md) if behavior changed (section 3.8 describes the gh_api step; I have not edited it. Say if this case should be written down there)
  • Ran uv run python scripts/validate_sync.py --verbose and it passes

Testing

  • Tests pass locally (uv run pytest tests/ -v): 5141 passed, 26 skipped (main: 5138)
  • Added tests for new functionality (if applicable)
  • Linting passes (uv run ruff check .)

Three tests in tests/darnit/sieve/test_gh_api_handler.py, using the existing recorded responder: no owner gives ERROR and makes no call; no owner with fail_on_status: [404] gives ERROR and makes no call; an endpoint that needs no identity still runs. The first two fail without the fix.

By hand, darnit audit --tags level=1 on a one-file repository with no remote: 26 controls and 10 errors before and after. Only the messages change, and nothing is requested from /repos//toy or /orgs/.

AI assistance

  • No AI assistance was used
  • AI assistance was used

Claude Fable 5. wrote the check, I ran every command and checked the results myself. The commit carries an Assisted-by: Claude:claude-fable-5-1.

Additional Notes

  • Error class: I used unavailable. github_branch_protection sets no class for the same case.
  • The handler still logs one warning per affected control, as it does for other measurement failures. On a repository with no remote that is about ten similar lines; a single summary line would read better, but that is a wider change to how environmental failures are reported.

mlieberman85
mlieberman85 previously approved these changes Oct 12, 2026

@mlieberman85 mlieberman85 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I checked this on a repo with no remote: statuses are unchanged and the 15 /repos//… and /orgs/ calls are gone. A few small things:

  1. Yes, please add the rule to framework-design.md 3.8's gh_api behavior list (endpoint needs $OWNER/$REPO and has no value → ERROR, no call).
  2. github_branch_protection reports the same case as evaluation; could both use one class? unavailable is documented as "the platform answered badly or never answered", so whichever you pick, it'd be good to make them match.
  3. The message says "no git remote was found", but an empty owner can also come from a caller passing only repo. Something like "no repository owner/name is known (no git remote, and none was given)" would avoid naming the wrong cause.
  4. Optional follow-up: exec steps (e.g. gh release list --repo $OWNER/$REPO in the has_releases detect, toml:314) and the mcp args substitution still substitute an empty owner.

Drafted with Claude Code; reviewed and posted by me.

On a checkout with no git remote, detect_owner_repo returns an empty owner. gh_api_handler substituted it and asked the platform about /repos//name. The 404 that came back was reported as a platform answer, and a step declaring 404 in fail_on_status would have returned FAIL for a request that was never about the audited repository.

Return ERROR (class unavailable) with a message that says the checkout has no repository identity, without calling the platform. github_branch_protection already refuses the same way.

Signed-off-by: Marc-cn <130138935+Marc-cn@users.noreply.github.com>
Assisted-by: Claude:claude-fable-5-1
@Marc-cn

Marc-cn commented Oct 12, 2026

Copy link
Copy Markdown
Collaborator Author

Rebased onto main after #582 and #577 merged. The only conflict was the CHANGELOG entry, and nothing else changed. Merging on your approval once CI is green. Your points 1 to 3 go into a follow-up PR.

@Marc-cn
Marc-cn force-pushed the fix/no-remote-platform-checks branch from 316cb68 to 3d7a714 Compare October 12, 2026 20:38
@Marc-cn
Marc-cn merged commit 920b5cc into darnitdevorg:main Oct 12, 2026
9 checks passed
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