diff --git a/actions/manifest-ownership-scan/scan.sh b/actions/manifest-ownership-scan/scan.sh index 40ae6d7..a05e41d 100755 --- a/actions/manifest-ownership-scan/scan.sh +++ b/actions/manifest-ownership-scan/scan.sh @@ -58,8 +58,18 @@ PATTERNS=( "Argo Application::kind[\"']?[[:space:]]*:[[:space:]]*[\"']?Application([\"']|[^[:alnum:]_]|$)|(type|class|struct|interface)[[:space:]]+Application([^[:alnum:]_]|$)" "Argo AppProject::kind[\"']?[[:space:]]*:[[:space:]]*[\"']?AppProject([\"']|[^[:alnum:]_]|$)|(type|class|struct|interface)[[:space:]]+AppProject([^[:alnum:]_]|$)" 'Repository URL binding::(^|[^[:alnum:]_])(repoURL|repositoryURL|repository_url|repository-url)([^[:alnum:]_]|$)' - 'Repository branch binding::(^|[^[:alnum:]_])(repoBranch|repositoryBranch|repository_branch|repository-branch|gitBranch|git_branch|git-branch|Branch)([^[:alnum:]_]|$)|refs/heads/' - 'Repository revision binding::(^|[^[:alnum:]_])(targetRevision|repoRevision|repositoryRevision|repository_revision|repository-revision|gitRevision|git_revision|git-revision)([^[:alnum:]_]|$)|(^|[^[:alnum:]_.])Revision([^[:alnum:]_]|$)' + # A bare token is ownership where the syntax BINDS it: a manifest key or list + # entry, an assignment (`cfg.Branch = ...`), a field declaration, a struct tag, + # a struct-literal field, a setter call. Matching the word anywhere made an + # English comment read as repository binding — a finding about a word rather + # than about ownership — while matching only a line-start identifier would let + # `cfg.Branch = branchFromManifest()` sail through the guard that exists to + # catch it. A plain READ (`req.Revision == ""`) stays allowed: reading a + # released contract field owns neither the type nor a binding. The explicit + # spellings and refs/heads/ stay matched wherever they appear. + "Repository branch binding::(^|[^[:alnum:]_])(repoBranch|repositoryBranch|repository_branch|repository-branch|gitBranch|git_branch|git-branch)([^[:alnum:]_]|$)|refs/heads/|^[[:space:]]*(-[[:space:]]*)?[\"']?branch[\"']?[[:space:]]*(:|=[^=])|^[[:space:]]*branch[[:space:]]+[^[:space:]]|[\"']branch[\"'][[:space:]]*:|:[\"']branch[\"']|[.>]branch[[:space:]]*=[^=]|[{,][[:space:]]*branch[[:space:]]*:|(set|with)_?branch[[:space:]]*\\(" + # Same rule as Branch above, including the allowed selector read. + "Repository revision binding::(^|[^[:alnum:]_])(targetRevision|repoRevision|repositoryRevision|repository_revision|repository-revision|gitRevision|git_revision|git-revision)([^[:alnum:]_]|$)|^[[:space:]]*(-[[:space:]]*)?[\"']?revision[\"']?[[:space:]]*(:|=[^=])|^[[:space:]]*revision[[:space:]]+[^[:space:]]|[\"']revision[\"'][[:space:]]*:|:[\"']revision[\"']|[.>]revision[[:space:]]*=[^=]|[{,][[:space:]]*revision[[:space:]]*:|(set|with)_?revision[[:space:]]*\\(" 'Git library::github\.com/(go-git/go-git|libgit2/git2go|src-d/go-git)/|org\.eclipse\.jgit|git2::Repository|simple-git|isomorphic-git|nodegit|GitPython|dulwich' 'GitHub API client::github\.com/google/go-github/|github\.com/shurcooL/githubv4|github\.NewClient|@octokit/|PyGithub|githubkit|octocrab' 'Pull request operation::PullRequest|pull_request|pull-request|pulls\.(Create|Edit|List|Merge)' diff --git a/actions/manifest-ownership-scan/scan_test.sh b/actions/manifest-ownership-scan/scan_test.sh index ccf19a5..8c1cd69 100755 --- a/actions/manifest-ownership-scan/scan_test.sh +++ b/actions/manifest-ownership-scan/scan_test.sh @@ -68,11 +68,50 @@ fi # Reading a released RPC request field named Revision is not ownership: the # plugin declares neither the type nor a repository binding. The declaration # form stays a violation, as the runtime-identifiers fixture above asserts. -if out="bash: : No such file or directory"; then +if out="$(SCAN_ROOT="$here/testdata/contract-field" bash "$scan" 2>&1)"; then pass "reading a released contract revision field is not ownership" else - fail "contract field read should pass but failed:"$' -'"" + fail "contract field read should pass but failed:"$'\n'"$out" +fi + +# Prose is not ownership. The English words "branch" and "revision" in a +# comment that explains control flow name no repository — including when the +# comment QUOTES them, which a quoted-key pattern reds unless it requires the +# colon that makes a key a key. A gate that reports either answers about a word +# rather than about ownership. Every binding spelling still fails, below. +if out="$(SCAN_ROOT="$here/testdata/runtime-prose" bash "$scan" 2>&1)"; then + pass "a comment explaining control flow, quoting either word, is not binding" +else + fail "prose comment should pass but failed:"$'\n'"$out" +fi + +# Every spelling that BINDS a branch or revision stays a violation: a manifest +# key, a list entry, a field declaration, a struct tag, a selector assignment +# (`cfg.Branch = "main"`), a setter call (`cfg.SetBranch(...)`), a +# struct-literal field (`Source{Branch: ...}`) and a bare assignment. Catching +# only the last of those — the one nobody writes in Go — would leave the guard +# green on every real binding. +if out="$(SCAN_ROOT="$here/testdata/token-declaration-forms" bash "$scan" 2>&1)"; then + fail "token declaration forms should fail but passed:"$'\n'"$out" +else + for expectation in \ + "[Repository branch binding] runtime.go:4:" \ + "[Repository branch binding] runtime.go:12:" \ + "[Repository branch binding] runtime.go:14:" \ + "[Repository branch binding] runtime.go:15:" \ + "[Repository branch binding] runtime.go:16:" \ + "[Repository branch binding] source.yaml:3:" \ + "[Repository branch binding] source.yaml:6:" \ + "[Repository revision binding] runtime.go:5:" \ + "[Repository revision binding] runtime.go:13:" \ + "[Repository revision binding] runtime.go:15:" \ + "[Repository revision binding] source.yaml:4:"; do + if grep -qF "$expectation" <<<"$out"; then + pass "token declaration reported: $expectation" + else + fail "token declaration scan missed: $expectation"$'\n'"$out" + fi + done fi # Excluded paths must not be the reason a dirty tree fails: prove the release diff --git a/actions/manifest-ownership-scan/testdata/runtime-prose/runtime.go b/actions/manifest-ownership-scan/testdata/runtime-prose/runtime.go new file mode 100644 index 0000000..fff74bf --- /dev/null +++ b/actions/manifest-ownership-scan/testdata/runtime-prose/runtime.go @@ -0,0 +1,15 @@ +package runtime + +// A comment that explains control flow is not ownership. This function takes +// the container branch when it must install nothing, whatever revision of the +// plan reached it, and no repository is bound anywhere in it. The caller names +// the "branch" it took, and the "revision" it saw: quoting an English word +// does not bind one either. The declaration forms stay violations — see the +// runtime-identifiers and token-declaration-forms fixtures. +func installsLinkedPackages(container bool) bool { + if container { + // Resolved before the container branch below, at any revision. + return false + } + return true +} diff --git a/actions/manifest-ownership-scan/testdata/token-declaration-forms/runtime.go b/actions/manifest-ownership-scan/testdata/token-declaration-forms/runtime.go new file mode 100644 index 0000000..6ffe361 --- /dev/null +++ b/actions/manifest-ownership-scan/testdata/token-declaration-forms/runtime.go @@ -0,0 +1,17 @@ +package runtime + +type Source struct { + Branch string `json:"branch"` + Revision string `json:"revision"` +} + +// Every spelling below BINDS a repository branch or revision. A guard that +// catches only a bare identifier at line start misses all but the last, which +// is the one nobody writes. +func bind(cfg *Config, s *Source) { + cfg.Branch = "main" + cfg.Revision = "HEAD" + cfg.SetBranch(s.Branch) + _ = Source{Branch: "main", Revision: "HEAD"} + Branch = s.Branch +} diff --git a/actions/manifest-ownership-scan/testdata/token-declaration-forms/source.yaml b/actions/manifest-ownership-scan/testdata/token-declaration-forms/source.yaml new file mode 100644 index 0000000..c3644d5 --- /dev/null +++ b/actions/manifest-ownership-scan/testdata/token-declaration-forms/source.yaml @@ -0,0 +1,6 @@ +spec: + source: + branch: main + revision: HEAD + tags: + - branch: release