Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -310,22 +310,29 @@ jobs:
steps:
- uses: actions/checkout@v7

# The reconcile is the binary's now, so this job needs one. It builds
# rather than pinning a release because what is under test is this
# commit's reconcile against this commit's manifest.
- uses: dtolnay/rust-toolchain@stable
- run: cargo build --release

# A hook is only proven by the path a consumer takes: a repository that
# is not this one, declaring a profile, running the released entry point.
- name: A consuming repository's declaration
run: |
set -euo pipefail
mkdir -p /tmp/consumer/policy
./uphold_check.py --init > /tmp/consumer/policy/upheld.toml
cd /tmp/consumer && "$GITHUB_WORKSPACE/uphold_check.py"
cp -r policy/principles.toml /tmp/consumer/policy/principles.toml
cd /tmp/consumer && "$GITHUB_WORKSPACE/target/release/uphold" check

- name: A declaration that cannot be read is not a pass
run: |
set -euo pipefail
mkdir -p /tmp/unreadable
cd /tmp/unreadable
set +e
"$GITHUB_WORKSPACE/uphold_check.py"
"$GITHUB_WORKSPACE/target/release/uphold" check
Comment on lines 329 to +335

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

The step name does not match what the step exercises.

/tmp/unreadable is empty. uphold check therefore fails at policy discovery, not at reading a declaration. The exit code is 2 in both cases, so the assertion passes for the wrong reason, and a regression in the unreadable-declaration path would not be caught here. Write an actual malformed declaration into the directory, or rename the step to describe missing policy.

🧪 Proposed fix
-      - name: A declaration that cannot be read is not a pass
+      - name: A declaration that cannot be parsed is not a pass
         run: |
           set -euo pipefail
-          mkdir -p /tmp/unreadable
+          mkdir -p /tmp/unreadable/policy
+          cp policy/principles.toml /tmp/unreadable/policy/principles.toml
+          printf '[[enforce]] this is not toml\n' > /tmp/unreadable/policy/upheld.toml
           cd /tmp/unreadable
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- name: A declaration that cannot be read is not a pass
run: |
set -euo pipefail
mkdir -p /tmp/unreadable
cd /tmp/unreadable
set +e
"$GITHUB_WORKSPACE/uphold_check.py"
"$GITHUB_WORKSPACE/target/release/uphold" check
- name: A declaration that cannot be parsed is not a pass
run: |
set -euo pipefail
mkdir -p /tmp/unreadable/policy
cp policy/principles.toml /tmp/unreadable/policy/principles.toml
printf '[[enforce]] this is not toml\n' > /tmp/unreadable/policy/upheld.toml
cd /tmp/unreadable
set +e
"$GITHUB_WORKSPACE/target/release/uphold" check
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/test.yml around lines 329 - 335, Update the workflow step
named “A declaration that cannot be read is not a pass” so it exercises an
actual unreadable or malformed declaration rather than an empty directory;
create the required declaration fixture in /tmp/unreadable and apply permissions
or content that triggers the intended read failure, while preserving the
expected nonzero assertion for uphold check. If the test is meant to cover
missing policy instead, rename the step accordingly.

status=$?
set -e
test "$status" -eq 2 || { echo "expected exit 2, got $status"; exit 1; }
15 changes: 0 additions & 15 deletions .lefthook/pre-commit/uphold-check

This file was deleted.

15 changes: 8 additions & 7 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,12 @@ repos:
hooks:
- id: actionlint

# The shell this repository ships is the part a consumer runs before anything
# else: the lefthook wrapper it publishes under `scripts:`, and the consumer
# harness that decides whether the three runners agree. Both were unlinted,
# and the first thing this hook found was a `CDPATH=` in the published wrapper
# that reads as an assignment and is a command prefix.
# The shell this repository ships is the consumer harness that decides whether
# the three runners agree, which is the part a consumer's experience rests on.
# It was unlinted, alongside a published lefthook wrapper this hook found a
# `CDPATH=` in -- reading as an assignment where it is a command prefix. The
# wrapper is gone with the Python reconciler it existed to reach; the harness
# is not.
- repo: https://github.com/shellcheck-py/shellcheck-py
rev: v0.11.0.1
hooks:
Expand Down Expand Up @@ -226,8 +227,8 @@ repos:
# `enforcement-needs-a-trigger` record refuses.
- id: uphold-check-here
name: this repo's own enforcement claims
entry: ./uphold_check.py
entry: cargo run --quiet -- check
language: system
pass_filenames: false
stages: [pre-commit, manual]
files: '^(policy/upheld\.toml|policy/principles\.toml|\.pre-commit-config\.yaml|lefthook\.yml|\.cmd-shims/checks\.enabled)$'
files: '^(policy/upheld\.toml|policy/principles\.toml|\.pre-commit-config\.yaml|lefthook\.yml)$'
17 changes: 14 additions & 3 deletions .pre-commit-hooks.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,25 @@
# claim into a false one. It does not run on every commit, and it carries no
# principle text into any runtime: what it prints is the rule that went missing
# and the file that says so.
#
# `language: rust` like every other id here, and it is the same environment.
# This was `language: script` running `uphold_check.py`, which had to
# re-implement `config::load` to know which rules resolve -- a second reader,
# free to disagree with the engine, which it did. The loader answers now. The
# script kept the catalog modes, which read no policy and so cannot disagree.
#
# `.cmd-shims/checks.enabled` left the trigger list with the tier: whether a
# shim is on PATH ahead of the real command is not written in any file a
# repository can be asked, so the reconcile reports that seam as one it could
# not establish rather than reading a file that does not settle it.
- id: uphold-check
name: uphold
description: reconcile policy/upheld.toml against the rules this repo actually runs
entry: uphold_check.py
language: script
entry: uphold check
language: rust
stages: [pre-commit, manual]
pass_filenames: false
files: '^(policy/upheld\.toml|policy/principles\.toml|\.pre-commit-config\.yaml|lefthook\.yml|\.cmd-shims/checks\.enabled)$'
files: '^(policy/upheld\.toml|policy/principles\.toml|\.pre-commit-config\.yaml|lefthook\.yml)$'

