fix: map author_association NONE to unapproved integrity

guards/github-guard/docs/INTEGRITY_TAG_SPEC.md

@@ -54,12 +54,34 @@ Integrity assignment is derived from a combination of:
 ### Author Association Initialization
 
 `author_association` provides the **initial integrity floor** for user-authored objects.
+Values are defined by the GitHub API
+([reference](https://docs.github.com/en/graphql/reference/enums#commentauthorassociation)).
 
 Initialization mapping:
 
 - `OWNER`, `MEMBER`, `COLLABORATOR` -> `approved:<scope>`
-- `CONTRIBUTOR`, `FIRST_TIME_CONTRIBUTOR` -> `unapproved:<scope>`
-- `FIRST_TIMER`, `NONE` -> `[]` (initial floor before baseline enforcement)
+- `CONTRIBUTOR`, `FIRST_TIME_CONTRIBUTOR`, `NONE` -> `unapproved:<scope>`
+- `FIRST_TIMER`, missing, unknown -> `[]` (initial floor before baseline enforcement)
+
+**Rationale for `NONE` → `unapproved`:**
+
+GitHub's definitions for `FIRST_TIMER`, `FIRST_TIME_CONTRIBUTOR`, and `NONE` are
+intentionally vague. Per the
+[CommentAuthorAssociation docs](https://docs.github.com/en/graphql/reference/enums#commentauthorassociation):
+
+| Value | GitHub definition |
+|---|---|
+| `FIRST_TIMER` | "Author has not previously committed to **GitHub**." |
+| `FIRST_TIME_CONTRIBUTOR` | "Author has not previously committed to **the repository**." |
+| `NONE` | "Author has **no association** with the repository." |
+
+`NONE` does **not** mean the user is established or trustworthy — only that they have
+no special relationship with the repository. However, `NONE` is distinct from
+`FIRST_TIMER` (brand-new to all of GitHub). In practice `NONE` covers users who have
+opened issues or commented but never committed, as well as users active elsewhere on
+GitHub. We treat `NONE` the same as `FIRST_TIME_CONTRIBUTOR` because both represent
+users with no prior contributions to the specific repo who are not brand-new to GitHub.
+Only `FIRST_TIMER` indicates a truly new GitHub account.
 
 Notes:
 
@@ -84,7 +106,7 @@ The label cannot be downgraded below its initialized level. Example: an issue au
 
 ### Public repositories
 
-- Issues (and issue-like user-submitted discussion objects) initialize from `author_association` and are commonly baseline `none:<scope>` for `NONE`/`FIRST_TIMER` after baseline enforcement.
+- Issues (and issue-like user-submitted discussion objects) initialize from `author_association` and are commonly baseline `unapproved:<scope>` for `NONE` or `none:<scope>` for `FIRST_TIMER` after baseline enforcement.
 - Pull requests:
   - Forked PR (head repo differs from base repo): endorsement baseline `unapproved:<scope>`
   - Direct PR (head repo == base repo): endorsement baseline `approved:<scope>`
@@ -99,7 +121,7 @@ Resource labels are coarse pre-check labels by tool call.
 
 | Tool / Resource Type | Private Repo | Public Repo |
 |---|---|---|
-| `get_issue`, `list_issues` | max(author_association floor, approved) | author_association floor (common NONE/FIRST_TIMER => baseline `none`) |
+| `get_issue`, `list_issues` | max(author_association floor, approved) | author_association floor (NONE => baseline `unapproved`, FIRST_TIMER => baseline `none`) |
 | `search_issues` | baseline `none` (cross-repo coarse) | baseline `none` |
 | `get_pull_request`, `list_pull_requests` | max(author_association floor, approved); merged/default-branch evidence can elevate to merged | start from author_association floor, then apply PR lineage baseline (direct => approved, forked => unapproved); merged/default-branch evidence can elevate to merged |
 | `search_pull_requests` | baseline `none` (cross-repo coarse) | baseline `none` |
@@ -131,7 +153,7 @@ Response labels are fine-grained per item and are authoritative when available.
 | Response Object Type | Private Repo | Public Repo |
 |---|---|---|
 | Repository item (`search_repositories`, `get_repository`) | approved | approved |
-| Issue item (`list_issues`, `search_issues`, `get_issue`) | max(author_association floor, approved) | author_association floor (commonly baseline `none` for `NONE`/`FIRST_TIMER`) |
+| Issue item (`list_issues`, `search_issues`, `get_issue`) | max(author_association floor, approved) | author_association floor (NONE => `unapproved`, FIRST_TIMER => `none`) |
 | Pull request item (`list_pull_requests`, `search_pull_requests`, `get_pull_request`) | max(author_association floor, approved); if merged/default-branch reachable => merged | start from author_association floor; apply lineage baseline (direct => approved, forked => unapproved); if merged/default-branch reachable => merged |
 | Commit item (`list_commits`, `get_commit`) | max(author_association floor, approved); if default-branch reachable => merged | author_association floor; if default-branch reachable => merged; otherwise stay at floor unless other endorsement evidence applies |
 | File content item (`get_file_contents`) | default/no-ref: merged; otherwise approved | default/no-ref: merged; otherwise approved |

guards/github-guard/docs/TESTING.md

@@ -310,10 +310,20 @@ integrity floor (case-insensitive) as follows:
 - `COLLABORATOR` → approved
 - `CONTRIBUTOR` → unapproved
 - `FIRST_TIME_CONTRIBUTOR` → unapproved
+- `NONE` → unapproved
 - `FIRST_TIMER` → none
-- `NONE` → none
 - missing/unknown value → none
 
+**Note on `NONE` vs `FIRST_TIMER`:** GitHub's API definitions for these values are
+intentionally vague
+([reference](https://docs.github.com/en/graphql/reference/enums#commentauthorassociation)).
+`FIRST_TIMER` ("Author has not previously committed to GitHub") indicates a brand-new
+GitHub account. `NONE` ("Author has no association with the repository") does **not**
+imply the user is established or trustworthy — only that they are not `FIRST_TIMER`.
+We map `NONE` to `unapproved` (same as `FIRST_TIME_CONTRIBUTOR`) because both
+represent users with no prior contributions to the specific repo who are not brand-new
+to GitHub.
+
 In the implementation:
 - approved floor uses `writer_integrity(scope)`
 - unapproved floor uses `reader_integrity(scope)`

guards/github-guard/rust-guard/src/labels/helpers.rs

@@ -1225,8 +1225,30 @@ pub fn max_integrity(
 ///
 /// Mapping (case-insensitive):
 /// - OWNER, MEMBER, COLLABORATOR => approved
-/// - CONTRIBUTOR, FIRST_TIME_CONTRIBUTOR => unapproved
-/// - FIRST_TIMER, NONE, missing => none
+/// - CONTRIBUTOR, FIRST_TIME_CONTRIBUTOR, NONE => unapproved
+/// - FIRST_TIMER, missing, unknown => [] (the `none:<scope>` floor is applied later by baseline enforcement)
+///
+/// ### `NONE` vs `FIRST_TIMER`
+///
+/// GitHub's API definitions for these values are intentionally vague
+/// (see <https://docs.github.com/en/graphql/reference/enums#commentauthorassociation>):
+///
+/// - `FIRST_TIMER`: "Author has not previously committed to GitHub."
+///   This indicates a brand-new GitHub account with no commit history anywhere.
+///
+/// - `FIRST_TIME_CONTRIBUTOR`: "Author has not previously committed to the repository."
+///   The user has committed elsewhere on GitHub but not to this specific repo.
+///
+/// - `NONE`: "Author has no association with the repository."
+///   This does **not** mean the user is established or trustworthy — only that
+///   they have no special relationship with the repo. In practice `NONE` covers
+///   users who have opened issues or commented but never committed, as well as
+///   accounts that have simply never interacted before.
+///
+/// We map `NONE` to `unapproved` (same as `FIRST_TIME_CONTRIBUTOR`) because
+/// both represent users with no prior contributions to the specific repo who
+/// are not brand-new to GitHub. The only value that indicates a truly new
+/// GitHub account is `FIRST_TIMER`.
 pub fn author_association_floor_from_str(
     scope: &str,
     association: Option<&str>,
@@ -1239,8 +1261,8 @@ pub fn author_association_floor_from_str(
     let normalized = raw.trim().to_ascii_uppercase();
     match normalized.as_str() {
         "OWNER" | "MEMBER" | "COLLABORATOR" => writer_integrity(scope, ctx),
-        "CONTRIBUTOR" | "FIRST_TIME_CONTRIBUTOR" => reader_integrity(scope, ctx),
-        _ => vec![], // FIRST_TIMER, NONE, or any unrecognised value
+        "CONTRIBUTOR" | "FIRST_TIME_CONTRIBUTOR" | "NONE" => reader_integrity(scope, ctx),
+        _ => vec![], // FIRST_TIMER or any unrecognised value
     }
 }
 
@@ -2184,6 +2206,50 @@ mod tests {
         assert_eq!(integrity_for_level("unknown", scope, &ctx), none_integrity(scope, &ctx));
     }
 
+    #[test]
+    fn test_author_association_none_maps_to_unapproved() {
+        // NONE means "no association with the repository" — NOT "brand new to GitHub".
+        // It should map to unapproved (reader_integrity), same as FIRST_TIME_CONTRIBUTOR.
+        // See https://docs.github.com/en/graphql/reference/enums#commentauthorassociation
+        let ctx = test_ctx();
+        let scope = "owner/repo";
+
+        // NONE → unapproved (reader_integrity)
+        assert_eq!(
+            author_association_floor_from_str(scope, Some("NONE"), &ctx),
+            reader_integrity(scope, &ctx),
+            "NONE should map to unapproved (reader) integrity"
+        );
+
+        // FIRST_TIME_CONTRIBUTOR → unapproved (same as NONE)
+        assert_eq!(
+            author_association_floor_from_str(scope, Some("FIRST_TIME_CONTRIBUTOR"), &ctx),
+            reader_integrity(scope, &ctx),
+            "FIRST_TIME_CONTRIBUTOR should map to unapproved (reader) integrity"
+        );
+
+        // FIRST_TIMER → none (brand-new GitHub account)
+        assert_eq!(
+            author_association_floor_from_str(scope, Some("FIRST_TIMER"), &ctx),
+            vec![] as Vec<String>,
+            "FIRST_TIMER should map to none (empty) integrity"
+        );
+
+        // NONE and FIRST_TIME_CONTRIBUTOR produce the same integrity
+        assert_eq!(
+            author_association_floor_from_str(scope, Some("NONE"), &ctx),
+            author_association_floor_from_str(scope, Some("FIRST_TIME_CONTRIBUTOR"), &ctx),
+            "NONE and FIRST_TIME_CONTRIBUTOR should produce identical integrity"
+        );
+
+        // NONE and FIRST_TIMER produce different integrity
+        assert_ne!(
+            author_association_floor_from_str(scope, Some("NONE"), &ctx),
+            author_association_floor_from_str(scope, Some("FIRST_TIMER"), &ctx),
+            "NONE and FIRST_TIMER should produce different integrity levels"
+        );
+    }
+
     #[test]
     fn test_effective_disapproval_integrity_defaults_to_none() {
         let ctx = PolicyContext::default();

guards/github-guard/rust-guard/src/labels/mod.rs

@@ -818,7 +818,7 @@ mod tests {
         let unknown_commit = json!({"author_association": "NONE"});
         assert_eq!(
             commit_integrity(&unknown_commit, repo, false, false, &ctx),
-            none_integrity(repo, &ctx)
+            reader_integrity(repo, &ctx)
         );
         assert_eq!(
             commit_integrity(&unknown_commit, repo, true, false, &ctx),
@@ -925,14 +925,15 @@ mod tests {
             writer_integrity(repo, &ctx)
         );
 
-        // Non-trusted bot still gets none integrity on public repo
+        // Non-trusted bot gets unapproved (reader) integrity on public repo
+        // because NONE association maps to unapproved
         let renovate_issue = json!({
             "user": {"login": "renovate[bot]"},
             "author_association": "NONE"
         });
         assert_eq!(
             issue_integrity(&renovate_issue, repo, false, &ctx),
-            none_integrity(repo, &ctx)
+            reader_integrity(repo, &ctx)
         );
     }
 
@@ -1064,11 +1065,12 @@ mod tests {
             writer_integrity(repo, &ctx_with_bots)
         );
 
-        // Without trusted_bots in context, the same bot gets none integrity
+        // Without trusted_bots in context, the same bot gets unapproved (reader)
+        // integrity because NONE association maps to unapproved
         let ctx_without_bots = default_ctx();
         assert_eq!(
             issue_integrity(&configured_bot_issue, repo, false, &ctx_without_bots),
-            none_integrity(repo, &ctx_without_bots)
+            reader_integrity(repo, &ctx_without_bots)
         );
     }
 
@@ -1091,11 +1093,11 @@ mod tests {
             writer_integrity(repo, &ctx_with_bots)
         );
 
-        // Without trusted_bots, the same bot gets none integrity
+        // Without trusted_bots, the same bot gets unapproved (reader) integrity
         let ctx_without_bots = default_ctx();
         assert_eq!(
             pr_integrity(&configured_bot_pr, repo, false, None, &ctx_without_bots),
-            none_integrity(repo, &ctx_without_bots)
+            reader_integrity(repo, &ctx_without_bots)
         );
     }
 
@@ -1128,14 +1130,14 @@ mod tests {
             writer_integrity(repo, &ctx_with_bots)
         );
 
-        // Unknown bot still gets none integrity
+        // Unknown bot gets unapproved (reader) integrity because NONE maps to unapproved
         let unknown_bot_issue = json!({
             "user": {"login": "unknown-bot[bot]"},
             "author_association": "NONE"
         });
         assert_eq!(
             issue_integrity(&unknown_bot_issue, repo, false, &ctx_with_bots),
-            none_integrity(repo, &ctx_with_bots)
+            reader_integrity(repo, &ctx_with_bots)
         );
     }
 
@@ -4044,11 +4046,12 @@ mod tests {
             writer_integrity(repo, &ctx_with_users)
         );
 
-        // Without trusted_users in context, the same user gets none integrity
+        // Without trusted_users in context, the same user gets unapproved (reader)
+        // integrity because NONE association maps to unapproved
         let ctx_without_users = default_ctx();
         assert_eq!(
             issue_integrity(&trusted_user_issue, repo, false, &ctx_without_users),
-            none_integrity(repo, &ctx_without_users)
+            reader_integrity(repo, &ctx_without_users)
         );
     }
 
@@ -4071,11 +4074,11 @@ mod tests {
             writer_integrity(repo, &ctx_with_users)
         );
 
-        // Without trusted_users, the same user gets none integrity
+        // Without trusted_users, the same user gets unapproved (reader) integrity
         let ctx_without_users = default_ctx();
         assert_eq!(
             pr_integrity(&trusted_user_pr, repo, false, None, &ctx_without_users),
-            none_integrity(repo, &ctx_without_users)
+            reader_integrity(repo, &ctx_without_users)
         );
     }
 
@@ -5082,10 +5085,10 @@ mod tests {
             "author_association": "NONE",
             "number": 10
         });
-        // No reactions → base integrity (none for external contributor on public repo)
+        // No reactions → base integrity (unapproved for NONE association on public repo)
         assert_eq!(
             issue_integrity(&issue, repo, false, &ctx),
-            helpers::none_integrity(repo, &ctx)
+            helpers::reader_integrity(repo, &ctx)
         );
     }
 
@@ -5101,10 +5104,10 @@ mod tests {
             "number": 11,
             "reactions": {"total_count": 5, "thumbs_up": 5, "+1": 5}
         });
-        // Gateway mode → fall through to base integrity (none)
+        // Gateway mode → fall through to base integrity (unapproved for NONE)
         assert_eq!(
             issue_integrity(&issue, repo, false, &ctx),
-            helpers::none_integrity(repo, &ctx)
+            helpers::reader_integrity(repo, &ctx)
         );
     }
 
