immich-app/immich
1
0
Fork 0
mirror of https://github.com/immich-app/immich.git synced 2026-02-05 00:30:57 +03:00
Code Issues 11 Packages Projects Releases 10 Wiki Activity

[CHORE] Sanitize login page message #153

New Issue
Closed
opened 2026-02-04 18:12:37 +03:00 by OVERLORD · 8 comments
OVERLORD commented 2026-02-04 18:12:37 +03:00
Owner
Copy Link

Originally created by @jbaez on GitHub (Jul 24, 2022).

Protect agains XSS when rendering HTML from "untrusted sources".
Currently the HTML is injected through VITE_LOGIN_PAGE_MESSAGE env variable, although this might change.

Would need to sanitize the HTML before rendering either in the client or in the server (depends where the source of the login page message is)

Originally created by @jbaez on GitHub (Jul 24, 2022). Protect agains XSS when rendering HTML from "untrusted sources". Currently the HTML is injected through `VITE_LOGIN_PAGE_MESSAGE` env variable, although this might change. Would need to sanitize the HTML before rendering either in the client or in the server (depends where the source of the login page message is)
OVERLORD commented 2026-02-04 18:12:44 +03:00
Author
Owner
Copy Link

@bo0tzz commented on GitHub (Jul 24, 2022):

If this is set by the server admin it can be considered trusted, right?

@bo0tzz commented on GitHub (Jul 24, 2022): If this is set by the server admin it can be considered trusted, right?
OVERLORD commented 2026-02-04 18:12:53 +03:00
Author
Owner
Copy Link

@alextran1502 commented on GitHub (Jul 24, 2022):

If this is set by the server admin it can be considered trusted, right?

Yes, the concern here is that less technical admin who could be the target of social engineering attacks.

@alextran1502 commented on GitHub (Jul 24, 2022): > If this is set by the server admin it can be considered trusted, right? Yes, the concern here is that less technical admin who could be the target of social engineering attacks.
OVERLORD commented 2026-02-04 18:12:56 +03:00
Author
Owner
Copy Link

@bo0tzz commented on GitHub (Jul 24, 2022):

There is a lot more surface area there that couldn't reasonably be covered - I don't think it's worth worrying about.

@bo0tzz commented on GitHub (Jul 24, 2022): There is a lot more surface area there that couldn't reasonably be covered - I don't think it's worth worrying about.
OVERLORD commented 2026-02-04 18:13:01 +03:00
Author
Owner
Copy Link

@bo0tzz commented on GitHub (Aug 10, 2022):

Is this issue still applicable, or can it be closed?

@bo0tzz commented on GitHub (Aug 10, 2022): Is this issue still applicable, or can it be closed?
OVERLORD commented 2026-02-04 18:13:10 +03:00
Author
Owner
Copy Link

@jbaez commented on GitHub (Aug 12, 2022):

I would consider rendering HTML unsafe unless it has been sanitized before, either in the server or the client.
I have implemented a "client sanitizer helper" long time ago, although it looks like there are libraries that do a better job and even a promising future native API https://developer.mozilla.org/en-US/docs/Web/API/HTML_Sanitizer_API 🙂

I think we should get this sanitized, as its a quick one to implement and it would remove any possibility of rendering malicious code from that env variable.

I have been busy lately and haven't had chance to work on Immich, but I could find some time to get this one done soon, if everybody is happy with that 😄

@jbaez commented on GitHub (Aug 12, 2022): I would consider rendering HTML unsafe unless it has been sanitized before, either in the server or the client. I have implemented a "client sanitizer helper" long time ago, although it looks like there are libraries that do a better job and even a promising future native API https://developer.mozilla.org/en-US/docs/Web/API/HTML_Sanitizer_API 🙂 I think we should get this sanitized, as its a quick one to implement and it would remove any possibility of rendering malicious code from that env variable. I have been busy lately and haven't had chance to work on Immich, but I could find some time to get this one done soon, if everybody is happy with that 😄
OVERLORD commented 2026-02-04 18:13:14 +03:00
Author
Owner
Copy Link

@bo0tzz commented on GitHub (Aug 12, 2022):

Another reason I'm partial to not sanitizing is because now the admin has complete control over what is rendered. The demo instance currently just uses some italicized text, but I can imagine use cases for including things such as links or images (or even more).

@bo0tzz commented on GitHub (Aug 12, 2022): Another reason I'm partial to not sanitizing is because now the admin has complete control over what is rendered. The demo instance currently just uses some italicized text, but I can imagine use cases for including things such as links or images (or even more).
OVERLORD commented 2026-02-04 18:13:19 +03:00
Author
Owner
Copy Link

@jbaez commented on GitHub (Aug 13, 2022):

Sanitizer libraries provide configuration for what things are allowed in the HTML. I think mainly what it shouldn't be allowed is JavaScript, either in <script> tags or in a tag as an event attribute.

Doing a quick search, looks like this library could be a good option: https://www.npmjs.com/package/xss.
This one also looks good: https://www.npmjs.com/package/dompurify

@jbaez commented on GitHub (Aug 13, 2022): Sanitizer libraries provide configuration for what things are allowed in the HTML. I think mainly what it shouldn't be allowed is JavaScript, either in `<script>` tags or in a tag as an event attribute. Doing a quick search, looks like this library could be a good option: https://www.npmjs.com/package/xss. This one also looks good: https://www.npmjs.com/package/dompurify
OVERLORD commented 2026-02-04 18:13:25 +03:00
Author
Owner
Copy Link

@bo0tzz commented on GitHub (Aug 18, 2022):

After some discussion elsewhere, we've decided to close this issue and revisit the topic when we implement a server side MOTD.

@bo0tzz commented on GitHub (Aug 18, 2022): After some discussion elsewhere, we've decided to close this issue and revisit the topic when we implement a server side MOTD.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
accessibility
changelog:enhancement
changelog:security
changelog:translation
cli
date-time
documentation
external-library
format
good first issue
needs-answer
nice to have
sharing
tech-debt
📱mobile
🖥️web
🗄️server
🧠machine-learning
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
OVERLORD
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: immich-app/immich#153
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?