Skip to content

Commit adb56c6

Browse files
leliaclaude
andcommitted
feat(comments): add --ignore-authorization
The write-access gate had no escape hatch, and its GitLab behavior when project membership cannot be read -- honor the command with a warning -- was the one deliberate weakness in it. Both are now a choice: enforce (default) require write access; honor with a warning where the provider cannot report it strict reject in that case instead off perform no check enforce closes the hole wherever the provider can answer without breaking a pipeline whose token cannot read membership, which is why it is the default. strict closes it everywhere and will fail those pipelines. off restores the prior behavior for anyone who needs comment-driven ignores from unverified authors. Threaded through the adapter constructors as a keyword argument with a default, so existing call sites keep working. With off the predicate is never handed to check_for_socket_comments at all, so nothing is filtered and no rejection is logged, rather than a gate that silently approves everything. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 7a986c8 commit adb56c6

7 files changed

Lines changed: 113 additions & 16 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,11 @@
6060
enabling this does not silently break pipelines that relied on ignore commands.
6161
Use a `GITLAB_TOKEN` with API read access to get enforcement.
6262
- A rejected command is logged and is also absent from the ignore telemetry, which
63-
records what was acted on.
63+
records what was acted on. No acknowledgement reaction is added to a comment that
64+
was not honored.
65+
- `--ignore-authorization` selects the policy: `enforce` (default) requires write
66+
access and honors the command with a warning where the provider cannot report it,
67+
`strict` rejects it in that case instead, and `off` performs no check.
6468

6569
### Fixed: pull request and merge request comment accuracy
6670

