Skip to content

feat: enhance asset path handling to include config directory and improve assets directory validation #604

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 14 commits into from

Conversation

vishalkadam47
Copy link
Contributor

Support Flexible Asset Path Resolution for Local and Docker Environments

Allow users to organize their Glance files in two ways:

  1. Simple: All files in one /config directory
  2. Advanced: Split between /config (settings) and /assets (static files) - Existing setup

Changes:

  • Improve path detection and validation
  • Keep compatibility with existing setups
  • Add helpful error messages

Single Volume Setup

Docker Compose

services:
  glance:
    image: glanceapp/glance
    volumes:
      - ./config:/app/config # All files in config directory
    ports:
      - 8080:8080

glance.yml

server:
assets-path: config
theme:
custom-css-file: /config/user.css
branding:
favicon-url: /config/favicon.png
logo-url: /config/logo.png

@@ -262,6 +262,7 @@ func (a *application) server() (func() error, func() error) {
absAssetsPath, _ = filepath.Abs(a.Config.Server.AssetsPath)
assetsFS := fileServerWithCache(http.Dir(a.Config.Server.AssetsPath), 2*time.Hour)
mux.Handle("/assets/{path...}", http.StripPrefix("/assets/", assetsFS))
mux.Handle("/config/{path...}", http.StripPrefix("/config/", assetsFS))
Copy link
Member

Choose a reason for hiding this comment

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

This is a gigantic security hole because not everyone will use environment variables for their passwords/API keys, and you're exposing their configs.

@svilenmarkov
Copy link
Member

I don't think this change is necessary because you can already do this by having the assets directory within your config directory, mounting just the config directory and then specifying the location of the assets within glance.yml like such:

server:
  assets-path: /app/config/assets

@vishalkadam47
Copy link
Contributor Author

vishalkadam47 commented May 19, 2025

I don't think this change is necessary because you can already do this by having the assets directory within your config directory, mounting just the config directory and then specifying the location of the assets within glance.yml like such:

server:
  assets-path: /app/config/assets

if I keep custom css, js and image files in /app/config/assets
this doesn't work

server:
  assets-path: /app/config/assets

theme:
  custom-css-file: /config/assets/user.css

branding:
  logo-url: /config/assets/logo.png
  favicon-url: /config/assets/favicon.png

I understand that my modifications might be wrong, but if there is any other option to use single path or with multiple sub dir for all the files please add a support

@svilenmarkov
Copy link
Member

You must specify the path relative to the server URL, not relative to local path:

server:
  # this is a local directory path
  assets-path: /app/config/assets 

theme:
  # this is a server URL path and will resolve to www.example.com/assets/user.css
  custom-css-file: /assets/user.css 

branding:
  logo-url: /assets/logo.png
  favicon-url: /assets/favicon.png

@vishalkadam47
Copy link
Contributor Author

You must specify the path relative to the server URL, not relative to local path:

server:
  # this is a local directory path
  assets-path: /app/config/assets 

theme:
  # this is a server URL path and will resolve to www.example.com/assets/user.css
  custom-css-file: /assets/user.css 

branding:
  logo-url: /assets/logo.png
  favicon-url: /assets/favicon.png

aah you are right, may bad it was confusing a little now i get this

@vishalkadam47
Copy link
Contributor Author

not relevant PR as it already works

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