# ── the content policy ───────────────────────────────────────────────
#
Expand Down
9 changes: 8 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,12 @@ could-not-look, never as a false claim.

A principle with no rule yet does not belong in this file. Build the rule first.

The split is which question the mode asks. Anything that decides whether a check
passed reads the policy, and the loader that resolves the policy is the binary,
so it lives there — one answer, not two programs entitled to disagree. What is
left in the script reads the catalog and renders prose for a person, and cannot
disagree with the engine about anything.

Exit codes, everywhere: `0` clean, `1` a claim is false / a violation, `2` could
not look — see [`explicit-unknown`](principles/explicit-unknown.toml).

Expand All @@ -94,14 +100,15 @@ not look — see [`explicit-unknown`](principles/explicit-unknown.toml).
```sh
uphold scan # content rules over the tree
uphold scan --text - # a commit message, release note, PR body
uphold check # the claims in policy/upheld.toml still hold
uphold check --coverage # which rules here carry a principle
uphold rules --effective # every rule inheritance resolved to, and where each runs
uphold guard --stage pre-push # the guards for that git hook
uphold shim gh pr create ... # stand in front of a command, then exec
uphold audit --for-publication # before flipping private -> public

uphold_check.py --explain ID # one record in full; also accepts a name
uphold_check.py --list # every id in the catalog
uphold_check.py --coverage # which rules here carry a principle
uphold_check.py --init # a starter declaration
uphold_check.py --oscal # OSCAL component-definition JSON
uphold_check.py --review # what routes to the review tier
Expand Down
23 changes: 15 additions & 8 deletions docs/REFERENCE.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ namespace — that is what lets a claim in
- [`uphold guard` — the guards](#uphold-guard--the-guards)
- [`uphold shim` — the shims](#uphold-shim--the-shims)
- [`uphold audit --for-publication`](#uphold-audit---for-publication)
- [`--coverage` and `--oscal`](#--coverage-and---oscal)
- [`uphold check --coverage` and `--oscal`](#uphold-check---coverage-and---oscal)
- [The review tier](#the-review-tier)

## Rule shape
Expand Down Expand Up @@ -137,11 +137,18 @@ uphold rules --effective # every resolved rule, and where it fires
uphold rules --effective --json # the same, for a program
```

The JSON is one array of `{"id": ..., "git_hooks": [...]}`, in the order the
engine resolved them. It exists so that nothing has to re-implement the loader
to find out what runs — a second reader of these fields is a reader free to
disagree with the engine, and it will disagree exactly where somebody used a
field it does not know about.
The JSON is one array of `{"id": ..., "git_hooks": [...], "seams": [...]}`, in
the order the engine resolved them. It exists so that nothing has to
re-implement the loader to find out what runs — a second reader of these fields
is a reader free to disagree with the engine, and it will disagree exactly where
somebody used a field it does not know about.

`seams` is `scan`, `guard`, `shim`, or more than one, and it is the half
`git_hooks` cannot express. An empty hook list is true of a content rule and of
a checker standing in front of a command alike, so a reader with only the hooks
has to guess between two unrelated places — and the reconciler guessed `scan`,
which credited a shim-only rule to a seam that never touches it. An empty
`seams` means nothing runs the rule at all, which the loader refuses.
Comment on lines +146 to +151

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Describe the valid seams cardinality correctly.