@@ -5142,10 +5145,10 @@ mod tests {
             "number": 30,
             "reactions": {"nodes": []}
         });
-        // Empty nodes → no change from base integrity
+        // Empty nodes → no change from base integrity (unapproved for NONE)
         assert_eq!(
             issue_integrity(&issue, repo, false, &ctx),
-            helpers::none_integrity(repo, &ctx)
+            helpers::reader_integrity(repo, &ctx)
         );
     }
 

internal/cmd/flags_test.go

@@ -103,41 +103,43 @@ func TestRegisterFlagCompletions(t *testing.T) {
 	t.Run("config flag completion returns toml extension filter", func(t *testing.T) {
 		cmd := setupCmd(t)
 
-		flag := cmd.Flags().Lookup("config")
-		require.NotNil(t, flag, "config flag should be registered")
-		exts, ok := flag.Annotations[cobra.BashCompFilenameExt]
-		require.True(t, ok, "config flag should have filename extension annotation")
-		assert.Equal(t, []string{"toml"}, exts,
+		compFunc, ok := cmd.GetFlagCompletionFunc("config")
+		require.True(t, ok, "config flag should have a completion function")
+		completions, directive := compFunc(cmd, nil, "")
+		assert.Equal(t, []string{"toml"}, completions,
 			"config flag should complete with .toml extension")
+		assert.Equal(t, cobra.ShellCompDirectiveFilterFileExt, directive)
 	})
 
 	t.Run("log-dir flag completion returns directory filter", func(t *testing.T) {
 		cmd := setupCmd(t)
 
-		flag := cmd.Flags().Lookup("log-dir")
-		require.NotNil(t, flag, "log-dir flag should be registered")
-		_, ok := flag.Annotations[cobra.BashCompSubdirsInDir]
-		assert.True(t, ok, "log-dir flag should have directory annotation")
+		compFunc, ok := cmd.GetFlagCompletionFunc("log-dir")
+		require.True(t, ok, "log-dir flag should have a completion function")
+		_, directive := compFunc(cmd, nil, "")
+		assert.Equal(t, cobra.ShellCompDirectiveFilterDirs, directive,
+			"log-dir flag should have directory completion directive")
 	})
 
 	t.Run("payload-dir flag completion returns directory filter", func(t *testing.T) {
 		cmd := setupCmd(t)
 
-		flag := cmd.Flags().Lookup("payload-dir")
-		require.NotNil(t, flag, "payload-dir flag should be registered")
-		_, ok := flag.Annotations[cobra.BashCompSubdirsInDir]
-		assert.True(t, ok, "payload-dir flag should have directory annotation")
+		compFunc, ok := cmd.GetFlagCompletionFunc("payload-dir")
+		require.True(t, ok, "payload-dir flag should have a completion function")
+		_, directive := compFunc(cmd, nil, "")
+		assert.Equal(t, cobra.ShellCompDirectiveFilterDirs, directive,
+			"payload-dir flag should have directory completion directive")
 	})
 
 	t.Run("env flag completion returns .env extension filter", func(t *testing.T) {
 		cmd := setupCmd(t)
 
-		flag := cmd.Flags().Lookup("env")
-		require.NotNil(t, flag, "env flag should be registered")
-		exts, ok := flag.Annotations[cobra.BashCompFilenameExt]
-		require.True(t, ok, "env flag should have filename extension annotation")
-		assert.Equal(t, []string{"env"}, exts,
+		compFunc, ok := cmd.GetFlagCompletionFunc("env")
+		require.True(t, ok, "env flag should have a completion function")
+		completions, directive := compFunc(cmd, nil, "")
+		assert.Equal(t, []string{"env"}, completions,
 			"env flag should complete with .env extension")
+		assert.Equal(t, cobra.ShellCompDirectiveFilterFileExt, directive)
 	})
 
 	t.Run("guards-mode flag completion returns valid enum values", func(t *testing.T) {