fix(sns__enum): paginate list_topics and list_subscriptions_by_topic to prevent enumeration false-negatives

[DevamShah]

Summary

sns__enum called list_topics() and list_subscriptions_by_topic() exactly
once per region/topic, silently capping enumeration at 100 items per call and
producing false-negatives in accounts with more than 100 SNS topics or more than
100 subscriptions on a single topic.

Problem / Motivation

The AWS SNS API paginates list_topics at 100 topics per response and
list_subscriptions_by_topic at 100 subscriptions per response. Prior to this
fix, both calls were made exactly once — the NextToken field in the response
was ignored. In any AWS account where either threshold is crossed, sns__enum
would silently under-report:

• A topology with 150 SNS topics would show 100. The 50 missing topics — and all
  their subscribers — would never be examined.
• A single high-fan-out topic with 150 confirmed subscribers would report only
  100 subscribers.

This is a direct enumeration false-negative: an operator running Pacu in a
large-scale AWS environment receives a clean-looking report that materially
misrepresents the SNS attack surface. From a threat-modelling perspective the
missed topics could include SNS→Lambda chains, cross-account notification
channels, or data-exfiltration primitives that a red-team or a defender needs to
account for (MITRE ATT&CK T1526 — Cloud Service Discovery).

The pattern is identical to the bug fixed in PR #503 and mirrors pagination
already correctly implemented in codebuild__enum (nextToken) and
dynamodb__enum (LastEvaluatedTableName).

Change

pacu/modules/sns__enum/main.py:

• Replaced the single client.list_topics() call with a NextToken while-loop
  that accumulates all topic ARNs into all_topics before iterating.
• Replaced the single client.list_subscriptions_by_topic() call per topic with
  a NextToken while-loop that accumulates all subscriptions into
  all_subscriptions before appending to the result.
