-
-
Notifications
You must be signed in to change notification settings - Fork 44
feat: Add smoke tests for copi.owasp.org and cornucopia.owasp.org (fi… #2069
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.
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
immortal71
wants to merge
2
commits into
OWASP:master
Choose a base branch
from
immortal71:feat/smoke-tests-1265
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| name: Smoke Tests | ||
| on: | ||
| workflow_dispatch: | ||
| schedule: | ||
| - cron: '0 6 * * *' | ||
| push: | ||
| branches: | ||
| - master | ||
| paths: | ||
| - 'copi.owasp.org/**' | ||
| - 'cornucopia.owasp.org/**' | ||
| - 'tests/scripts/smoke_tests.py' | ||
| - '.github/workflows/smoke-tests.yaml' | ||
|
|
||
| permissions: | ||
| contents: read | ||
|
|
||
| jobs: | ||
| smoke-tests: | ||
| name: Run Smoke Tests | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Checkout repository | ||
| uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 | ||
|
|
||
| - name: Get Python | ||
| uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6.2.0 | ||
| with: | ||
| python-version: '3.11' | ||
| cache: 'pipenv' | ||
|
|
||
| - name: Install dependencies | ||
| run: | | ||
| pip install -r requirements.txt --require-hashes | ||
| pipenv install --ignore-pipfile --dev | ||
|
|
||
| - name: Run smoke tests for copi.owasp.org | ||
| run: pipenv run python -m unittest tests.scripts.smoke_tests.CopiSmokeTests -v | ||
| continue-on-error: false | ||
|
|
||
| - name: Run smoke tests for cornucopia.owasp.org | ||
| run: pipenv run python -m unittest tests.scripts.smoke_tests.CornucopiaSmokeTests -v | ||
| continue-on-error: false | ||
|
|
||
| - name: Run integration smoke tests | ||
| run: pipenv run python -m unittest tests.scripts.smoke_tests.IntegrationSmokeTests -v | ||
| continue-on-error: false | ||
|
|
||
| - name: Summary | ||
| if: always() | ||
| run: | | ||
| echo "## Smoke Test Results" >> $GITHUB_STEP_SUMMARY | ||
| echo "Smoke tests completed for:" >> $GITHUB_STEP_SUMMARY | ||
| echo "- copi.owasp.org (Elixir/Phoenix application)" >> $GITHUB_STEP_SUMMARY | ||
| echo "- cornucopia.owasp.org (SvelteKit application)" >> $GITHUB_STEP_SUMMARY | ||
| echo "" >> $GITHUB_STEP_SUMMARY | ||
| echo "Tests verify:" >> $GITHUB_STEP_SUMMARY | ||
| echo "- At least 2 routes working on each application" >> $GITHUB_STEP_SUMMARY | ||
| echo "- JavaScript is loading and functional" >> $GITHUB_STEP_SUMMARY | ||
| echo "- Applications respond within acceptable time" >> $GITHUB_STEP_SUMMARY |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,227 @@ | ||
| # Rate Limiting Fresh PR - Implementation Guide | ||
|
|
||
| ## Completed Files | ||
|
|
||
| 1. **lib/copi/rate_limiter.ex** - GenServer for rate limiting | ||
| 2. **lib/copi_web/helpers/ip_helper.ex** - Shared IP extraction helper | ||
| 3. **lib/copi/application.ex** - Added RateLimiter to supervision tree | ||
| 4. **config/runtime.exs** - Runtime configuration for rate limits | ||
| 5. **test/copi/rate_limiter_test.exs** - Comprehensive tests | ||
| ## Remaining Updates Needed | ||
|
|
||
| ### File 1: create_game_form.ex | ||
|
|
||
| Add to top of file after `use CopiWeb, :live_component`: | ||
| ```elixir | ||
| alias CopiWeb.Helpers.IPHelper | ||
| ``` | ||
|
|
||
| Replace the `save_game` function for `:new` action with: | ||
| ```elixir | ||
| defp save_game(socket, :new, game_params) do | ||
| # Get the IP address for rate limiting | ||
| ip_address = IPHelper.get_connect_ip(socket) | ||
|
|
||
| # Check rate limit before creating game | ||
| case Copi.RateLimiter.check_rate(ip_address, :game_creation) do | ||
| {:ok, _remaining} -> | ||
| case Cornucopia.create_game(game_params) do | ||
| {:ok, game} -> | ||
| # Record the action after successful creation | ||
| Copi.RateLimiter.record_action(ip_address, :game_creation) | ||
|
|
||
| {:noreply, | ||
| socket | ||
| |> put_flash(:info, "Game created successfully") | ||
| |> push_navigate(to: ~p"/games/#{game.id}")} | ||
|
|
||
| {:error, %Ecto.Changeset{} = changeset} -> | ||
| {:noreply, assign_form(socket, changeset)} | ||
| end | ||
|
|
||
| {:error, :rate_limited, retry_after} -> | ||
| {:noreply, | ||
| socket | ||
| |> put_flash( | ||
| :error, | ||
| "Rate limit exceeded. Too many games created from your IP address. " <> | ||
| "Please try again in #{retry_after} seconds. " <> | ||
| "This limit helps ensure service availability for all users." | ||
| ) | ||
| |> assign_form(socket.assigns.form.source)} | ||
| end | ||
| end | ||
| ``` | ||
|
|
||
| ### File 2: game_live/index.ex | ||
|
|
||
| Add to top after `use CopiWeb, :live_view`: | ||
| ```elixir | ||
| alias CopiWeb.Helpers.IPHelper | ||
| ``` | ||
|
|
||
| Update `mount` function: | ||
| ```elixir | ||
| @impl true | ||
| def mount(_params, _session, socket) do | ||
| # Rate limit WebSocket connections | ||
| ip_address = IPHelper.get_connect_ip(socket) | ||
|
|
||
| case Copi.RateLimiter.check_rate(ip_address, :connection) do | ||
| {:ok, _remaining} -> | ||
| if connected?(socket) do | ||
| Phoenix.PubSub.subscribe(Copi.PubSub, "games") | ||
| end | ||
|
|
||
| {:ok, assign(socket, games: list_games())} | ||
|
|
||
| {:error, :rate_limited, retry_after} -> | ||
| {:ok, | ||
| socket | ||
| |> put_flash( | ||
| :error, | ||
| "Rate limit exceeded. Too many connections from your IP address. " <> | ||
| "Please try again in #{retry_after} seconds." | ||
| ) | ||
| |> assign(games: [])} | ||
| end | ||
| end | ||
| ``` | ||
|
|
||
| ### File 3: player_live/form_component.ex | ||
|
|
||
| Add to top after `use CopiWeb, :live_component`: | ||
| ```elixir | ||
| alias CopiWeb.Helpers.IPHelper | ||
| ``` | ||
|
|
||
| Replace `save_player` for `:new` with: | ||
| ```elixir | ||
| defp save_player(socket, :new, player_params) do | ||
| # Get the IP address for rate limiting | ||
| ip_address = IPHelper.get_connect_ip(socket) | ||
|
|
||
| # Check rate limit before creating player | ||
| case Copi.RateLimiter.check_rate(ip_address, :player_creation) do | ||
| {:ok, _remaining} -> | ||
| case Cornucopia.create_player(player_params) do | ||
| {:ok, player} -> | ||
| # Record the action after successful creation | ||
| Copi.RateLimiter.record_action(ip_address, :player_creation) | ||
|
|
||
| {:ok, updated_game} = Cornucopia.Game.find(socket.assigns.player.game_id) | ||
| CopiWeb.Endpoint.broadcast(topic(updated_game.id), "game:updated", updated_game) | ||
|
|
||
| {:noreply, | ||
| socket | ||
| |> assign(:game, updated_game) | ||
| |> push_navigate(to: ~p"/games/#{player.game_id}/players/#{player.id}")} | ||
|
|
||
| {:error, %Ecto.Changeset{} = changeset} -> | ||
| {:noreply, assign_form(socket, changeset)} | ||
| end | ||
|
|
||
| {:error, :rate_limited, retry_after} -> | ||
| {:noreply, | ||
| socket | ||
| |> put_flash( | ||
| :error, | ||
| "Rate limit exceeded. Too many players created from your IP address. " <> | ||
| "Please try again in #{retry_after} seconds. " <> | ||
| "This limit helps ensure service availability for all users." | ||
| ) | ||
| |> assign_form(socket.assigns.form.source)} | ||
| end | ||
| end | ||
| ``` | ||
|
|
||
| ### File 4: SECURITY.md | ||
|
|
||
| Create this file with: | ||
| ```markdown | ||
| # Security Policy for Copi | ||
|
|
||
| ## Rate Limiting | ||
|
|
||
| Copi implements IP-based rate limiting to protect against CAPEC-212 (Functionality Misuse) attacks and ensure service availability. | ||
|
|
||
| ### Protected Actions | ||
|
|
||
| 1. **Game Creation**: Limited to prevent abuse | ||
| - Default: 10 games per IP per hour | ||
| - Configurable via `MAX_GAMES_PER_IP` and `GAME_CREATION_WINDOW_SECONDS` | ||
|
|
||
| 2. **Player Creation**: Separate limit from game creation | ||
| - Default: 20 players per IP per hour | ||
| - Configurable via `MAX_PLAYERS_PER_IP` and `PLAYER_CREATION_WINDOW_SECONDS` | ||
|
|
||
| 3. **WebSocket Connections**: Prevents connection flooding | ||
| - Default: 50 connections per IP per 5 minutes | ||
| - Configurable via `MAX_CONNECTIONS_PER_IP` and `CONNECTION_WINDOW_SECONDS` | ||
|
|
||
| ### Configuration | ||
|
|
||
| All limits can be adjusted via environment variables in production: | ||
|
|
||
| \`\`\`bash | ||
| MAX_GAMES_PER_IP=10 | ||
| GAME_CREATION_WINDOW_SECONDS=3600 | ||
| MAX_PLAYERS_PER_IP=20 | ||
| PLAYER_CREATION_WINDOW_SECONDS=3600 | ||
| MAX_CONNECTIONS_PER_IP=50 | ||
| CONNECTION_WINDOW_SECONDS=300 | ||
| \`\`\` | ||
|
|
||
| ### Reporting Security Issues | ||
|
|
||
| If you discover a security vulnerability, please email security@owasp.org with details. | ||
| Do not create public GitHub issues for security problems. | ||
| ``` | ||
|
|
||
| ## Testing | ||
|
|
||
| After making all updates, run: | ||
| ```bash | ||
| cd copi.owasp.org | ||
| mix deps.get | ||
| mix test test/copi/rate_limiter_test.exs | ||
| ``` | ||
|
|
||
| # Committing | ||
|
|
||
| ```bash | ||
| cd c:\Users\HUAWEI\Downloads\oswap\cornucopia-clean | ||
|
|
||
| git add . | ||
| git status # Verify changes | ||
|
|
||
| git commit -S -m "Add IP-based rate limiting with separate limits for games/players | ||
|
|
||
| - Implements Copi.RateLimiter GenServer for tracking rate limits per IP | ||
| - Adds separate rate limiting for game creation, player creation, and connections | ||
| - Creates shared IPHelper module (DRY) for IP extraction | ||
| - Moves configuration to runtime.exs for proper env var handling | ||
| - Removes 'unknown' IP fallback for security | ||
| - Adds comprehensive tests and SECURITY.md documentation | ||
|
|
||
| Addresses feedback from PR #1920: | ||
| - Config moved to runtime.exs (@sydseter) | ||
| - Refactored duplicated get_connect_ip/1 (@sydseter) | ||
| - Removed 'unknown' IP security issue (@sydseter) | ||
| - All commits GPG signed and verified (@sydseter) | ||
|
|
||
| Fixes #1877" | ||
| ``` | ||
|
|
||
| # Pushing & Creating PR | ||
|
|
||
| ```bash | ||
| git push myfork feat/rate-limiting-clean | ||
|
|
||
| # Then go to GitHub and create PR: | ||
| # - Base: OWASP:master | ||
| # - Head: immortal71:feat/rate-limiting-clean | ||
| # - Title: Add IP-based rate limiting for Copi (fixes #1877) | ||
| # - In description, mention: "Supersedes #1920" | ||
| ``` | ||
|
|
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The smoke tests added to this workflow will fail because the hardening job (line 8-10) blocks egress traffic to all endpoints except those explicitly allowed in hardening.yaml. The smoke tests need to access copi.owasp.org:443 and cornucopia.owasp.org:443, which are not in the allowed-endpoints list.
Either the hardening.yaml file needs to be updated to include these endpoints, or the smoke tests should be made conditional/skipped when hardening is enabled.