Governance rejections name the resolved person, not just the fingerprint #109

Merged
a73x merged 1 commit from issue-46c1bb64 into main 2026-08-19 10:54:08 +00:00
a73x commented 2026-08-19 10:32:03 +00:00 (Migrated from git.a73x.sh)

[claude 2026-08-19] Three connected fixes in the log-identity family:

  1. Key sessions now rebind the display principal on rejection lines to
    "{name} ({fingerprint})" for governed key sessions (mirroring the
    pattern delegate sessions already used) — via a new log_principal
    helper in src/server/ssh/session.rs. ACL checks keep using the raw
    fingerprint principal; only log lines use the resolved name.

  2. "may not create" no longer fires for git-upload-pack (fetch/clone) of
    an absent repo, since creation was never on the table for that
    command regardless of the C rules. Only git-receive-pack still asks
    the create permission; the denial wording is now "no such repository
    {path}, and {principal} may not create it", while a plain missing
    repo (fetch, or ungoverned) says "no such repository {path}".

  3. The tokio::spawn'd git subprocess task is now .instrumented with
    Span::current() at spawn time, so "Git subprocess error" lines
    carry the connection's principal — the one gap left by the earlier
    per-connection span work.

TDD: two new behavioral tests in tests/governance_test.rs
(a_refused_create_names_the_resolved_person_and_the_fingerprint,
a_fetch_of_an_absent_repo_is_not_phrased_as_a_denied_create), both
watched RED against current code before the fix, GREEN after. The
fingerprint assertion uses ssh-keygen -lf as an external oracle,
following the pattern in ssh_key_principal's own test. Full cargo test (all binaries, 0 failed) and cargo clippy --all-targets clean.
Existing delegate attribution test
(a_delegate_writes_collab_refs_and_may_not_write_branches) passes
untouched.

Not covered by a dedicated test: fix 3 (span on the spawned
subprocess) is verified by code inspection/pattern-matching against
the file's other .instrument(span) call sites rather than a new
test — forcing an actual subprocess spawn failure would require
mutating PATH for the test binary process, which is shared and racy
with other concurrently-running tests that need git/ssh-keygen on
PATH.


Migrated from git-collab patche 9de4ef4b4971aae6e88a556dab484607b00b81c0.

Original author: a73x dev@a73x.sh.

Fixes #102

Revision 1: 2551fecdfe272de71c94f5bfa9058b39f792066f; base 3c72eeb60c09690f016549ee9db18799a1d45928

[claude 2026-08-19] Three connected fixes in the log-identity family: 1. Key sessions now rebind the display principal on rejection lines to "{name} ({fingerprint})" for governed key sessions (mirroring the pattern delegate sessions already used) — via a new `log_principal` helper in src/server/ssh/session.rs. ACL checks keep using the raw fingerprint principal; only log lines use the resolved name. 2. "may not create" no longer fires for git-upload-pack (fetch/clone) of an absent repo, since creation was never on the table for that command regardless of the `C` rules. Only git-receive-pack still asks the create permission; the denial wording is now "no such repository {path}, and {principal} may not create it", while a plain missing repo (fetch, or ungoverned) says "no such repository {path}". 3. The tokio::spawn'd git subprocess task is now `.instrument`ed with `Span::current()` at spawn time, so "Git subprocess error" lines carry the connection's principal — the one gap left by the earlier per-connection span work. TDD: two new behavioral tests in tests/governance_test.rs (a_refused_create_names_the_resolved_person_and_the_fingerprint, a_fetch_of_an_absent_repo_is_not_phrased_as_a_denied_create), both watched RED against current code before the fix, GREEN after. The fingerprint assertion uses `ssh-keygen -lf` as an external oracle, following the pattern in ssh_key_principal's own test. Full `cargo test` (all binaries, 0 failed) and `cargo clippy --all-targets` clean. Existing delegate attribution test (a_delegate_writes_collab_refs_and_may_not_write_branches) passes untouched. Not covered by a dedicated test: fix 3 (span on the spawned subprocess) is verified by code inspection/pattern-matching against the file's other `.instrument(span)` call sites rather than a new test — forcing an actual subprocess spawn failure would require mutating PATH for the test binary process, which is shared and racy with other concurrently-running tests that need git/ssh-keygen on PATH. --- Migrated from git-collab patche `9de4ef4b4971aae6e88a556dab484607b00b81c0`. Original author: a73x <dev@a73x.sh>. Fixes #102 Revision 1: `2551fecdfe272de71c94f5bfa9058b39f792066f`; base `3c72eeb60c09690f016549ee9db18799a1d45928`
a73x (Migrated from git.a73x.sh) approved these changes 2026-08-19 10:52:18 +00:00
a73x (Migrated from git.a73x.sh) left a comment

[claude 2026-08-19] Reviewed the full diff. All three fixes correct: log_principal resolves only governed key sessions and ACL checks keep the fingerprint form; the create/absent split is a real flow improvement (C is now only consulted for receive-pack, so a typo'd fetch is never phrased as a permission denial); spawn span capture is the connection span. Tests use the ssh-keygen -lf oracle and pin both wordings through the real server log. Style nit only, no revision needed: the log_principal local shadows the fn name.

[claude 2026-08-19] Reviewed the full diff. All three fixes correct: log_principal resolves only governed key sessions and ACL checks keep the fingerprint form; the create/absent split is a real flow improvement (C is now only consulted for receive-pack, so a typo'd fetch is never phrased as a permission denial); spawn span capture is the connection span. Tests use the ssh-keygen -lf oracle and pin both wordings through the real server log. Style nit only, no revision needed: the log_principal local shadows the fn name.
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
a73x/git-collab!109
No description provided.