A loaded rule cannot currently have more than one seam. Validation permits git.hooks only on builtin rules and command.before only on exec rules. File-reading non-builtins cannot use either. State that each valid effective rule contains exactly one of scan, guard, or shim.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/REFERENCE.md` around lines 146 - 151, Update the `seams` documentation
to state that every valid effective rule contains exactly one seam: `scan`,
`guard`, or `shim`. Clarify that multiple seams are not currently supported,
`git.hooks` is valid only for `builtin` rules, `command.before` only for `exec`
rules, and file-reading non-builtins support neither.


The two requests this shape exists to make writable:

Expand Down Expand Up @@ -596,10 +603,10 @@ found, `2` where a surface this run tried to read could not be read, `0` when
every surface a flip would republish was read and was clean — subject to the
standing caveats, which the clean line says.

## `--coverage` and `--oscal`
## `uphold check --coverage` and `--oscal`

```sh
uphold_check.py --coverage # every rule the four tiers run, vs the claims
uphold check --coverage # every rule this repository runs, vs the claims
uphold_check.py --oscal > component-definition.json
```

Expand Down
37 changes: 15 additions & 22 deletions hooks/lefthook.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,32 +34,25 @@ pre-commit:
run: uphold scan
uphold-guard:
run: uphold guard --stage pre-commit
# The declaration check is the one part of this repository that is not the
# binary, so PATH cannot reach it. lefthook resolves a script against the
# remote clone rather than the consumer's tree, which is the only mechanism
# here that can reach a file in THIS repository -- so the checker arrives as a
# script and .lefthook/pre-commit/uphold-check is a shim onto it.
# The declaration check fires on a CONDITION rather than on every commit, which
# is why it is a job and not another command: `glob` is a job key.
#
# It is a `jobs:` entry rather than a `scripts:` one for a single reason:
# `glob` is a job key and a script has no equivalent. A script with no firing
# condition runs on EVERY commit, which loads and validates the whole catalog
# in front of a one-line fix -- and .pre-commit-hooks.yaml publishes the
# opposite as the design ("It does not run on every commit") and holds itself
# to it with a `files:` regex. Two distribution paths that fire on different
# occasions are two products wearing one version number, and the one that
# fires more is the one a consumer switches off.
# It arrived as a `script:` onto `.lefthook/pre-commit/uphold-check` while the
# checker was a Python file in this repository that PATH could not reach --
# lefthook's `scripts` is the one mechanism that resolves against the remote
# clone rather than the consumer's tree. The reconcile is `uphold check` now,
# so it is on PATH like every other command here and the shim is gone.
#
# The list is the same list as that regex, file for file: the declaration
# itself, plus every file a claim is reconciled against. Those are exactly the
# edits that can turn a true enforcement claim into a false one -- a rule
# deleted from policy/principles.toml, a hook id dropped from a runner's
# config, a shim check disabled. Nothing else can, which is why nothing else
# is worth a catalog load.
# .pre-commit-hooks.yaml publishes the same condition as a `files:` regex, and
# the two lists are the same list file for file: the declaration itself, plus
# every file a claim is reconciled against. Those are exactly the edits that
# can turn a true enforcement claim into a false one. Two distribution paths
# that fire on different occasions are two products wearing one version
# number, and the one that fires more is the one a consumer switches off.
jobs:
- name: uphold-check
script: "uphold-check"
runner: sh
glob: "{policy/upheld.toml,policy/principles.toml,.pre-commit-config.yaml,lefthook.yml,.cmd-shims/checks.enabled}"
run: uphold check
glob: "{policy/upheld.toml,policy/principles.toml,.pre-commit-config.yaml,lefthook.yml}"

commit-msg:
commands:
Expand Down
13 changes: 7 additions & 6 deletions lefthook.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ pre-commit:
catalog-tests:
run: python3 -m unittest discover -s tests
uphold-check:
run: python3 uphold_check.py
run: cargo run --quiet -- check
content-policy:
run: cargo run --quiet -- scan
guards:
Expand Down Expand Up @@ -45,14 +45,15 @@ pre-commit:
glob: ".github/workflows/*.{yml,yaml}"
run: actionlint {staged_files}
# The two shell files this repository ships: the consumer harness and the
# wrapper a lefthook consumer runs out of the clone. The brace pattern is
# what keeps this hook and the pre-commit one asking the same question --
# `*.sh` alone would miss the wrapper, which has no extension.
# extensionless wrapper a lefthook consumer ran out of the clone, which
# `*.sh` alone would have missed. That wrapper existed to reach a Python
# reconciler PATH could not; the reconcile is `uphold check` now, the
# directory is gone, and the pattern is the plain one again.
shellcheck:
glob: "{scripts/*.sh,.lefthook/pre-commit/*}"
glob: "scripts/*.sh"
run: shellcheck {staged_files}
bashate:
glob: "{scripts/*.sh,.lefthook/pre-commit/*}"
glob: "scripts/*.sh"
run: bashate --ignore E006 {staged_files}

# Not git hooks. lefthook has no equivalent of pre-commit's manual stage, so the
Expand Down
Loading
Loading