‎docs/cli-reference.md‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -432,6 +432,7 @@ The launcher can be tuned via the `SOCKET_CLI_COANA_LAUNCHER` environment variab
432432
| `--ignore-commit-files` | False | False | Ignore commit files |
433433
| `--disable-blocking` | False | False | Non-blocking CI mode: the CLI always exits **0**, even when blocking alerts are present (including with `--strict-blocking`). Also exits 0 on uncaught runtime errors and Socket API failures, so the job is treated as successful while findings and errors are still logged. Takes precedence over `--strict-blocking`. |
434434
| `--disable-ignore` | False | False | Disable support for `@SocketSecurity ignore` commands in PR comments. When set, alerts cannot be suppressed via comments and ignore instructions are hidden from comment output. See [Who can ignore an alert](#who-can-ignore-an-alert). |
435+
| `--ignore-authorization` | False | enforce | Who may suppress alerts with `@SocketSecurity ignore`. `enforce` requires write access and honors the command with a warning when the provider cannot report it; `strict` rejects it in that case; `off` honors any commenter. See [Who can ignore an alert](#who-can-ignore-an-alert). |
435436
| `--strict-blocking` | False | False | Fail on ANY security policy violations (blocking severity), not just new ones. Only works in diff mode. See [Strict Blocking Mode](#strict-blocking-mode) for details. |
436437
| `--enable-diff` | False | False | Enable diff mode even when using `--integration api` (forces diff mode without SCM integration) |
437438
| `--scm` | False | api | Source control management type |
@@ -704,10 +705,21 @@ reported. `--disable-ignore` turns the feature off entirely.
704705
| GitLab | Project membership, read once per run when an ignore command is present. Developer (30) or above is honored. | The command is honored and a warning is logged. |
705706
706707
GitLab notes carry no permission field, so the check needs a `GITLAB_TOKEN` that
707-
can read `GET /projects/:id/members/all`. A `CI_JOB_TOKEN` generally cannot, and
708-
in that case the CLI logs a warning and still honors the command rather than
709-
breaking a pipeline that was already relying on it. Use a personal or group access
710-
token with API read access to get enforcement.
708+
can read `GET /projects/:id/members/all`. A `CI_JOB_TOKEN` generally cannot.
709+
710+
`--ignore-authorization` decides what happens when access cannot be determined:
711+
712+
| Value | Verified write access | Access cannot be determined |
713+
|:------|:----------------------|:----------------------------|
714+
| `enforce` (default) | Honored | Honored, with a warning naming the author |
715+
| `strict` | Honored | Rejected |
716+
| `off` | Honored | Honored, no check performed |
717+
718+
`enforce` closes the hole wherever the provider can answer, without breaking a
719+
pipeline whose token cannot read membership. `strict` closes it everywhere, at the
720+
cost of failing those pipelines. `off` restores the prior behavior and should be
721+
paired with `--disable-ignore` unless you specifically need comment-driven ignores
722+
from unverified authors.
711723
712724
## GitLab Token Configuration
713725

‎socketsecurity/config.py‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,7 @@ class CliConfig:
144144
ignore_commit_files: bool = False
145145
disable_blocking: bool = False
146146
disable_ignore: bool = False
147+
ignore_authorization: str = "enforce"
147148
# Tri-state log-upload preference: True = --upload-logs, False = --no-upload-logs,
148149
# None = neither (server-side override decides).
149150
upload_logs: Optional[bool] = None
@@ -305,6 +306,7 @@ def from_args(cls, args_list: Optional[List[str]] = None) -> 'CliConfig':
305306
'ignore_commit_files': args.ignore_commit_files,
306307
'disable_blocking': args.disable_blocking,
307308
'disable_ignore': args.disable_ignore,
309+
'ignore_authorization': args.ignore_authorization,
308310
'upload_logs': args.upload_logs,
309311
'strict_blocking': args.strict_blocking,
310312
'integration_type': integration_type,
@@ -724,6 +726,19 @@ def create_argument_parser() -> argparse.ArgumentParser:
724726
action="store_true",
725727
help="If true, the new scan will be set as the branch's head scan"
726728
)
729+
config_group.add_argument(
730+
"--ignore-authorization",
731+
dest="ignore_authorization",
732+
choices=["enforce", "strict", "off"],
733+
default="enforce",
734+
help=(
735+
"Who may suppress alerts with @SocketSecurity ignore comments. "
736+
"'enforce' (default) requires write access, and honors the command with "
737+
"a warning when the provider cannot report the commenter's access. "
738+
"'strict' rejects the command in that case instead. "
739+
"'off' honors a command from any commenter."
740+
)
741+
)
727742
config_group.add_argument(
728743
"--pending_head",
729744
dest="pending_head",

‎socketsecurity/core/scm/github.py‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,9 +154,15 @@ def from_env(cls, pr_number: Optional[str] = None) -> 'GithubConfig':
154154

155155

156156
class Github:
157-
def __init__(self, client: CliClient, config: Optional[GithubConfig] = None):
157+
def __init__(
158+
self,
159+
client: CliClient,
160+
config: Optional[GithubConfig] = None,
161+
ignore_authorization: str = "enforce",
162+
):
158163
self.config = config or GithubConfig.from_env()
159164
self.client = client
165+
self.ignore_authorization = ignore_authorization
160166

161167
if not self.config.token:
162168
log.error("Unable to get Github API Token")
@@ -224,7 +230,8 @@ def get_comments_for_pr(self) -> dict:
224230
else:
225231
log.error(raw_comments)
226232

227-
return Comments.check_for_socket_comments(comments, self.is_ignore_authorized)
233+
gate = None if self.ignore_authorization == "off" else self.is_ignore_authorized
234+
return Comments.check_for_socket_comments(comments, gate)
228235

229236
def is_ignore_authorized(self, comment: Comment) -> bool:
230237
"""Whether a commenter may suppress alerts with @SocketSecurity ignore.

‎socketsecurity/core/scm/gitlab.py‎

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -134,9 +134,15 @@ class Gitlab:
134134
MEMBER_PAGE_SIZE = 100
135135
MEMBER_PAGE_LIMIT = 10
136136

137-
def __init__(self, client: CliClient, config: Optional[GitlabConfig] = None):
137+
def __init__(
138+
self,
139+
client: CliClient,
140+
config: Optional[GitlabConfig] = None,
141+
ignore_authorization: str = "enforce",
142+
):
138143
self.config = config or GitlabConfig.from_env()
139144
self.client = client
145+
self.ignore_authorization = ignore_authorization
140146
# None until the first ignore comment forces a lookup; stays None when the
141147
# members API cannot be read, which is the "undetermined" state.
142148
self._member_access: Optional[dict] = None
@@ -268,7 +274,8 @@ def get_comments_for_pr(self) -> dict:
268274
comment.body_list = comment.body.split("\n")
269275
else:
270276
log.error(raw_comments)
271-
return Comments.check_for_socket_comments(comments, self.is_ignore_authorized)
277+
gate = None if self.ignore_authorization == "off" else self.is_ignore_authorized
278+
return Comments.check_for_socket_comments(comments, gate)
272279

273280
def _load_member_access(self) -> Optional[dict]:
274281
"""Map project member user id -> access level, or None if unreadable.
@@ -331,11 +338,18 @@ def is_ignore_authorized(self, comment: Comment) -> bool:
331338
"""
332339
access = self._load_member_access()
333340
if access is None:
341+
author = Comments.comment_author_name(comment)
342+
if self.ignore_authorization == "strict":
343+
log.warning(
344+
f"Rejecting @SocketSecurity ignore from {author}: GitLab project "
345+
"membership could not be read and --ignore-authorization is strict."
346+
)
347+
return False
334348
log.warning(
335-
"Honoring @SocketSecurity ignore from "
336-
f"{Comments.comment_author_name(comment)} without verifying write "
337-
"access: GitLab project membership could not be read. Use a token "
338-
"with API read access to enforce this."
349+
f"Honoring @SocketSecurity ignore from {author} without verifying "
350+
"write access: GitLab project membership could not be read. Use a "
351+
"token with API read access, or --ignore-authorization strict to "
352+
"reject instead."
339353
)
340354
return True
341355

‎socketsecurity/socketcli.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -563,11 +563,11 @@ def main_code():
563563
# Only pass pr_number if it's not "0" (the default)
564564
pr_number = config.pr_number if config.pr_number != "0" else None
565565
github_config = GithubConfig.from_env(pr_number=pr_number)
566-
scm = Github(client=client, config=github_config)
566+
scm = Github(client=client, config=github_config, ignore_authorization=config.ignore_authorization)
567567
elif config.scm == 'gitlab':
568568
from socketsecurity.core.scm.gitlab import Gitlab, GitlabConfig
569569
gitlab_config = GitlabConfig.from_env()
570-
scm = Gitlab(client=client, config=gitlab_config)
570+
scm = Gitlab(client=client, config=gitlab_config, ignore_authorization=config.ignore_authorization)
571571
# Don't override config.default_branch if it was explicitly set via --default-branch flag
572572
# Only use SCM detection if --default-branch wasn't provided
573573
if scm is not None and not config.default_branch:

‎tests/unit/test_ignore_authorization.py‎

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,10 @@ def test_ignore_all_from_an_outsider_is_rejected_too():
7272
# --- GitLab: notes carry no permission field, so membership is looked up -----
7373

7474

75-
def _gitlab(members_pages=None, raises=None):
75+
def _gitlab(members_pages=None, raises=None, policy="enforce"):
7676
gitlab = Gitlab.__new__(Gitlab)
7777
gitlab.config = SimpleNamespace(mr_project_id="42", headers={}, api_url="https://gl/api/v4")
78+
gitlab.ignore_authorization = policy
7879
gitlab._member_access = None
7980
gitlab._member_lookup_attempted = False
8081

@@ -142,3 +143,47 @@ def test_gitlab_oversized_membership_is_undetermined():
142143
# Undetermined falls back to honoring the command, same as an API failure.
143144
assert gitlab.is_ignore_authorized(_comment(author={"id": 999})) is True
144145
assert len(gitlab.calls) == Gitlab.MEMBER_PAGE_LIMIT
146+
147+
148+
# --- --ignore-authorization ---------------------------------------------------
149+
150+
151+
def test_strict_rejects_when_membership_cannot_be_read(caplog):
152+
"""strict closes the gap enforce leaves open, at the cost of breaking a
153+
pipeline whose token cannot read members."""
154+
gitlab = _gitlab(raises=Exception("403 Forbidden"), policy="strict")
155+
156+
with caplog.at_level("WARNING", logger="socketcli"):
157+
allowed = gitlab.is_ignore_authorized(_comment(author={"id": 7, "username": "dev"}))
158+
159+
assert allowed is False
160+
assert "strict" in caplog.text
161+
162+
163+
def test_strict_still_honors_a_verified_member():
164+
gitlab = _gitlab([[{"id": 7, "access_level": 40}]], policy="strict")
165+
166+
assert gitlab.is_ignore_authorized(_comment(author={"id": 7})) is True
167+
168+
169+
def test_off_skips_the_gate_entirely():
170+
"""off restores the prior behavior: no predicate reaches the bucketing, so
171+
nothing is filtered and no rejection is logged."""
172+
github = Github.__new__(Github)
173+
github.ignore_authorization = "off"
174+
github.config = SimpleNamespace(owner="o", repository="r", pr_number="1",
175+
headers={}, api_url="https://api.github.com")
176+
github.client = SimpleNamespace(request=lambda **kw: SimpleNamespace(
177+
json=lambda: [{"id": 1, "body": "@SocketSecurity ignore npm/lodash@4.17.21",
178+
"author_association": "NONE", "user": {"login": "outsider"}}],
179+
text=""))
180+
181+
bucketed = github.get_comments_for_pr()
182+
183+
assert len(bucketed.get("ignore", [])) == 1
184+
185+
186+
def test_enforce_is_the_default_policy():
187+
from socketsecurity.config import CliConfig
188+
189+
assert CliConfig.from_args(["--api-token", "t"]).ignore_authorization == "enforce"

0 commit comments

Comments
 (0)