Add read-only mode to reject write operations#230
Conversation
| import org.springframework.test.context.TestPropertySource | ||
|
|
||
| @TestPropertySource(properties = ["actionbase.read-only=true"]) | ||
| class ReadOnlyModeStartUpTest : E2ETestBase() { |
There was a problem hiding this comment.
We should also cover the case actionbase.read-only=false, even though other tests cover it implicitly.
There was a problem hiding this comment.
Done in c1fcea1. Added StartUpWithReadOnlyDisabledTest (read-only=false) and a default case test in StartUpTest (read-only not configured).
|
|
||
| @Test | ||
| fun `should allow GET requests on graph v2 paths`() { | ||
| val exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/graph/v2/service/s/label/l/edge")) |
There was a problem hiding this comment.
Please define ALL paths as constants like READ_PATHS = ..., WRITE_PATHS = ... and consolidate the test cases.
There was a problem hiding this comment.
|
Reviewed. Please address the comments. @eazyhozy |
|
@em3s Addressed all 4 feedback items. Ready for re-review. |
| private val log = LoggerFactory.getLogger(ReadOnlyRequestFilter::class.java) | ||
|
|
||
| private val paths = setOf("/graph/v2", "/graph/v3") | ||
| private val readMethods = setOf(HttpMethod.GET, HttpMethod.HEAD, HttpMethod.OPTIONS) |
There was a problem hiding this comment.
We only use GET.
private val readMethods = setOf(HttpMethod.GET) or private val readMethod = HttpMethod.GET
|
|
||
| @ObjectSourceParameterizedTest | ||
| @ObjectSource( | ||
| """ |
There was a problem hiding this comment.
split into get and post cases. (means two tests)
and please add all paths.
There was a problem hiding this comment.
Done in cef984e. Split into should allow GET requests on graph paths and should allow read-only POST requests. Added scan, count, counts, agg paths.
| path: /graph/v3/databases/db/tables/t | ||
| - method: DELETE | ||
| path: /graph/v2/admin/service/test | ||
| - method: PATCH |
|
Please check the updates. |
|
@em3s Merged main and synced endpoint constants — added edges/cache/{cache}, removed POST /graph/v2/query (deleted in main). Force-pushed to fix incorrect committer on my 2 commits. |
05303a5 to
935749c
Compare
Introduce ReadOnlyRequestFilter that blocks mutating HTTP methods (POST, PUT, DELETE, PATCH) on /graph/v2 and /graph/v3 paths when actionbase.read-only=true. Read-only POST endpoints (/edges/get, /multi-edges/ids, /query) are allowlisted. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering>
- Add ReadOnlyModeDisabledStartUpTest (actionbase.read-only=false) - Add default case test in StartUpTest (read-only not configured) Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering>
…urce - Merge ReadOnlyModeStartUpTest and ReadOnlyModeDisabledStartUpTest into StartUpTest.kt - Use @ObjectSourceParameterizedTest for read-only enabled/disabled cases - Add default (not configured) case in StartUpTest - Delete separate test files Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering>
Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering>
…estFilterTest Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering>
…ReadOnlyRequestFilterTest Constants serve as programmatic reference, @ObjectSource YAML serves as independent spec-level assertions. Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering>
- protectedPaths -> paths - mutatingMethods -> readMethods (inverted to allowlist) - readOnlyPostSuffixes -> readSuffixes, isReadOnlyPost -> isRead - Add strategy comment - Early return for allowed cases Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering>
…e PATCH - readMethods set -> readMethod = HttpMethod.GET - Split allowed test into GET and read-only POST cases - Add all graph query paths (scan, count, counts, agg) - Remove PATCH (not used) Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering>
- GET: all v2/v3 query, admin, metadata, datastore endpoints - Read-only POST: /query, /edges/get, /multi-edges/ids - Blocked write: all v2/v3 POST/PUT/DELETE mutation endpoints - Outside protected paths: PUT /graph/health/readiness - readMethods -> readMethod (GET only) - Remove PATCH (not used) - Remove actuator paths from test (no write calls expected) Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering>
…ases Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…lter coverage - Extract EndpointScanner to testFixtures for reusable controller annotation scanning - Replace ObjectSource with MethodSource backed by reflection-scanned endpoints - Declare all endpoints in READ/WRITE/NON_GRAPH constants with verbatim annotation paths - Exhaustiveness test ensures scanned endpoints match declared constants - Add Content-Type: application/json to read-only filter 403 response Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ery) Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering>
935749c to
77bddcf
Compare
|
@em3s Accidentally pushed a broken rebase that included main commits in the PR diff. To fix this, I reset to main and cherry-picked all PR commits preserving original author/committer. Force-pushed as a result. If you have a local checkout, please run: |
- Replace explicit lambda parameter with `it` for cleaner syntax
Summary
Add a read-only mode that rejects write operations at the WebFilter level.
Closes #229
Changes
readOnlyproperty toServerProperties(actionbase.read-only, default:false)ReadOnlyRequestFilterthat blocks non-GET methods on/graph/v2and/graph/v3paths/edges/get,/multi-edges/ids,/query)Usage
To enable read-only mode, set
actionbase.read-onlyin the application profile:Or, to inject via environment variable (e.g. K8s manifest):
No configuration is needed to keep the default behavior (read-only disabled).
How to Test
./gradlew :server:test --tests "*ReadOnlyRequestFilterTest*"— unit tests./gradlew :server:test --tests "*StartUp*"— E2E startup tests./gradlew :server:test— full regression