Skip to content

historyarchive: add size bound to GetPathHAS to prevent resource exhaustion#5918

Merged
tamirms merged 2 commits intostellar:mainfrom
tamirms:getpathhas-size-limit
Mar 13, 2026
Merged

historyarchive: add size bound to GetPathHAS to prevent resource exhaustion#5918
tamirms merged 2 commits intostellar:mainfrom
tamirms:getpathhas-size-limit

Conversation

@tamirms
Copy link
Copy Markdown
Contributor

@tamirms tamirms commented Mar 13, 2026

Summary

  • Wraps the reader in GetPathHAS() with io.LimitReader (10 MB cap) before JSON decoding to prevent unbounded memory allocation from malicious/oversized history archive responses
  • Returns a clear error message when the size limit is exceeded
  • Follows the existing pattern from clients/stellartoml/client.go

🤖 Generated with Claude Code

Copilot AI review requested due to automatic review settings March 13, 2026 19:21
…ustion

Wrap the reader with io.LimitReader (10 MB) before JSON decoding to
prevent unbounded memory allocation from malicious history archive
responses. This follows the existing pattern in clients/stellartoml.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@tamirms tamirms force-pushed the getpathhas-size-limit branch from 2dbe54a to bd0c822 Compare March 13, 2026 19:25
@tamirms tamirms requested a review from a team March 13, 2026 19:27
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR hardens history archive state fetching against oversized responses and also introduces an ingestion load-test mode (including snapshot/restore) plus supporting integration-test harness changes.

Changes:

  • Add a 10MB read cap when decoding stellar-history.json in Archive.GetPathHAS().
  • Add Horizon ingestion “load-test” + “load-test-restore” commands, with DB snapshot/save/restore support.
  • Update integration harness/config generation (dynamic peer port) and expand integration tests around load-test flows.

Reviewed changes

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
historyarchive/archive.go Limit HAS JSON read size during decode.
ingest/loadtest/ledger_backend.go Add completion sentinel (ErrLoadTestDone) and locking/cleanup behavior.
services/horizon/cmd/ingest.go Add ingest load-test and ingest load-test-restore commands/flags.
services/horizon/internal/ingest/main.go Add System.LoadTest(...) and terminal-error handling via runOptions.
services/horizon/internal/ingest/fsm.go Gate start/build/resume on pending load-test snapshot checks.
services/horizon/internal/ingest/loadtest.go Implement snapshot save/check + restore logic around load tests.
services/horizon/internal/db2/history/main.go Extend IngestionQ with load-test restore state operations.
services/horizon/internal/db2/history/key_value.go Implement load-test restore state in key-value store.
services/horizon/internal/db2/history/key_value_test.go Add tests for load-test restore state storage semantics.
services/horizon/internal/ingest/*_test.go Update mocks/tests for new load-test snapshot checks and runOptions.
services/horizon/internal/integration/ingestion_load_test.go Add integration coverage for new CLI load-test/restore behavior.
services/horizon/internal/test/integration/core_config.go Add configurable PEER_PORT in captive-core config template.
services/horizon/internal/test/integration/integration.go Generate captive-core configs via helper + choose peer port when unset.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Comment thread historyarchive/archive.go
Comment thread historyarchive/archive.go Outdated
Copy link
Copy Markdown
Contributor

@Shaptic Shaptic left a comment

Choose a reason for hiding this comment

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

I assume the limit is way beyond what would really be needed to check HA state right?

Comment thread historyarchive/archive.go Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@tamirms
Copy link
Copy Markdown
Contributor Author

tamirms commented Mar 13, 2026

I assume the limit is way beyond what would really be needed to check HA state right?

yes, this is what HAS looks like:

https://history.stellar.org/prd/core-testnet/core_testnet_003/.well-known/stellar-history.json

@tamirms tamirms enabled auto-merge (squash) March 13, 2026 21:36
@tamirms tamirms merged commit 580bf40 into stellar:main Mar 13, 2026
11 checks passed
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