Skip to content

Conversation

@dacoburn
Copy link
Collaborator

Fix GitLab MR comment handling causing AttributeError: 'str' object has no attribute 'id'

This bug was preventing the GitLab integration from properly updating existing Socket Security comments on merge requests, causing the CLI to crash with an AttributeError when trying to access the .id attribute on a string instead of a Comment object.

Root Cause

In commit cc45ff4 (v2.2.15), the GitLab comment retrieval logic was changed from comments.get("overview") to comments.get("overview", ""), which introduced a breaking change:

  • Before: When no existing comments were found, .get() returned None
  • After: When no existing comments were found, .get() returned an empty string ""

The conditional check if existing_overview_comment is not None: now evaluates to True for empty strings (since "" is not None is True), causing the code to attempt updating a non-existent comment by accessing .id on a string, resulting in AttributeError: 'str' object has no attribute 'id'.

Fix

Reverted the GitLab comment retrieval logic to the working v2.0.2 behavior by removing the default empty string parameters:

  • Changed comments.get("overview", "") back to comments.get("overview")
  • Changed comments.get("security", "") back to comments.get("security")
  • Applied the same fix to the remove_comment_alerts method

This ensures that when no existing comments are found, the variables are None, making the is not None checks work correctly:

  • When None: New comments are posted (correct behavior)
  • When Comment object exists: Existing comments are updated via .id access (correct behavior)

Added explanatory comments about Python's type narrowing to maintain type safety without reintroducing the bug.

Public Changelog

  • Fixed an issue with Gitlab MR commenting introduced in 2.2.15

@dacoburn dacoburn requested a review from a team as a code owner November 11, 2025 06:42
@dacoburn dacoburn requested review from Raynos and mtorp and removed request for a team November 11, 2025 06:42
@socket-security-staging
Copy link

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Updatedpypi/​socketdev@​3.0.16 ⏵ 3.0.17100 +2100100100100

View full report

@github-actions
Copy link

🚀 Preview package published!

Install with:

pip install --index-url https://test.pypi.org/simple/ --extra-index-url https://pypi.org/simple socketsecurity==2.2.27.dev1

Docker image: socketdev/cli:pr-129

@socket-security
Copy link

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Updatedpypi/​socketdev@​3.0.16 ⏵ 3.0.17100 +2100100100100

View full report

@dacoburn dacoburn merged commit 55ebf3d into main Nov 11, 2025
6 checks passed
@dacoburn dacoburn deleted the doug/fix-gitlab-mr-comments branch November 11, 2025 06:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants