Skip to content

Avoid atom creation for activity table lookup#254

Open
BrodoB wants to merge 1 commit into
algora-io:mainfrom
BrodoB:fix-safe-activity-table-lookup
Open

Avoid atom creation for activity table lookup#254
BrodoB wants to merge 1 commit into
algora-io:mainfrom
BrodoB:fix-safe-activity-table-lookup

Conversation

@BrodoB
Copy link
Copy Markdown

@BrodoB BrodoB commented May 12, 2026

Summary

  • Avoid converting activity table names from strings with String.to_atom/1
  • Use precomputed string-keyed allowlist maps for binary lookups
  • Add regression coverage for unknown activity table names

Security
/a/:table_prefix/:activity_id accepts a public path segment. Previously, unknown table names were converted with String.to_atom/1 before the allowlist lookup failed, which could allow unbounded BEAM atom creation from unauthenticated requests.

Testing

  • Focused Docker ExUnit harness: 2 tests, 0 failures

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented May 12, 2026

CLA assistant check
All committers have signed the CLA.

@nagiexplorer88
Copy link
Copy Markdown

This avoids creating atoms, but the public /a/:table_prefix/:activity_id path can still turn unknown table names into a KeyError. ActivityController.get/2 calls Activities.assoc_url("#{table}_activities", id), and schema_from_table/1 now uses Map.fetch!(@schema_from_table_by_name, name), so an unknown public path segment raises instead of returning {:error, :not_found} for the controller to handle.

I think the lookup should use Map.fetch (or an allowlist guard before assoc_url) and return a normal not-found tuple/404 for unsupported table names. The current regression test proves no atom is created, but it also locks in the exception behavior, so it would not catch public requests still producing 500s/noisy exception logs.

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