• Both loops guard with and response["NextToken"] != "". A bare
  while "NextToken" in response does not terminate when the terminal page
  carries an empty token instead of omitting the key — the same defect class as
  fix: secrets__enum infinite loop from broken pagination (closes #487) #503. This matches the idiom in glue__enum/main.py:56 and
  transfer_family__enum/main.py:44.
• No changes to data structures, summary output, or session-persistence logic;
  the fix is narrowly scoped to the two pagination gaps.

pacu/modules/sns__enum/tests/test_sns__enum.py (new file):

• Five pytest tests, four backed by moto.mock_aws:
  1. test_list_topics_follows_next_token — creates 105 topics, asserts all 105
     are in the result (fails on the unfixed code).
  2. test_list_subscriptions_by_topic_follows_next_token — creates 1 topic with
     105 subscriptions, asserts all 105 subscribers are collected (fails on the
     unfixed code).
  3. test_single_page_topics_unaffected — smoke test confirming the common
     (<100 topics) path is unchanged.
  4. test_empty_region_excluded_from_result — confirms the existing behaviour
     of dropping empty regions is preserved.
  5. test_empty_next_token_terminates_pagination — a fake client whose last
     page returns NextToken: "", with a hard call cap so a regression fails
     fast instead of hanging the suite.

The tests live in the module's tests/ subdir, matching the existing convention
used by cfn__resource_injection and cognito__attack. pyproject.toml's
testpaths already includes pacu/modules, so pytest / python -m pytest
discovers them automatically. Note: the Makefile test target currently only
points at ./tests and the cfn lambda tests, so make test will not pick these
up as-is — if maintainers prefer, the test target can be widened, but that is
out of scope here.

Security Rationale

Incomplete cloud asset enumeration is a well-documented source of security
blind-spots. An assessor who misses 50 SNS topics in a large AWS account may
overlook notification-triggered Lambda functions, cross-account delivery
channels, or subscriber endpoints that are in scope for privilege-escalation or
data-exfiltration analysis. Pacu is a red-team tool; its enumeration modules are
the foundation on which every subsequent attack module depends. A false-negative
here propagates silently downstream.

This class of bug — assuming a single paginated API response is exhaustive —
maps to incomplete enumeration under MITRE ATT&CK T1526 and is the exact defect
pattern documented in CWE-390 (Detection of Error Condition Without Action) when
the NextToken presence is passively ignored.

Testing / Validation

# Environment: Python 3.14.5, moto 5.2.3, boto3 1.28.85, pytest 9.1.1
# Isolation: fresh venv, no live AWS credentials. Not run against a real AWS account.

$ python -m pytest pacu/modules/sns__enum/tests/test_sns__enum.py -q
5 passed

# Same tests against the main.py currently on master:
FAILED test_list_topics_follows_next_token
FAILED test_list_subscriptions_by_topic_follows_next_token
FAILED test_empty_next_token_terminates_pagination
3 failed, 2 passed

# Full discovery per pyproject.toml testpaths:
$ python -m pytest -q
61 passed

The three pagination tests fail deterministically against the pre-fix code and
pass deterministically against the fix. The two behaviour-preservation tests
(single page, empty region) pass in both directions.

Note: the Makefile test target CI runs points only at ./tests and the cfn
lambda tests (53 of the 61 above), so CI will not execute these tests as-is.
Switching it to python3 -m pytest picks up all 61 and passes clean. Left
unchanged here as a maintainer call.

[DevamShah]

@nobodynate — pushed f5ae539 on top of this. It fixes a defect in my own first commit, plus I went through the rest of pacu/modules for the same pattern and found five more call sites.

Loop guard: an empty NextToken never terminates

My original commit used while "NextToken" in response. That does not terminate when the terminal page carries NextToken: "" instead of omitting the key — which is the same defect class as #503 (secrets__enum infinite loop from broken pagination). An infinite loop in an enum module is strictly worse than the truncation I opened this PR to fix, so I fixed it here rather than trade one bug for another.

Both loops now read while "NextToken" in response and response["NextToken"] != "". That is the existing idiom at pacu/modules/glue__enum/main.py:56 and pacu/modules/transfer_family__enum/main.py:44, and it is equivalent to the next_token = response.get('NextToken') / if not next_token: break form you landed in secrets__enum via #503.

I kept the hand-rolled loop rather than switching to client.get_paginator. Counting main.py files under pacu/modules: 23 hand-roll a raw NextToken/nextToken loop, 9 use get_paginator (two files do both). More to the point, the two modules that already page these exact response shapes — glue__enum and transfer_family__enum — hand-roll it, and so does the secrets__enum fix. Happy to convert to get_paginator if you'd rather standardise the other way, but I didn't want to introduce a third pattern.

What I verified

Regression proof for the new guard, using a fake SNS client whose second page returns NextToken: "", with a 25-call cap so a regression fails fast instead of hanging the suite:

# against ac9f9d3 (this PR's first commit, unguarded loop):
E   AssertionError: Expected exactly 2 list_topics calls, got 26
1 failed

All five tests, patched vs. the main.py currently on master:

$ python -m pytest pacu/modules/sns__enum/tests/test_sns__enum.py -q     # f5ae539
5 passed

$ python -m pytest pacu/modules/sns__enum/tests/test_sns__enum.py -q     # master's main.py
FAILED test_list_topics_follows_next_token
FAILED test_list_subscriptions_by_topic_follows_next_token
FAILED test_empty_next_token_terminates_pagination
3 failed, 2 passed

The two that pass in both directions are the behaviour-preservation tests (single page, empty region).

Full discovery per pyproject.toml testpaths:

$ python -m pytest -q
61 passed, 473 warnings in 4.51s

Environment: Python 3.14.5, moto 5.2.3, boto3 1.28.85, pytest 9.1.1, no live AWS credentials. I have not run this against a real AWS account.

flake8 --max-line-length=160 pacu/modules/sns__enum/tests/ is clean. I also dropped an unused import pytest (F401) I'd left in the test file. main.py still carries pre-existing F401/F841/E275 that I deliberately did not touch — out of scope, and make flake8 doesn't lint this module anyway.

One thing worth flagging: the Makefile test target that CI runs points only at ./tests and the cfn lambda tests — 53 of those 61 — so CI will not execute these tests. Changing it to python3 -m pytest (i.e. honouring testpaths) picks up all 61 and passes clean on my machine today. I left the Makefile alone since it's your call; say the word and I'll add the one-line change here.

The same bug in two other modules

I walked every client.list_* / client.describe_* call site under pacu/modules and cross-checked each against botocore's own model with client.can_paginate(). Five treat a single response as exhaustive on an API that paginates:

call site	API
vpc__enum_lateral_movement/main.py:74	describe_direct_connect_gateways() — unfiltered
vpc__enum_lateral_movement/main.py:81	describe_direct_connect_gateway_associations(directConnectGatewayId=…)
vpc__enum_lateral_movement/main.py:142	describe_vpc_peering_connections() — unfiltered
elasticbeanstalk__enum/main.py:131	describe_environments(ApplicationName=…)
elasticbeanstalk__enum/main.py:149	describe_environments() — unfiltered

All five come back can_paginate=True. I have not measured the server-side page size for any of them, so I can't tell you how many resources it takes to trip each one — but the failure mode is identical to the SNS one: a clean-looking result that silently omits everything past page one, and vpc__enum_lateral_movement in particular feeds pivot analysis. I have not touched any of them. Happy to fix them in this PR or as a separate one, whichever is easier for you to review.

Things I checked and left alone: describe_applications(), describe_vpn_connections(), and describe_vpn_gateways() report can_paginate=False, and describe_application_versions at elasticbeanstalk__enum/main.py:230 is filtered to a single VersionLabels entry, so none of those truncate in practice.

State

The branch is still on top of current master (e597f23) — nothing to rebase. GitHub reports MERGEABLE / BLOCKED with no checks on the branch, which I read as waiting on a maintainer approval rather than anything on my end, but tell me if there's a check I should be triggering.

Would appreciate a review when you have a cycle.