Release/v1.5#553
Merged
Merged
Conversation
Entire-Checkpoint: bc18358a9343
A second-pass audit surfaced three severe issues missed by the previous review, each a sibling code path of a bug class that was only partially fixed before: - auth: JWT session login (jwtSession.go) registered its authenticator even when CROSS_LOGIN_JWT_HS512_KEY was unset, leaving an empty HMAC key. golang-jwt verifies any HS256/HS512 signature against an empty key, allowing unauthenticated admin token forgery. Init() now refuses to register without a key, with a defense-in-depth empty-key guard in the keyfunc. - repository: metric names from GraphQL ([String!]) were interpolated raw into json_extract(footprint, "$.<name>") SQL. SQLite parses double-quoted strings as literals, enabling SQL injection by any authenticated user. Validate metric names against ^[a-zA-Z0-9_]+$ in jobsMetricStatisticsHistogram and buildFloatJSONCondition. - metricstore: cluster/host line-protocol tags flowed unvalidated into path.Join(RootDir, cluster, host) for checkpoint/WAL files, allowing arbitrary file write outside the checkpoint root via NATS (unauthenticated) or POST /api/write. Reject path-traversal sequences in DecodeLine before the tags become path components. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Entire-Checkpoint: b57246993ec1
Addresses the remaining medium findings from the second-pass audit:
- DoS hardening: bound GraphQL query cost with FixedComplexityLimit, and
reject non-positive items-per-page / page values so uint64 conversion
cannot underflow into an unbounded LIMIT/OFFSET. The -1 "load all"
sentinel stays valid for dashboards; REST now returns 400 for bad input.
- Security headers: add X-Content-Type-Options, X-Frame-Options,
Referrer-Policy and a conservative CSP (frame-ancestors/object-src/
base-uri) that hardens against clickjacking and base-tag injection
without restricting the self-hosted SPA's inline scripts.
- Stored XSS: render job.metaData.message as escaped text instead of
{@html ...} in Job.root and JobFootprint, preserving line breaks via
white-space: pre-wrap.
- SQL injection hardening: parameterize the tag-scope IN list and the
manager project subquery in CountTags instead of interpolating
user.Username / user.Projects (externally sourced via OIDC/LDAP).
- CSRF defense-in-depth: reject cross-site state-changing requests via
Sec-Fetch-Site, failing open for non-browser clients, on top of the
existing SameSite=Lax session cookie.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: de7d47a85c7c
Entire-Checkpoint: 14328c112325
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
No description provided.