models: hoist the duplicated egress access dedup out of ttyrec and scp#28
Merged
Conversation
Previously, getUniqueAccessFromAvailableAccesses existed twice — once in cmd/ttyrec.go and once in cmd/scp.go — with a verbatim-identical deduplication loop (collapse accesses with the same host/user/port, complete wide-prefix grants with the host the user typed) and only the final ambiguity policy differing: ttyrec prompts the user interactively, scp refuses because its wrapper has no terminal to prompt on. This was problematic because the shared logic had no tests and any fix to one copy could silently miss the other — the classic drift risk of duplicated security-adjacent code. This patch hoists the shared part into models.UniqueAccessesForHost (order-preserving dedup plus in-place wide-prefix host completion, documented including why the completion participates in dedup), and reduces both command methods to thin policy wrappers: single match → use it; several → ttyrec prompts, scp errors with its historical message. No behavior change. Table tests pin the hoisted function: duplicate collapse with order preserved, distinctness by user and port, wide-prefix completion with the typed host, a completed prefix deduplicating against an explicit grant, and the empty input. go build ./..., go test ./... and golangci-lint run are green, and the demo-stack e2e suite passes against this branch (the host-connection and SCP paths exercise both wrappers end to end).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
getUniqueAccessFromAvailableAccessesexisted twice — verbatim-identical dedup loops incmd/ttyrec.goandcmd/scp.go— differing only in what happens on an ambiguous target. The shared logic now lives once asmodels.UniqueAccessesForHost, tested; the commands keep only their policy tails. No behavior change.Previous behavior
Both commands carried their own copy of the loop that collapses accesses with the same host/user/port and completes wide-prefix grants (stored with an empty host) with the host the user typed.
Why it was a problem
The shared logic had no tests, and a fix to one copy could silently miss the other — the classic drift risk of duplicated security-adjacent code on the egress path.
What changed
models.UniqueAccessesForHost(accesses, host): order-preserving dedup plus in-place wide-prefix host completion, with the rationale documented (the completion participates in dedup, so a prefix grant collapses against an explicit grant for the same target).ttyrecandscpkeep thin wrappers holding only their policy: single match → use it; several → ttyrec prompts the user interactively, scp refuses with its historical message (its wrapper has no terminal to prompt on).How it's tested
Table tests pin the hoisted function: duplicate collapse with order preserved, distinctness by user and port, wide-prefix completion with the typed host, a completed prefix deduplicating against an explicit grant, and empty input.
go build ./...,go test ./...,golangci-lint rungreen; demo-stack e2e passes against this branch (host connections and SCP exercise both wrappers end to end).Test plan
go test ./internal/models/ -run TestUniqueAccessesForHost -vtests/e2e-local.sh