Skip to content

fix: Fix parsing of non-JSON messages #265

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix: Fix parsing of non-JSON messages #265

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion functions/messages/text_message.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
"MessageId": "f86e3c5b-cd17-1ab8-80e9-c0776d4f1e7a",
"TopicArn": "arn:aws:sns:us-gov-west-1:123456789012:ExampleTopic",
"Subject": "All Fine",
"Message": "This\nis\na typical multi-line\nmessage from SNS!\n\nHave a ~good~ amazing day! :)",
"Message": "This\nis\na typical multi-line\ntext-formatted message from SNS!\n\nHave a ~good~ amazing day! :)",
"Timestamp": "2019-02-12T15:45:24.091Z",
"SignatureVersion": "1",
"Signature": "EXAMPLE",
Expand Down
15 changes: 5 additions & 10 deletions functions/notify_slack.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -574,7 +574,7 @@ def format_default(
return attachments


def parse_notification(message: Dict[str, Any], subject: Optional[str], region: str) -> Optional[Dict]:
def parse_notification(message: Union[str, Dict[str, Any]], subject: Optional[str], region: str) -> Dict[str, Any]:
"""
Parse notification message and format into Slack message payload

Expand All @@ -583,7 +583,7 @@ def parse_notification(message: Dict[str, Any], subject: Optional[str], region:
:params region: AWS region where the event originated from
:returns: Slack message payload
"""
if "AlarmName" in message:
if isinstance(message, Dict) and "AlarmName" in message:
return format_cloudwatch_alarm(message=message, region=region)
Comment on lines +586 to 587
Copy link
Author

@cdepillabout cdepillabout Nov 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

format_cloudwatch_alarm() is assuming it is receiving a Dict, so I believe adding the isinstance(message, Dict) check here is correct.

if isinstance(message, Dict) and message.get("detail-type") == "GuardDuty Finding":
return format_guardduty_finding(message=message, region=message["region"])
Expand Down Expand Up @@ -619,23 +619,18 @@ def get_slack_message_payload(
"username": slack_username,
"icon_emoji": slack_emoji,
}
attachment = None

if isinstance(message, str):
try:
message = json.loads(message)
except json.JSONDecodeError:
logging.info("Not a structured payload, just a string message")

message = cast(Dict[str, Any], message)

if "attachments" in message or "text" in message:
if isinstance(message, Dict) and ("attachments" in message or "text" in message):
message = cast(Dict[str, Any], message)
payload = {**payload, **message}
else:
attachment = parse_notification(message, subject, region)

if attachment:
payload["attachments"] = [attachment] # type: ignore
payload["attachments"] = [parse_notification(message, subject, region)]

return payload
Comment on lines 623 to 640
Copy link
Author

@cdepillabout cdepillabout Nov 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the core of the bug.

Imagine this get_slack_message_payload() function gets a message: str that looks like:

"Hi this is a message about context-switching."

In the original code, the if isinstance(message, str) would be triggered on line 624, and the message would be tried to be parsed as JSON, but that would fail.

So after the if block, message is still just a str, but it was being incorrectly cast to Dict[str, Any].

And then in the next line we were doing if "attachments" in message or "text" in message:. Here, if message was a str, then "text" in message will just check whether or not message contains the string "text":

$ python
>>> "text" in "Hi this is a message about context-switching."
True

I'm pretty sure this isn't what was intended.


Expand Down
2 changes: 1 addition & 1 deletion functions/snapshots/snap_notify_slack_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -645,7 +645,7 @@
'value': '''This
is
a typical multi-line
message from SNS!
text-formatted message from SNS!

Have a ~good~ amazing day! :)'''
}
Expand Down
Loading