Support component#1044
Open
Gerviba wants to merge 3 commits into
Open
Conversation
📝 WalkthroughWalkthroughAdds a support-ticket system with backend entities, service, API, admin UI, statistics, and frontend pages; updates frontend runtime environment injection in the container; and removes Nova integration while changing repository iteration types to read-only ChangesSupport Module
Frontend Docker Runtime Env Config
Minor Cleanup
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
🧹 Nitpick comments (3)
backend/src/main/kotlin/hu/bme/sch/cmsch/component/support/SupportThreadEntity.kt (1)
25-29: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAdd an index for
userInternalIdlookups.
SupportService.countOpenThreadsForUser()andgetThreadsForUser()both filter onuserInternalId, butsupportThreadsonly indexesuuid,userEmail, andstatus. Once this table grows, every authenticated support action will scan it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/main/kotlin/hu/bme/sch/cmsch/component/support/SupportThreadEntity.kt` around lines 25 - 29, Add an index for userInternalId on the SupportThreadEntity table mapping, since SupportService.countOpenThreadsForUser() and getThreadsForUser() filter by that field and currently only uuid, userEmail, and status are indexed. Update the `@Table`(indexes = ...) definition in SupportThreadEntity to include a dedicated index for userInternalId so those lookups can use an index instead of scanning supportThreads.backend/src/main/resources/templates/settings.html (1)
59-59: 🔒 Security & Privacy | 🔵 Trivial | 🏗️ Heavy liftAvoid making each extension hand-escape raw HTML.
th:utextrenders every extension form as trusted HTML, so one missed escape in anAdminSettingsExtensionbecomes stored/reflected XSS on the settings page. Prefer a typed fragment/model contract, or centralize sanitization/escaping at this boundary.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/main/resources/templates/settings.html` at line 59, The settings template is rendering extension-provided form markup as trusted HTML via the settings page boundary, which risks XSS if an AdminSettingsExtension supplies unsafe content. Update the settings rendering contract around ext.formHtml in the settings.html template and the related AdminSettingsExtension flow so the HTML is either produced from a typed fragment/model or centrally sanitized before display, instead of relying on each extension to escape it correctly.frontend/src/pages/support/supportThread.page.tsx (1)
49-52: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winCheck whether the extra
refetch()is now redundant.
useAddSupportMessageMutationalready invalidates the thread query on success. IfuseSupportThreadQueryuses the same key prefix, keepingrefetch()here will fire a second GET after every reply for no benefit.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/pages/support/supportThread.page.tsx` around lines 49 - 52, The reply success handler in supportThread.page.tsx appears to trigger a redundant refresh because useAddSupportMessageMutation already invalidates the thread query. Review the onSuccess logic in supportThread.page.tsx and the related useSupportThreadQuery/useAddSupportMessageMutation hooks, then remove the explicit refetch() call if the invalidation covers the same query key so the thread reloads only once after sending a reply.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@backend/src/main/kotlin/hu/bme/sch/cmsch/component/support/SupportAdminSettingsExtension.kt`:
- Around line 24-33: In getCardFormHtml, stop calling userEntity.get() directly
and use the Optional safely when deriving the placeholder. Pull the fullName
from the mapped user only when present, otherwise fall back to an empty string
or another safe default, and run it through escapeHtml before inserting it into
the rendered HTML. Keep the currentName handling unchanged, but make the
placeholder generation follow the same null-safe pattern as the existing
userEntity mapping.
In
`@backend/src/main/kotlin/hu/bme/sch/cmsch/component/support/SupportApiController.kt`:
- Around line 80-98: The public thread creation flow in
SupportApiController.createThread validates title and author, but it still
allows blank or oversized initial content to be saved. Apply the same
message-content validation used by addMessage before calling
supportService.createThread, and reject invalid request.content for both
authenticated and anonymous paths so the initial post cannot bypass content
limits.
- Around line 127-135: The thread detail endpoint in SupportApiController
currently exposes SupportThreadView and messages to anyone who has the UUID,
because secretMatches/isOwner only gates canReply. Update the handler to require
either a valid secret or ownership before assembling the response, and return
not found/forbidden otherwise; only after that should it call
supportService.getPublicMessagesForThread and build ThreadDetailView from
SupportThreadView.from(thread).
In
`@backend/src/main/kotlin/hu/bme/sch/cmsch/component/support/SupportComponent.kt`:
- Around line 70-80: Require a non-empty `incomingEmailSecret` whenever
`emailWebhookEnabled` is enabled in `SupportComponent`; the current
`BooleanSettingRef`/`StringSettingRef` setup allows the inbound-email webhook to
be active without any secret. Update the
`emailWebhookEnabled`/`incomingEmailSecret` settings handling so enabling the
webhook is rejected or automatically blocked unless `incomingEmailSecret` is
set, and keep `allowedSenderHostRegex` as an independent check rather than
relying on it for request authenticity.
In
`@backend/src/main/kotlin/hu/bme/sch/cmsch/component/support/SupportService.kt`:
- Around line 46-49: `findMatchingThread()` currently matches replies by
normalized subject and sender email, which can route a reply to the wrong open
ticket when multiple threads share the same subject. Update the reply lookup
flow in `SupportService` to use a stable ticket/thread identifier from the
inbound message instead of relying on `normalizeSubject()` matching; if that
identifier is unavailable, do not auto-attach by subject alone.
- Around line 101-149: Move the customer-message validation into SupportService
so processIncomingEmail() cannot bypass the public endpoint protections. In
createThread() and addCustomerMessage(), enforce the same checks currently done
in SupportApiController.kt, including isBlockedUser, maxOpenThreads,
maxResponseLength, and maxCustomerResponsesWithoutAnswer, before persisting
SupportThreadEntity or SupportMessageEntity. Keep the guard logic centralized in
SupportService and have the webhook path reuse it so both createThread() and
addCustomerMessage() reject the same invalid customer input.
- Around line 226-230: The sender host allowlist in SupportService should use
whole-host matching instead of substring matching, since containsMatchIn() can
wrongly accept hosts like evil-example.com. Update the host check in the email
rejection logic to compare the extracted host against the configured regex as a
full match, keeping the existing hostRegex, fromEmail, and log warning flow
intact.
In `@backend/src/main/resources/templates/supportThreadView.html`:
- Around line 67-70: The support thread view is rendering the current date
instead of the actual last customer reply time because the template uses
`#temporals.createNow()` in the `thread-meta-item` block. Update
`supportThreadView.html` to display the value derived from
`thread.lastCustomerAnswerAt`, and if formatting is needed, compute
`lastCustomerAnswerAtFormatted` in the controller and bind that in the template
using the existing `thread.lastCustomerAnswerAt`/`lastCustomerAnswerAtFormatted`
symbols.
In `@frontend/entrypoint.sh`:
- Around line 15-35: The env-config.js generation in entrypoint.sh is building
window.__env__ with raw shell interpolation, so values like VITE_NAME and
VITE_DESCRIPTION can break the JavaScript payload. Update the env-config.js
creation logic to serialize the object with proper JSON encoding instead of
manual string concatenation, and keep the same keys so
frontend/src/util/configs/environment.config.ts can read the runtime values
reliably.
- Around line 22-23: The TTL default for app config cache is inconsistent
between the shell entrypoint and environment.config.ts, causing the container
path to override the intended value. Update the
VITE_APP_CONFIG_CACHE_TTL_SECONDS default in entrypoint.sh to match the fallback
in frontend/src/util/configs/environment.config.ts, or remove the shell default
so the TypeScript config remains the single source of truth. Use the
VITE_APP_CONFIG_CACHE_TTL_SECONDS setting and the environment.config.ts default
as the symbols to align.
- Around line 10-12: The HTML rewrite in entrypoint.sh uses a predictable
/tmp/index.html path, which should be replaced with a unique temporary file.
Update the envsubst/mv flow to create a temp file with mktemp, write the
rendered HTML there, and then move it into $NGINX_HTML/index.html so the startup
path in the shell script is not vulnerable to pre-created symlinks.
In `@frontend/src/api/hooks/support/useAddSupportMessageMutation.ts`:
- Around line 19-22: The axios.post call in useAddSupportMessageMutation is not
formatted the way Prettier expects. Update the request in
useAddSupportMessageMutation so the axios.post invocation is formatted inline
according to the formatter output, keeping the same arguments and behavior while
matching the project’s linting style.
In `@frontend/src/api/hooks/support/useSupportThreadQuery.ts`:
- Around line 7-16: The support-thread query cache in useSupportThreadQuery is
only keyed by QueryKeys.SUPPORT_THREAD, uuid, and secret, so authenticated
threads can be reused across account/session changes. Update the queryKey to
include a stable auth/session discriminator (for example from the current user
or auth state) or ensure support-thread queries are invalidated/cleared when
auth changes, so cached thread content cannot leak between accounts.
In `@frontend/src/api/hooks/support/useSupportThreadsQuery.ts`:
- Around line 7-15: The SUPPORT_THREADS query is currently using a global cache
key in useSupportThreadsQuery, which can expose the prior user’s cached threads
after an auth switch. Update the queryKey in useSupportThreadsQuery to include a
stable authenticated-user identifier from the current auth context, or add
explicit cache clearing on auth changes so cached data is scoped per user.
Ensure the change is applied where QueryKeys.SUPPORT_THREADS is used in the
hook.
In `@frontend/src/pages/support/supportList.page.tsx`:
- Around line 55-58: The support thread link in SupportList.page.tsx is exposing
the thread recovery token by appending uuidSecret to authenticated URLs. Update
the Link in the support list rendering so it uses only the plain thread path
with thread.uuid, and keep uuidSecret reserved for anonymous handoff flows
handled elsewhere.
In `@frontend/src/pages/support/supportNew.page.tsx`:
- Around line 100-125: The supportNew.page.tsx UI currently hides the
create-thread form behind isLoggedIn, which blocks the guest flow even though
handleSubmit already accepts anonymous authorEmail and a public create-thread
endpoint exists. Update the conditional rendering in supportNew.page.tsx so
logged-out users can see and submit the form, and make sure the guest path still
collects/passes the email needed by handleSubmit while keeping the logged-in
experience unchanged.
In `@frontend/src/util/configs/environment.config.ts`:
- Line 5: The APP_CONFIG_CACHE_TTL_SECONDS assignment in environment.config.ts
exceeds the 140-character lint limit; reformat that constant declaration across
multiple lines so the long Number(...) fallback expression is wrapped cleanly.
Keep the same logic and symbols (APP_CONFIG_CACHE_TTL_SECONDS,
env.VITE_APP_CONFIG_CACHE_TTL_SECONDS, and
import.meta.env.VITE_APP_CONFIG_CACHE_TTL_SECONDS) while splitting the
expression to satisfy the linter.
In `@frontend/src/util/views/support.view.ts`:
- Around line 20-23: The public support thread list DTO is exposing
sensitive/internal fields through SupportThreadEntity, specifically uuidSecret
and userInternalId. Split the contract so ThreadListResponse (and the public
SupportThreadView path) only returns non-sensitive thread data, and introduce a
separate internal/staff DTO for code that needs uuidSecret or userInternalId;
update any consumers of SupportThreadEntity accordingly.
---
Nitpick comments:
In
`@backend/src/main/kotlin/hu/bme/sch/cmsch/component/support/SupportThreadEntity.kt`:
- Around line 25-29: Add an index for userInternalId on the SupportThreadEntity
table mapping, since SupportService.countOpenThreadsForUser() and
getThreadsForUser() filter by that field and currently only uuid, userEmail, and
status are indexed. Update the `@Table`(indexes = ...) definition in
SupportThreadEntity to include a dedicated index for userInternalId so those
lookups can use an index instead of scanning supportThreads.
In `@backend/src/main/resources/templates/settings.html`:
- Line 59: The settings template is rendering extension-provided form markup as
trusted HTML via the settings page boundary, which risks XSS if an
AdminSettingsExtension supplies unsafe content. Update the settings rendering
contract around ext.formHtml in the settings.html template and the related
AdminSettingsExtension flow so the HTML is either produced from a typed
fragment/model or centrally sanitized before display, instead of relying on each
extension to escape it correctly.
In `@frontend/src/pages/support/supportThread.page.tsx`:
- Around line 49-52: The reply success handler in supportThread.page.tsx appears
to trigger a redundant refresh because useAddSupportMessageMutation already
invalidates the thread query. Review the onSuccess logic in
supportThread.page.tsx and the related
useSupportThreadQuery/useAddSupportMessageMutation hooks, then remove the
explicit refetch() call if the invalidation covers the same query key so the
thread reloads only once after sending a reply.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c3e413c0-d93d-46e4-b1cb-5a41ad2961cc
📒 Files selected for processing (45)
backend/src/main/kotlin/hu/bme/sch/cmsch/addon/nova/NovaIntegrationController.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/addon/nova/NovaIntegrationService.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/debt/SoldProductEntity.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/location/LocationService.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/support/SupportAdminController.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/support/SupportAdminSettingsExtension.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/support/SupportApiController.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/support/SupportComponent.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/support/SupportComponentController.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/support/SupportComponentEntityConfiguration.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/support/SupportMessageEntity.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/support/SupportMessageRepository.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/support/SupportService.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/support/SupportStatsDashboard.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/support/SupportThreadEntity.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/support/SupportThreadRepository.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/support/SupportThreadViewExtension.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/support/dto/IncomingEmailDto.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/controller/admin/AdminSettingsExtension.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/controller/admin/SettingsController.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/dto/SiteContext.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/repository/EntityPageDataSource.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/repository/ManualRepository.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/service/PermissionsService.ktbackend/src/main/resources/config/application.propertiesbackend/src/main/resources/templates/settings.htmlbackend/src/main/resources/templates/supportThreadView.htmlfrontend/Dockerfilefrontend/entrypoint.shfrontend/index.htmlfrontend/public/env-config.jsfrontend/src/App.tsxfrontend/src/api/contexts/config/types.tsfrontend/src/api/hooks/queryKeys.tsfrontend/src/api/hooks/support/useAddSupportMessageMutation.tsfrontend/src/api/hooks/support/useCreateSupportThreadMutation.tsfrontend/src/api/hooks/support/useSupportThreadQuery.tsfrontend/src/api/hooks/support/useSupportThreadsQuery.tsfrontend/src/pages/support/supportList.page.tsxfrontend/src/pages/support/supportNew.page.tsxfrontend/src/pages/support/supportThread.page.tsxfrontend/src/util/configs/environment.config.tsfrontend/src/util/paths.tsfrontend/src/util/views/support.view.tsfrontend/src/vite-env.d.ts
💤 Files with no reviewable changes (2)
- backend/src/main/kotlin/hu/bme/sch/cmsch/addon/nova/NovaIntegrationController.kt
- backend/src/main/kotlin/hu/bme/sch/cmsch/addon/nova/NovaIntegrationService.kt
Comment on lines
+24
to
+33
| override fun getCardFormHtml(user: CmschUser): String { | ||
| val userEntity = userRepository.findByInternalId(user.internalId) | ||
| val currentName = userEntity | ||
| .map { userService.resolveConfig(it.config).supportDefaultName } | ||
| .orElse("") | ||
| return """ | ||
| <form action="/admin/control/support/settings/default-name" method="post"> | ||
| <div class="field-group"> | ||
| <label>Megjelenített neved</label> | ||
| <input type="text" name="supportDefaultName" value="${escapeHtml(currentName)}" placeholder="${userEntity.get().fullName}" maxlength="255"> |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Escape the placeholder and avoid Optional.get().
currentName handles a missing user, but Line 33 immediately calls userEntity.get() and injects fullName into raw HTML rendered by th:utext.
Proposed fix
val currentName = userEntity
.map { userService.resolveConfig(it.config).supportDefaultName }
.orElse("")
+ val placeholderName = userEntity
+ .map { escapeHtml(it.fullName) }
+ .orElse("")
return """
<form action="/admin/control/support/settings/default-name" method="post">
<div class="field-group">
<label>Megjelenített neved</label>
- <input type="text" name="supportDefaultName" value="${escapeHtml(currentName)}" placeholder="${userEntity.get().fullName}" maxlength="255">
+ <input type="text" name="supportDefaultName" value="${escapeHtml(currentName)}" placeholder="$placeholderName" maxlength="255">
</div>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| override fun getCardFormHtml(user: CmschUser): String { | |
| val userEntity = userRepository.findByInternalId(user.internalId) | |
| val currentName = userEntity | |
| .map { userService.resolveConfig(it.config).supportDefaultName } | |
| .orElse("") | |
| return """ | |
| <form action="/admin/control/support/settings/default-name" method="post"> | |
| <div class="field-group"> | |
| <label>Megjelenített neved</label> | |
| <input type="text" name="supportDefaultName" value="${escapeHtml(currentName)}" placeholder="${userEntity.get().fullName}" maxlength="255"> | |
| override fun getCardFormHtml(user: CmschUser): String { | |
| val userEntity = userRepository.findByInternalId(user.internalId) | |
| val currentName = userEntity | |
| .map { userService.resolveConfig(it.config).supportDefaultName } | |
| .orElse("") | |
| val placeholderName = userEntity | |
| .map { escapeHtml(it.fullName) } | |
| .orElse("") | |
| return """ | |
| <form action="/admin/control/support/settings/default-name" method="post"> | |
| <div class="field-group"> | |
| <label>Megjelenített neved</label> | |
| <input type="text" name="supportDefaultName" value="${escapeHtml(currentName)}" placeholder="$placeholderName" maxlength="255"> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@backend/src/main/kotlin/hu/bme/sch/cmsch/component/support/SupportAdminSettingsExtension.kt`
around lines 24 - 33, In getCardFormHtml, stop calling userEntity.get() directly
and use the Optional safely when deriving the placeholder. Pull the fullName
from the mapped user only when present, otherwise fall back to an empty string
or another safe default, and run it through escapeHtml before inserting it into
the rendered HTML. Keep the currentName handling unchanged, but make the
placeholder generation follow the same null-safe pattern as the existing
userEntity mapping.
Comment on lines
+80
to
+98
| if (request.title.isBlank()) return ResponseEntity.badRequest().build() | ||
| val user = auth?.getUserOrNull() | ||
| return if (user != null && supportComponent.minRole.isAvailableForRole(user.role)) { | ||
| val email = getEmail(user) | ||
| if (supportService.isBlockedUser(user.internalId, email)) | ||
| return ResponseEntity.status(HttpStatus.FORBIDDEN).build() | ||
| if (supportService.countOpenThreadsForUser(user.internalId, "") >= supportComponent.maxOpenThreads) | ||
| return ResponseEntity.status(HttpStatus.TOO_MANY_REQUESTS).build() | ||
| ResponseEntity.ok(supportService.createThread(request.title, request.content, user.internalId, email, user.userName)) | ||
| } else { | ||
| if (request.authorEmail.isBlank()) return ResponseEntity.badRequest().build() | ||
| if (supportService.isBlockedUser("", request.authorEmail)) | ||
| return ResponseEntity.status(HttpStatus.FORBIDDEN).build() | ||
| if (supportService.countOpenThreadsForUser("", request.authorEmail) >= supportComponent.maxOpenThreads) | ||
| return ResponseEntity.status(HttpStatus.TOO_MANY_REQUESTS).build() | ||
| ResponseEntity.ok(supportService.createThread( | ||
| request.title, request.content, "", request.authorEmail, | ||
| request.authorName.ifBlank { request.authorEmail } | ||
| )) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Apply the message validation to initial thread content too.
addMessage rejects blank/oversized content, but createThread can persist an empty or oversized initial message through the public endpoint.
Proposed fix
fun createThread(`@RequestBody` request: CreateThreadRequest, auth: Authentication?): ResponseEntity<SupportThreadEntity> {
- if (request.title.isBlank()) return ResponseEntity.badRequest().build()
+ if (request.title.isBlank() || request.content.isBlank()) return ResponseEntity.badRequest().build()
+ if (supportService.isContentTooLong(request.content))
+ return ResponseEntity.status(HttpStatus.PAYLOAD_TOO_LARGE).build()
val user = auth?.getUserOrNull()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (request.title.isBlank()) return ResponseEntity.badRequest().build() | |
| val user = auth?.getUserOrNull() | |
| return if (user != null && supportComponent.minRole.isAvailableForRole(user.role)) { | |
| val email = getEmail(user) | |
| if (supportService.isBlockedUser(user.internalId, email)) | |
| return ResponseEntity.status(HttpStatus.FORBIDDEN).build() | |
| if (supportService.countOpenThreadsForUser(user.internalId, "") >= supportComponent.maxOpenThreads) | |
| return ResponseEntity.status(HttpStatus.TOO_MANY_REQUESTS).build() | |
| ResponseEntity.ok(supportService.createThread(request.title, request.content, user.internalId, email, user.userName)) | |
| } else { | |
| if (request.authorEmail.isBlank()) return ResponseEntity.badRequest().build() | |
| if (supportService.isBlockedUser("", request.authorEmail)) | |
| return ResponseEntity.status(HttpStatus.FORBIDDEN).build() | |
| if (supportService.countOpenThreadsForUser("", request.authorEmail) >= supportComponent.maxOpenThreads) | |
| return ResponseEntity.status(HttpStatus.TOO_MANY_REQUESTS).build() | |
| ResponseEntity.ok(supportService.createThread( | |
| request.title, request.content, "", request.authorEmail, | |
| request.authorName.ifBlank { request.authorEmail } | |
| )) | |
| if (request.title.isBlank() || request.content.isBlank()) return ResponseEntity.badRequest().build() | |
| if (supportService.isContentTooLong(request.content)) | |
| return ResponseEntity.status(HttpStatus.PAYLOAD_TOO_LARGE).build() | |
| val user = auth?.getUserOrNull() | |
| return if (user != null && supportComponent.minRole.isAvailableForRole(user.role)) { | |
| val email = getEmail(user) | |
| if (supportService.isBlockedUser(user.internalId, email)) | |
| return ResponseEntity.status(HttpStatus.FORBIDDEN).build() | |
| if (supportService.countOpenThreadsForUser(user.internalId, "") >= supportComponent.maxOpenThreads) | |
| return ResponseEntity.status(HttpStatus.TOO_MANY_REQUESTS).build() | |
| ResponseEntity.ok(supportService.createThread(request.title, request.content, user.internalId, email, user.userName)) | |
| } else { | |
| if (request.authorEmail.isBlank()) return ResponseEntity.badRequest().build() | |
| if (supportService.isBlockedUser("", request.authorEmail)) | |
| return ResponseEntity.status(HttpStatus.FORBIDDEN).build() | |
| if (supportService.countOpenThreadsForUser("", request.authorEmail) >= supportComponent.maxOpenThreads) | |
| return ResponseEntity.status(HttpStatus.TOO_MANY_REQUESTS).build() | |
| ResponseEntity.ok(supportService.createThread( | |
| request.title, request.content, "", request.authorEmail, | |
| request.authorName.ifBlank { request.authorEmail } | |
| )) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@backend/src/main/kotlin/hu/bme/sch/cmsch/component/support/SupportApiController.kt`
around lines 80 - 98, The public thread creation flow in
SupportApiController.createThread validates title and author, but it still
allows blank or oversized initial content to be saved. Apply the same
message-content validation used by addMessage before calling
supportService.createThread, and reject invalid request.content for both
authenticated and anonymous paths so the initial post cannot bypass content
limits.
Comment on lines
+127
to
+135
| val thread = supportService.findByUuid(uuid) ?: return ResponseEntity.notFound().build() | ||
|
|
||
| val secretMatches = secret != null && thread.uuidSecret == secret | ||
| val user = auth?.getUserOrNull() | ||
| val isOwner = user != null && thread.userInternalId.isNotBlank() && thread.userInternalId == user.internalId | ||
| val canReply = (secretMatches || isOwner) && thread.status != SupportThreadStatus.DONE | ||
|
|
||
| val messages = supportService.getPublicMessagesForThread(uuid).map { PublicMessageView.from(it) } | ||
| return ResponseEntity.ok(ThreadDetailView(SupportThreadView.from(thread), messages, canReply)) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Require owner or secret before returning thread details.
This endpoint currently returns the thread, customer identity, email, and messages to any caller with the UUID; secretMatches/isOwner only affects canReply. Gate the whole response before loading/returning messages.
Proposed fix
val secretMatches = secret != null && thread.uuidSecret == secret
val user = auth?.getUserOrNull()
val isOwner = user != null && thread.userInternalId.isNotBlank() && thread.userInternalId == user.internalId
+ if (!secretMatches && !isOwner)
+ return ResponseEntity.status(HttpStatus.FORBIDDEN).build()
val canReply = (secretMatches || isOwner) && thread.status != SupportThreadStatus.DONE📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| val thread = supportService.findByUuid(uuid) ?: return ResponseEntity.notFound().build() | |
| val secretMatches = secret != null && thread.uuidSecret == secret | |
| val user = auth?.getUserOrNull() | |
| val isOwner = user != null && thread.userInternalId.isNotBlank() && thread.userInternalId == user.internalId | |
| val canReply = (secretMatches || isOwner) && thread.status != SupportThreadStatus.DONE | |
| val messages = supportService.getPublicMessagesForThread(uuid).map { PublicMessageView.from(it) } | |
| return ResponseEntity.ok(ThreadDetailView(SupportThreadView.from(thread), messages, canReply)) | |
| val thread = supportService.findByUuid(uuid) ?: return ResponseEntity.notFound().build() | |
| val secretMatches = secret != null && thread.uuidSecret == secret | |
| val user = auth?.getUserOrNull() | |
| val isOwner = user != null && thread.userInternalId.isNotBlank() && thread.userInternalId == user.internalId | |
| if (!secretMatches && !isOwner) | |
| return ResponseEntity.status(HttpStatus.FORBIDDEN).build() | |
| val canReply = (secretMatches || isOwner) && thread.status != SupportThreadStatus.DONE | |
| val messages = supportService.getPublicMessagesForThread(uuid).map { PublicMessageView.from(it) } | |
| return ResponseEntity.ok(ThreadDetailView(SupportThreadView.from(thread), messages, canReply)) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@backend/src/main/kotlin/hu/bme/sch/cmsch/component/support/SupportApiController.kt`
around lines 127 - 135, The thread detail endpoint in SupportApiController
currently exposes SupportThreadView and messages to anyone who has the UUID,
because secretMatches/isOwner only gates canReply. Update the handler to require
either a valid secret or ownership before assembling the response, and return
not found/forbidden otherwise; only after that should it call
supportService.getPublicMessagesForThread and build ThreadDetailView from
SupportThreadView.from(thread).
Comment on lines
+70
to
+80
| var emailWebhookEnabled by BooleanSettingRef(false, serverSideOnly = true, | ||
| fieldName = "Bejövő email webhook engedélyezve", | ||
| description = "Ha ki van kapcsolva, a bejövő email webhook endpoint 404-et ad vissza") | ||
|
|
||
| var incomingEmailSecret by StringSettingRef("", serverSideOnly = true, | ||
| fieldName = "Bejövő email titok", | ||
| description = "Ha üres, nincs ellenőrzés. Ha meg van adva, X-Support-Secret fejlécként kell küldeni.") | ||
|
|
||
| var allowedSenderHostRegex by StringSettingRef("", serverSideOnly = true, | ||
| fieldName = "Engedélyezett küldő host regex", | ||
| description = "Ha üres, nincs ellenőrzés. Ha meg van adva, a bejövő email küldőjének host-ja ennek kell megfeleljen.") |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Require a non-empty secret when the inbound-email webhook is enabled.
The current settings explicitly allow emailWebhookEnabled=true with no incomingEmailSecret, which leaves /support/incoming-email open to forged ticket creation. allowedSenderHostRegex does not mitigate that because the sender address is part of the submitted payload.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@backend/src/main/kotlin/hu/bme/sch/cmsch/component/support/SupportComponent.kt`
around lines 70 - 80, Require a non-empty `incomingEmailSecret` whenever
`emailWebhookEnabled` is enabled in `SupportComponent`; the current
`BooleanSettingRef`/`StringSettingRef` setup allows the inbound-email webhook to
be active without any secret. Update the
`emailWebhookEnabled`/`incomingEmailSecret` settings handling so enabling the
webhook is rejected or automatically blocked unless `incomingEmailSecret` is
set, and keep `allowedSenderHostRegex` as an independent check rather than
relying on it for request authenticity.
Comment on lines
+46
to
+49
| fun findMatchingThread(normalizedSubject: String, email: String): SupportThreadEntity? { | ||
| return threadRepository.findByUserEmail(email) | ||
| .filter { it.status != SupportThreadStatus.DONE } | ||
| .firstOrNull { normalizeSubject(it.title).equals(normalizedSubject, ignoreCase = true) } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Subject-only matching can file replies into the wrong ticket.
findMatchingThread() returns the first open thread with the same normalized title/email. If the same user has two open tickets with the same subject, an inbound reply can be persisted onto the wrong thread. This needs a stable ticket identifier in the reply path; ordering by updatedAt would only be a partial mitigation.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/src/main/kotlin/hu/bme/sch/cmsch/component/support/SupportService.kt`
around lines 46 - 49, `findMatchingThread()` currently matches replies by
normalized subject and sender email, which can route a reply to the wrong open
ticket when multiple threads share the same subject. Update the reply lookup
flow in `SupportService` to use a stable ticket/thread identifier from the
inbound message instead of relying on `normalizeSubject()` matching; if that
identifier is unavailable, do not auto-attach by subject alone.
Comment on lines
+7
to
+15
| export const useSupportThreadsQuery = (enabled = true) => { | ||
| return useQuery<ThreadListResponse, Error>({ | ||
| queryKey: [QueryKeys.SUPPORT_THREADS], | ||
| queryFn: async () => { | ||
| const res = await axios.get<ThreadListResponse>(ApiPaths.SUPPORT_THREADS) | ||
| return res.data | ||
| }, | ||
| enabled | ||
| }) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether auth transitions already clear/query-scope support caches.
fd -i 'useAuthContext*' frontend/src
rg -n -C3 'logout|logOut|signout|signOut|clear\(|removeQueries|resetQueries|invalidateQueries' frontend/src --iglob '*.ts' --iglob '*.tsx'
rg -n -C3 'userId|internalId|kerberos|uuid' frontend/src/api/contexts/auth frontend/src --iglob '*.ts' --iglob '*.tsx'Repository: kir-dev/cmsch
Length of output: 23536
Scope SUPPORT_THREADS by authenticated user. The global key can surface the previous account’s cached support threads until the refetch completes. Include a stable auth identifier in the key, or clear this query on auth changes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/api/hooks/support/useSupportThreadsQuery.ts` around lines 7 -
15, The SUPPORT_THREADS query is currently using a global cache key in
useSupportThreadsQuery, which can expose the prior user’s cached threads after
an auth switch. Update the queryKey in useSupportThreadsQuery to include a
stable authenticated-user identifier from the current auth context, or add
explicit cache clearing on auth changes so cached data is scoped per user.
Ensure the change is applied where QueryKeys.SUPPORT_THREADS is used in the
hook.
Comment on lines
+55
to
+58
| <Link | ||
| key={thread.uuid} | ||
| to={`${AbsolutePaths.SUPPORT}/${thread.uuid}?secret=${thread.uuidSecret}`} | ||
| className="block rounded-lg border p-4 hover:bg-accent transition-colors" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Don't leak uuidSecret in authenticated thread links.
This page only lists the signed-in user's own threads, and the backend already accepts owner-session access without the secret. Appending ?secret=... here needlessly exposes the recovery token in the URL. Link to the plain thread path on this page and reserve the secret URL for anonymous handoff flows only.
Suggested change
- to={`${AbsolutePaths.SUPPORT}/${thread.uuid}?secret=${thread.uuidSecret}`}
+ to={`${AbsolutePaths.SUPPORT}/${thread.uuid}`}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <Link | |
| key={thread.uuid} | |
| to={`${AbsolutePaths.SUPPORT}/${thread.uuid}?secret=${thread.uuidSecret}`} | |
| className="block rounded-lg border p-4 hover:bg-accent transition-colors" | |
| <Link | |
| key={thread.uuid} | |
| to={`${AbsolutePaths.SUPPORT}/${thread.uuid}`} | |
| className="block rounded-lg border p-4 hover:bg-accent transition-colors" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/pages/support/supportList.page.tsx` around lines 55 - 58, The
support thread link in SupportList.page.tsx is exposing the thread recovery
token by appending uuidSecret to authenticated URLs. Update the Link in the
support list rendering so it uses only the plain thread path with thread.uuid,
and keep uuidSecret reserved for anonymous handoff flows handled elsewhere.
Comment on lines
+100
to
+125
| {!isLoggedIn ? ( | ||
| <div className="rounded-lg border p-4 bg-muted text-sm text-muted-foreground"> | ||
| <p className="font-medium text-foreground mb-1">Bejelentkezés szükséges</p> | ||
| <p>Új ügyfélszolgálati szálat csak bejelentkezés után nyithatsz. Email küldéssel is kapcsolatba léphetsz.</p> | ||
| </div> | ||
| ) : ( | ||
| <form onSubmit={handleSubmit} className="w-full mx-auto mt-5"> | ||
| <div className="mt-5"> | ||
| <Label htmlFor="title" className="text-xl">Tárgy *</Label> | ||
| <div className="mt-2"> | ||
| <Input id="title" name="title" placeholder="Mi a kérdésed témája?" required /> | ||
| </div> | ||
| </div> | ||
| <div className="mt-5"> | ||
| <Label htmlFor="content" className="text-xl">Üzenet *</Label> | ||
| <div className="mt-2"> | ||
| <Textarea id="content" name="content" rows={6} placeholder="Írd le részletesen a kérdésedet..." required /> | ||
| </div> | ||
| </div> | ||
| <div className="flex justify-end mt-10"> | ||
| <Button type="submit" disabled={mutation.isPending}> | ||
| {mutation.isPending ? 'Küldés...' : 'Küldés'} | ||
| </Button> | ||
| </div> | ||
| </form> | ||
| )} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Anonymous users can't create a thread from this page.
handleSubmit already supports guest authorEmail, and this PR adds a public create-thread endpoint, but the UI hides the entire form behind isLoggedIn. That makes the anonymous path unreachable and contradicts the logged-out copy on supportList.page.tsx.
Suggested direction
- {!isLoggedIn ? (
- <div className="rounded-lg border p-4 bg-muted text-sm text-muted-foreground">
- <p className="font-medium text-foreground mb-1">Bejelentkezés szükséges</p>
- <p>Új ügyfélszolgálati szálat csak bejelentkezés után nyithatsz. Email küldéssel is kapcsolatba léphetsz.</p>
- </div>
- ) : (
- <form onSubmit={handleSubmit} className="w-full mx-auto mt-5">
+ <form onSubmit={handleSubmit} className="w-full mx-auto mt-5">
+ {!isLoggedIn && (
+ <>
+ <div className="mt-5">
+ <Label htmlFor="authorName" className="text-xl">Név</Label>
+ <div className="mt-2">
+ <Input id="authorName" name="authorName" />
+ </div>
+ </div>
+ <div className="mt-5">
+ <Label htmlFor="authorEmail" className="text-xl">Email *</Label>
+ <div className="mt-2">
+ <Input id="authorEmail" name="authorEmail" type="email" required />
+ </div>
+ </div>
+ </>
+ )}
...
- </form>
- )}
+ </form>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| {!isLoggedIn ? ( | |
| <div className="rounded-lg border p-4 bg-muted text-sm text-muted-foreground"> | |
| <p className="font-medium text-foreground mb-1">Bejelentkezés szükséges</p> | |
| <p>Új ügyfélszolgálati szálat csak bejelentkezés után nyithatsz. Email küldéssel is kapcsolatba léphetsz.</p> | |
| </div> | |
| ) : ( | |
| <form onSubmit={handleSubmit} className="w-full mx-auto mt-5"> | |
| <div className="mt-5"> | |
| <Label htmlFor="title" className="text-xl">Tárgy *</Label> | |
| <div className="mt-2"> | |
| <Input id="title" name="title" placeholder="Mi a kérdésed témája?" required /> | |
| </div> | |
| </div> | |
| <div className="mt-5"> | |
| <Label htmlFor="content" className="text-xl">Üzenet *</Label> | |
| <div className="mt-2"> | |
| <Textarea id="content" name="content" rows={6} placeholder="Írd le részletesen a kérdésedet..." required /> | |
| </div> | |
| </div> | |
| <div className="flex justify-end mt-10"> | |
| <Button type="submit" disabled={mutation.isPending}> | |
| {mutation.isPending ? 'Küldés...' : 'Küldés'} | |
| </Button> | |
| </div> | |
| </form> | |
| )} | |
| <form onSubmit={handleSubmit} className="w-full mx-auto mt-5"> | |
| {!isLoggedIn && ( | |
| <> | |
| <div className="mt-5"> | |
| <Label htmlFor="authorName" className="text-xl">Név</Label> | |
| <div className="mt-2"> | |
| <Input id="authorName" name="authorName" /> | |
| </div> | |
| </div> | |
| <div className="mt-5"> | |
| <Label htmlFor="authorEmail" className="text-xl">Email *</Label> | |
| <div className="mt-2"> | |
| <Input id="authorEmail" name="authorEmail" type="email" required /> | |
| </div> | |
| </div> | |
| </> | |
| )} | |
| <div className="mt-5"> | |
| <Label htmlFor="title" className="text-xl">Tárgy *</Label> | |
| <div className="mt-2"> | |
| <Input id="title" name="title" placeholder="Mi a kérdésed témája?" required /> | |
| </div> | |
| </div> | |
| <div className="mt-5"> | |
| <Label htmlFor="content" className="text-xl">Üzenet *</Label> | |
| <div className="mt-2"> | |
| <Textarea id="content" name="content" rows={6} placeholder="Írd le részletesen a kérdésedet..." required /> | |
| </div> | |
| </div> | |
| <div className="flex justify-end mt-10"> | |
| <Button type="submit" disabled={mutation.isPending}> | |
| {mutation.isPending ? 'Küldés...' : 'Küldés'} | |
| </Button> | |
| </div> | |
| </form> |
🧰 Tools
🪛 GitHub Check: run-linters
[failure] 114-114:
Replace Üzenet·* with ⏎··············Üzenet·*⏎············
[failure] 108-108:
Replace Tárgy·* with ⏎··············Tárgy·*⏎············
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/pages/support/supportNew.page.tsx` around lines 100 - 125, The
supportNew.page.tsx UI currently hides the create-thread form behind isLoggedIn,
which blocks the guest flow even though handleSubmit already accepts anonymous
authorEmail and a public create-thread endpoint exists. Update the conditional
rendering in supportNew.page.tsx so logged-out users can see and submit the
form, and make sure the guest path still collects/passes the email needed by
handleSubmit while keeping the logged-in experience unchanged.
|
|
||
| export const API_BASE_URL = env.VITE_API_BASE_URL ?? import.meta.env.VITE_API_BASE_URL ?? 'http://localhost:8080' | ||
| export const DISABLE_APP_CONFIG_CACHE = (env.VITE_DISABLE_APP_CONFIG_CACHE ?? import.meta.env.VITE_DISABLE_APP_CONFIG_CACHE) === 'true' | ||
| export const APP_CONFIG_CACHE_TTL_SECONDS = Number(env.VITE_APP_CONFIG_CACHE_TTL_SECONDS ?? import.meta.env.VITE_APP_CONFIG_CACHE_TTL_SECONDS ?? 3600) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Wrap this line to unblock the linter.
run-linters is already failing here because the expression exceeds the repository's 140-character limit.
💡 Minimal fix
-export const APP_CONFIG_CACHE_TTL_SECONDS = Number(env.VITE_APP_CONFIG_CACHE_TTL_SECONDS ?? import.meta.env.VITE_APP_CONFIG_CACHE_TTL_SECONDS ?? 3600)
+export const APP_CONFIG_CACHE_TTL_SECONDS = Number(
+ env.VITE_APP_CONFIG_CACHE_TTL_SECONDS ?? import.meta.env.VITE_APP_CONFIG_CACHE_TTL_SECONDS ?? 3600
+)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export const APP_CONFIG_CACHE_TTL_SECONDS = Number(env.VITE_APP_CONFIG_CACHE_TTL_SECONDS ?? import.meta.env.VITE_APP_CONFIG_CACHE_TTL_SECONDS ?? 3600) | |
| export const APP_CONFIG_CACHE_TTL_SECONDS = Number( | |
| env.VITE_APP_CONFIG_CACHE_TTL_SECONDS ?? import.meta.env.VITE_APP_CONFIG_CACHE_TTL_SECONDS ?? 3600 | |
| ) |
🧰 Tools
🪛 GitHub Check: run-linters
[failure] 5-5:
Replace env.VITE_APP_CONFIG_CACHE_TTL_SECONDS·??·import.meta.env.VITE_APP_CONFIG_CACHE_TTL_SECONDS·??·3600 with ⏎··env.VITE_APP_CONFIG_CACHE_TTL_SECONDS·??·import.meta.env.VITE_APP_CONFIG_CACHE_TTL_SECONDS·??·3600⏎
[failure] 5-5:
This line has a length of 150. Maximum allowed is 140
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/util/configs/environment.config.ts` at line 5, The
APP_CONFIG_CACHE_TTL_SECONDS assignment in environment.config.ts exceeds the
140-character lint limit; reformat that constant declaration across multiple
lines so the long Number(...) fallback expression is wrapped cleanly. Keep the
same logic and symbols (APP_CONFIG_CACHE_TTL_SECONDS,
env.VITE_APP_CONFIG_CACHE_TTL_SECONDS, and
import.meta.env.VITE_APP_CONFIG_CACHE_TTL_SECONDS) while splitting the
expression to satisfy the linter.
Source: Linters/SAST tools
Comment on lines
+20
to
+23
| export interface SupportThreadEntity extends SupportThreadView { | ||
| uuidSecret: string | ||
| userInternalId: string | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Keep uuidSecret and userInternalId out of the public list DTO.
ThreadListResponse currently exposes both fields to the SPA, but uuidSecret is effectively a bearer token for anonymous thread access and userInternalId is unnecessary PII here. This should be split into a staff/internal DTO, while the public list stays on SupportThreadView.
Suggested contract split
-export interface SupportThreadEntity extends SupportThreadView {
+export interface SupportThreadAdminView extends SupportThreadView {
uuidSecret: string
userInternalId: string
}
@@
export interface ThreadListResponse {
- threads: SupportThreadEntity[]
+ threads: SupportThreadView[]
}Also applies to: 40-41
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/util/views/support.view.ts` around lines 20 - 23, The public
support thread list DTO is exposing sensitive/internal fields through
SupportThreadEntity, specifically uuidSecret and userInternalId. Split the
contract so ThreadListResponse (and the public SupportThreadView path) only
returns non-sensitive thread data, and introduce a separate internal/staff DTO
for code that needs uuidSecret or userInternalId; update any consumers of
SupportThreadEntity accordingly.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@backend/src/main/kotlin/hu/bme/sch/cmsch/component/support/SupportApiController.kt`:
- Around line 182-195: The incoming email webhook in
SupportApiController.incomingEmail currently takes the secret from the URL path,
which can expose it in logs and tracing. Update the endpoint to read the secret
from a request header instead of the path variable, and adjust the secret
validation against supportComponent.incomingEmailSecret accordingly. If you need
a transition period, temporarily support both header and path forms in
incomingEmail and clearly deprecate the URL-based secret handling.
In
`@backend/src/main/kotlin/hu/bme/sch/cmsch/component/support/SupportService.kt`:
- Around line 217-226: The SRS decoding in decodeSrsEmail is too permissive and
can rewrite untrusted sender addresses based only on a substring match. Update
SupportService so processIncomingEmail only calls or accepts decodeSrsEmail for
messages that have already passed the trusted-forwarder check, such as a
positive spf signal or equivalent inbound trust marker, and tighten
decodeSrsEmail to recognize SRS only when it appears at the start of the
local-part. Keep the customer identity derived from the decoded address only
after that trust gate, and preserve the original raw address otherwise.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3dc9b53c-377f-4b0b-ba9e-64df23ed1f7a
📒 Files selected for processing (6)
backend/src/main/kotlin/hu/bme/sch/cmsch/component/support/SupportApiController.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/support/SupportComponent.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/support/SupportComponentController.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/support/SupportService.ktbackend/src/main/kotlin/hu/bme/sch/cmsch/component/support/dto/IncomingEmailDto.ktbackend/src/main/resources/templates/supportThreadView.html
🚧 Files skipped from review as they are similar to previous changes (3)
- backend/src/main/kotlin/hu/bme/sch/cmsch/component/support/dto/IncomingEmailDto.kt
- backend/src/main/kotlin/hu/bme/sch/cmsch/component/support/SupportComponentController.kt
- backend/src/main/resources/templates/supportThreadView.html
Comment on lines
+182
to
+195
| @PostMapping("/support/incoming-email/{secret}") | ||
| @Operation(summary = "Receive an incoming email webhook (secret is part of the URL)") | ||
| @ApiResponses(value = [ | ||
| ApiResponse(responseCode = "200", description = "Email processed"), | ||
| ApiResponse(responseCode = "404", description = "Webhook disabled or invalid secret") | ||
| ]) | ||
| fun incomingEmail( | ||
| @PathVariable secret: String, | ||
| @RequestBody dto: IncomingEmailDto | ||
| ): ResponseEntity<String> { | ||
| if (!supportComponent.emailWebhookEnabled) | ||
| return ResponseEntity.notFound().build() | ||
| if (secret != supportComponent.incomingEmailSecret) | ||
| return ResponseEntity.notFound().build() |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether deployment/logging config records request paths that would include the webhook secret.
rg -n -C3 'incoming-email|access_log|log_format|request_uri|uri|path|trace|metrics' .Repository: kir-dev/cmsch
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- SupportApiController excerpt ---'
sed -n '160,220p' backend/src/main/kotlin/hu/bme/sch/cmsch/component/support/SupportApiController.kt
echo
echo '--- backend config files mentioning logging/access/tracing/request uri/path ---'
rg -n --hidden --glob 'backend/**' -g '!**/build/**' -g '!**/.gradle/**' \
'(access[_-]?log|request[_-]?uri|requesturi|log_format|trace|tracing|server\.tomcat\.accesslog|logging\.level|include-message|include-stacktrace|path\{\w*\}|uri)' \
backend/src/main/resources backend/src/main/kotlin backend/src/test || trueRepository: kir-dev/cmsch
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- access/logging/tracing related backend config ---'
rg -n --hidden -g 'backend/src/main/resources/**' -g '!**/build/**' -g '!**/.gradle/**' \
'accesslog|access-log|request[_-]?uri|requesturi|uri\(|server\.tomcat\.accesslog|logging\.level\.org\.springframework\.web|trace|tracing|spring\.mvc\.log|include-message|include-stacktrace' \
backend/src/main/resources || true
echo
echo '--- webhook / secret-bearing endpoint patterns in backend ---'
rg -n --hidden -g 'backend/src/main/kotlin/**' -g '!**/build/**' -g '!**/.gradle/**' \
'Webhook|webhook|secret' backend/src/main/kotlin/hu/bme/sch/cmsch/component backend/src/main/kotlin/hu/bme/sch/cmsch/config || trueRepository: kir-dev/cmsch
Length of output: 50370
Move the webhook secret out of the URL path. Path secrets can leak through logs and observability tooling; use a header-based secret, or accept both temporarily and deprecate the URL form.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@backend/src/main/kotlin/hu/bme/sch/cmsch/component/support/SupportApiController.kt`
around lines 182 - 195, The incoming email webhook in
SupportApiController.incomingEmail currently takes the secret from the URL path,
which can expose it in logs and tracing. Update the endpoint to read the secret
from a request header instead of the path variable, and adjust the secret
validation against supportComponent.incomingEmailSecret accordingly. If you need
a transition period, temporarily support both header and path forms in
incomingEmail and clearly deprecate the URL-based secret handling.
Comment on lines
+217
to
+226
| fun decodeSrsEmail(rawAddress: String): String { | ||
| val local = rawAddress.substringBefore("@") | ||
| val srsIndex = local.uppercase().indexOf("SRS=") | ||
| if (srsIndex < 0) return rawAddress | ||
| val afterSrs = local.substring(srsIndex + 4) | ||
| val parts = afterSrs.split("=") | ||
| if (parts.size < 4) return rawAddress | ||
| val domain = parts[parts.size - 2] | ||
| val user = parts[parts.size - 1] | ||
| return if (domain.isNotBlank() && user.isNotBlank()) "$user@$domain" else rawAddress |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether incoming email DTO/config exposes authenticated envelope/SPF/DKIM
# or trusted-forwarder metadata that can safely gate SRS decoding.
rg -n -C4 'decodeSrsEmail|IncomingEmailDto|allowedResentFromAddress|spf|dkim|envelope|forward' backendRepository: kir-dev/cmsch
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the service file, then inspect only the relevant slices.
ast-grep outline backend/src/main/kotlin/hu/bme/sch/cmsch/component/support/SupportService.kt --view expanded || true
sed -n '180,280p' backend/src/main/kotlin/hu/bme/sch/cmsch/component/support/SupportService.kt
# Find the exact call sites and the source of the sender address.
rg -n -C3 'decodeSrsEmail\(|rawFrom|fromAddress|envelope|spf|dkim|trusted|forward' backend/src/main/kotlin backend/src/main/resources
# Inspect the incoming email model(s) and handler flow.
fd -a 'IncomingEmailDto.kt|*Email*Dto*.kt|*Mail*Dto*.kt|*Support*Service*.kt' backend/src/main/kotlinRepository: kir-dev/cmsch
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,220p' backend/src/main/kotlin/hu/bme/sch/cmsch/component/support/dto/IncomingEmailDto.kt
sed -n '1,320p' backend/src/main/kotlin/hu/bme/sch/cmsch/component/support/SupportService.kt
rg -n -C3 'spf|dkim|envelope|resentFrom|allowedResentFromAddress|addresses\.from|addresses\.to|addresses\.resentFrom|decodeSrsEmail' backend/src/main/kotlin/hu/bme/sch/cmsch/component/supportRepository: kir-dev/cmsch
Length of output: 19011
Only decode SRS after a trust check. decodeSrsEmail() rewrites any sender whose local-part merely contains SRS=, and processIncomingEmail() uses that rewritten value as the customer identity without consulting the inbound spf signal or any other trusted-forwarder marker. Restrict this to messages known to come through your forwarder, and only accept SRS when it starts the local-part.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/src/main/kotlin/hu/bme/sch/cmsch/component/support/SupportService.kt`
around lines 217 - 226, The SRS decoding in decodeSrsEmail is too permissive and
can rewrite untrusted sender addresses based only on a substring match. Update
SupportService so processIncomingEmail only calls or accepts decodeSrsEmail for
messages that have already passed the trusted-forwarder check, such as a
positive spf signal or equivalent inbound trust marker, and tighten
decodeSrsEmail to recognize SRS only when it appears at the start of the
local-part. Keep the customer identity derived from the decoded address only
after that trust gate, and preserve the original raw address otherwise.
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.
Needs to be rebased
Summary by CodeRabbit