Skip to content

Conversation

@ashwinvaidya17
Copy link
Contributor

@ashwinvaidya17 ashwinvaidya17 commented Nov 21, 2025

📝 Description

✨ Changes

Select what type of change your PR is:

  • 🚀 New feature (non-breaking change which adds functionality)
  • 🐞 Bug fix (non-breaking change which fixes an issue)
  • 🔄 Refactor (non-breaking change which refactors the code base)
  • ⚡ Performance improvements
  • 🎨 Style changes (code style/formatting)
  • 🧪 Tests (adding/modifying tests)
  • 📚 Documentation update
  • 📦 Build system changes
  • 🚧 CI/CD configuration
  • 🔧 Chore (general maintenance)
  • 🔒 Security update
  • 💥 Breaking change (fix or feature that would cause existing functionality to not work as expected)

✅ Checklist

Before you submit your pull request, please make sure you have completed the following steps:

  • 📚 I have made the necessary updates to the documentation (if applicable).
  • 🧪 I have written tests that support my changes and prove that my fix is effective or my feature works (if applicable).
  • 🏷️ My PR title follows conventional commit format.

For more information about code review checklists, see the Code Review Checklist.

MarkRedeman and others added 30 commits October 8, 2025 13:52
* initial backend commit

Signed-off-by: Ma, Xiangxiang <xiangxiang.ma@intel.com>

* app -> src

Signed-off-by: Ma, Xiangxiang <xiangxiang.ma@intel.com>

* Remove empty file

Signed-off-by: Ma, Xiangxiang <xiangxiang.ma@intel.com>

* move code

Signed-off-by: Ma, Xiangxiang <xiangxiang.ma@intel.com>

* fix style backend

Signed-off-by: Ma, Xiangxiang <xiangxiang.ma@intel.com>

* rename media endpoint

Signed-off-by: Ma, Xiangxiang <xiangxiang.ma@intel.com>

---------

Signed-off-by: Ma, Xiangxiang <xiangxiang.ma@intel.com>
…#2942)

add unit tests for endpoints

Signed-off-by: Ma, Xiangxiang <xiangxiang.ma@intel.com>
These won't be needed yet
…tform#2948)

* Use src folder inside run.sh

* Set openapi_url path

* Update to react 19

* Specify bash language in readme
…edge-platform#2945)

* add training + inference endpoint

Signed-off-by: Ma, Xiangxiang <xiangxiang.ma@intel.com>

* remove model api

Signed-off-by: Ma, Xiangxiang <xiangxiang.ma@intel.com>

* cleanup code

Signed-off-by: Ma, Xiangxiang <xiangxiang.ma@intel.com>

* update async execution

Signed-off-by: Ma, Xiangxiang <xiangxiang.ma@intel.com>

* improve training worker loop and predict endpoint

Signed-off-by: Ma, Xiangxiang <xiangxiang.ma@intel.com>

* fix style

Signed-off-by: Ma, Xiangxiang <xiangxiang.ma@intel.com>

* fix style to use python3.10 generics

Signed-off-by: Ma, Xiangxiang <xiangxiang.ma@intel.com>

* add tests for services

Signed-off-by: Ma, Xiangxiang <xiangxiang.ma@intel.com>

* style fix

Signed-off-by: Ma, Xiangxiang <xiangxiang.ma@intel.com>

* style fix

Signed-off-by: Ma, Xiangxiang <xiangxiang.ma@intel.com>

* style fix

Signed-off-by: Ma, Xiangxiang <xiangxiang.ma@intel.com>

* style fix

Signed-off-by: Ma, Xiangxiang <xiangxiang.ma@intel.com>

* style fix

Signed-off-by: Ma, Xiangxiang <xiangxiang.ma@intel.com>

---------

Signed-off-by: Ma, Xiangxiang <xiangxiang.ma@intel.com>
…edge-platform#2961)

* Add github actions for ui and server of geti inspect

* Exclude UI from pre-commit prettier configuration

The UI uses a different prettier configuration that does not seem to be
picked up by pre-commit.

* Add newline to .prettierignore

* Apply prettier to `geti-inspect.yaml`

* Generate OpenAPI spec before running UI checks

* Checkout with lfs

* Fix lint issues by removing wip components

* Try installing git lfs in the playwright docker image

* Fix unused noqa
…edge-platform#2963)

* Add OpenAPI route

* Remove MSW browser worker

* Rename infernece to inspect

* Update navbar title

* Rename infernece to inspect
* rename models/ to pydantic_models/

Signed-off-by: Ma, Xiangxiang <xiangxiang.ma@intel.com>

* switch to use async session context manager

Signed-off-by: Ma, Xiangxiang <xiangxiang.ma@intel.com>

* fix unit tests

Signed-off-by: Ma, Xiangxiang <xiangxiang.ma@intel.com>

* add pipeline endpoints

Signed-off-by: Ma, Xiangxiang <xiangxiang.ma@intel.com>

* fix frame aquisition worker and rename pipiline endpoints

Signed-off-by: Ma, Xiangxiang <xiangxiang.ma@intel.com>

* add sources and sinks endpoints

Signed-off-by: Ma, Xiangxiang <xiangxiang.ma@intel.com>

* stream loading working

Signed-off-by: Ma, Xiangxiang <xiangxiang.ma@intel.com>

* add webrtc endpoints

Signed-off-by: Ma, Xiangxiang <xiangxiang.ma@intel.com>

* fix workers: stream loading + inference + dispatcher

Signed-off-by: Ma, Xiangxiang <xiangxiang.ma@intel.com>

* fix sinks

Signed-off-by: Ma, Xiangxiang <xiangxiang.ma@intel.com>

* style

Signed-off-by: Ma, Xiangxiang <xiangxiang.ma@intel.com>

* add unit tests and address comments

Signed-off-by: Ma, Xiangxiang <xiangxiang.ma@intel.com>

* add tests

Signed-off-by: Ma, Xiangxiang <xiangxiang.ma@intel.com>

* fix example schema

Signed-off-by: Ma, Xiangxiang <xiangxiang.ma@intel.com>

* add todo

Signed-off-by: Ma, Xiangxiang <xiangxiang.ma@intel.com>

---------

Signed-off-by: Ma, Xiangxiang <xiangxiang.ma@intel.com>
…2970)

* chore: Add path alias to icons

* feat: Add sidebar with dataset, models and stats
* feat: Display placeholders for images

* refactor: Fix scrollbar

* chore: Remove learn more
* chore(inspect): Update UI scripts

* chore: Update github actions

* chore: Update port to 8000
* chore: Add project route

* feat: Add project management

* revert ui lock change

* chore: Remove not needed code for project management
… training progress (open-edge-platform#2984)

* feat: Allow user to upload images

* feat: List uploaded images

* chore: Extract components to separate files and add ready to train and training progress

* chore: comment thumbnail url generation
…en-edge-platform#2989)

* chore(inspect): Renamed app to application

* chore(inspect): Rename app to application in github actions
…of email (open-edge-platform#2990)

refactor: Update photo placeholder to use indicator instead of email
…dge-platform#2991)

Improve error and suspense handling in router

By moving all of the routes into a single root route we can make sure
that all routes are rendered inside of an layout that has a suspense and
error boundary.
Signed-off-by: Ashwin Vaidya <ashwin.vaidya@intel.com>
* add trainable models endpoint

* fix

Signed-off-by: Ma, Xiangxiang <xiangxiang.ma@intel.com>

* add test

Signed-off-by: Ma, Xiangxiang <xiangxiang.ma@intel.com>

* add copyright

Signed-off-by: Ma, Xiangxiang <xiangxiang.ma@intel.com>

---------

Signed-off-by: Ma, Xiangxiang <xiangxiang.ma@intel.com>
…rm#3004)

* add thumbnails endpoint

Signed-off-by: Ma, Xiangxiang <xiangxiang.ma@intel.com>

* add tests

Signed-off-by: Ma, Xiangxiang <xiangxiang.ma@intel.com>

* generate thumbnails as background task

Signed-off-by: Ma, Xiangxiang <xiangxiang.ma@intel.com>

* update docstring

Signed-off-by: Ma, Xiangxiang <xiangxiang.ma@intel.com>

---------

Signed-off-by: Ma, Xiangxiang <xiangxiang.ma@intel.com>
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 13 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +49 to +52
# Map all host devices to provide access to webcams and other attached devices
privileged: true
devices:
- /dev:/dev
Copy link

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

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

Running the container in privileged mode with full access to /dev is a significant security risk that grants the container nearly unrestricted access to the host system. Consider mapping only specific required devices (e.g., /dev/video0 for webcams) and removing privileged: true unless absolutely necessary for the application's core functionality.

Suggested change
# Map all host devices to provide access to webcams and other attached devices
privileged: true
devices:
- /dev:/dev
# Map only required host devices (e.g., webcam) to the container for security.
# privileged: true # Removed for security; only enable if absolutely necessary.
# devices:
# - /dev/video0:/dev/video0 # Example: map only webcam device if needed.

Copilot uses AI. Check for mistakes.
"http://localhost:9000",
"http://127.0.0.1:9000",
],
allow_origins=["*"],
Copy link

Choose a reason for hiding this comment

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

# Alembic
alembic_config_path: str = "src/alembic.ini"
alembic_script_location: str = "src/alembic"
alembic_config_path: str = str(_MODULE_DIR / "alembic.ini")
Copy link

Choose a reason for hiding this comment

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

Should be adjusted further when we introduce Pyinstaller

@@ -0,0 +1,52 @@
# PID file in a location non-root user can write to
Copy link

Choose a reason for hiding this comment

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

Can we omit having nginx by serving UI static using FastAPI?

Copy link
Contributor

Choose a reason for hiding this comment

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

Just a question: why put everything in a parent folder .packaging/?

Copy link
Contributor

Choose a reason for hiding this comment

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

Could we move it into application/docker (without .packaging?) no need to hide it imho. This would also align well with the other applications.


EXPOSE 80

CMD ["sh", "-c", "nginx && exec uv run src/main.py"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we use backend/run.sh?

Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add example of running the container with camera passthrough? I guess would be a common usecase

Copy link
Contributor

Choose a reason for hiding this comment

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

I've tried to build it and it works. However, couldn't get camera passthrough to work on mac. Have you tested it on linux?

Signed-off-by: Ashwin Vaidya <ashwin.vaidya@intel.com>
Signed-off-by: Ashwin Vaidya <ashwin.vaidya@intel.com>
Copilot AI review requested due to automatic review settings December 6, 2025 05:17
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 11 out of 14 changed files in this pull request and generated 5 comments.

Comments suppressed due to low confidence (1)

application/docker/Dockerfile:1

  • The dockerfile path references application/.packaging/docker/Dockerfile, but based on the file structure, the Dockerfile is located at application/docker/Dockerfile. This mismatch will cause the build to fail.
#------------------------------------------

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

## To create CPU build

```bash
cd application/.packaging/docker
Copy link

Copilot AI Dec 6, 2025

Choose a reason for hiding this comment

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

The directory path in the instructions is incorrect. The actual docker files are in application/docker/, not application/.packaging/docker/. This inconsistency appears in all three build instruction sections and will cause users to encounter errors when following the documentation.

Copilot uses AI. Check for mistakes.


@webui_router.get("/", include_in_schema=False)
async def get_webui(full_path: str = "") -> FileResponse: # noqa: ARG001
Copy link

Copilot AI Dec 6, 2025

Choose a reason for hiding this comment

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

The function parameter full_path is declared but never used, and there's no logic to prevent potential path traversal attacks or handle different routes. If this endpoint is meant to handle multiple paths, the implementation should use full_path to serve the appropriate files. If it only serves index.html, the parameter should be removed.

Suggested change
async def get_webui(full_path: str = "") -> FileResponse: # noqa: ARG001
async def get_webui() -> FileResponse:

Copilot uses AI. Check for mistakes.
@webui_router.get("/", include_in_schema=False)
async def get_webui(full_path: str = "") -> FileResponse: # noqa: ARG001
"""Get the webui index.html file."""
if settings.static_files_dir and not (file_path := Path(settings.static_files_dir) / "index.html").exists():
Copy link

Copilot AI Dec 6, 2025

Choose a reason for hiding this comment

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

If settings.static_files_dir is None or empty, file_path will not be defined, causing an UnboundLocalError on line 20. The condition should ensure file_path is always defined before the return statement, or raise an appropriate error when static_files_dir is not configured.

Suggested change
if settings.static_files_dir and not (file_path := Path(settings.static_files_dir) / "index.html").exists():
if not settings.static_files_dir:
raise HTTPException(status_code=500, detail="Static files directory is not configured")
file_path = Path(settings.static_files_dir) / "index.html"
if not file_path.exists():

Copilot uses AI. Check for mistakes.
"http://localhost:9000",
"http://127.0.0.1:9000",
],
allow_origins=["*"],
Copy link

Copilot AI Dec 6, 2025

Choose a reason for hiding this comment

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

Allowing all origins with allow_origins=[\"*\"] is a security risk in production environments as it permits any domain to make requests to the API. Consider making this configurable through settings and restricting it to specific trusted origins in production.

Copilot uses AI. Check for mistakes.
Comment on lines +130 to +131
&& echo "deb [arch=amd64,i386 signed-by=/usr/share/keyrings/intel-graphics.gpg] https://repositories.intel.com/gpu/ubuntu jammy unified" | \
tee /etc/apt/sources.list.d/intel-gpu-jammy.list \
Copy link

Copilot AI Dec 6, 2025

Choose a reason for hiding this comment

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

The XPU build stage is adding Ubuntu 'jammy' repositories to a Debian-based image (python:3.13-slim is based on Debian). This repository mismatch may cause package installation issues or incompatibilities. Use the appropriate Debian-compatible Intel GPU repository instead.

Suggested change
&& echo "deb [arch=amd64,i386 signed-by=/usr/share/keyrings/intel-graphics.gpg] https://repositories.intel.com/gpu/ubuntu jammy unified" | \
tee /etc/apt/sources.list.d/intel-gpu-jammy.list \
&& echo "deb [arch=amd64,i386 signed-by=/usr/share/keyrings/intel-graphics.gpg] https://repositories.intel.com/gpu/debian bookworm unified" | \
tee /etc/apt/sources.list.d/intel-gpu-bookworm.list \

Copilot uses AI. Check for mistakes.
ashwinvaidya17 and others added 2 commits December 8, 2025 10:28
Signed-off-by: Ashwin Vaidya <ashwinnitinvaidya@gmail.com>
Signed-off-by: Ashwin Vaidya <ashwin.vaidya@intel.com>
Copilot AI review requested due to automatic review settings December 10, 2025 12:36

EXPOSE 8000

CMD ["sh", "-c", "exec ./run.sh"]

Check failure

Code scanning / Semgrep OSS

Semgrep Finding: dockerfile.security.missing-user.missing-user Error

By not specifying a USER, a program in the container may run as 'root'. This is a security hazard. If an attacker can control a process running as root, they may have control over the container. Ensure that the last USER in a Dockerfile is a USER other than 'root'.

EXPOSE 8000

CMD ["sh", "-c", "exec ./run.sh"]

Check failure

Code scanning / Semgrep OSS

Semgrep Finding: dockerfile.security.missing-user.missing-user Error

By not specifying a USER, a program in the container may run as 'root'. This is a security hazard. If an attacker can control a process running as root, they may have control over the container. Ensure that the last USER in a Dockerfile is a USER other than 'root'.

EXPOSE 8000

CMD ["sh", "-c", "exec ./run.sh"]

Check failure

Code scanning / Semgrep OSS

Semgrep Finding: dockerfile.security.missing-user.missing-user Error

By not specifying a USER, a program in the container may run as 'root'. This is a security hazard. If an attacker can control a process running as root, they may have control over the container. Ensure that the last USER in a Dockerfile is a USER other than 'root'.
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 13 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

# - "${RENDER_GROUP_ID}" # This is needed to allow access to the 'render' group for torch XPU support
volumes:
# Persist database and uploaded data
- /backend/data:/app/data
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

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

The volume mount source path /backend/data is an absolute host path that likely doesn't exist. This should either be a relative path like ./data:/app/data or use a named volume. The same issue exists for the logs volume on line 33.

Copilot uses AI. Check for mistakes.
def database_url(self) -> str:
"""Get database URL"""
return f"sqlite+aiosqlite:///./{self.data_dir / self.database_file}?journal_mode=WAL"
return f"sqlite+aiosqlite:///{self.data_dir / self.database_file}?journal_mode=WAL"
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

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

The change from sqlite+aiosqlite:///./{path} to sqlite+aiosqlite:///{path} removes the relative path prefix. While this might work for absolute paths, it could break existing deployments using relative paths. Consider maintaining backward compatibility or documenting this breaking change.

Copilot uses AI. Check for mistakes.
Comment on lines +125 to +126
&& echo "deb [arch=amd64,i386 signed-by=/usr/share/keyrings/intel-graphics.gpg] https://repositories.intel.com/gpu/ubuntu jammy unified" | \
tee /etc/apt/sources.list.d/intel-gpu-jammy.list \
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

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

The XPU build stage is based on Debian 12 (bookworm) as indicated by the base image python:3.13-slim, but this configures Ubuntu Jammy (22.04) repositories. This architecture mismatch will likely cause package installation failures or runtime incompatibilities.

Suggested change
&& echo "deb [arch=amd64,i386 signed-by=/usr/share/keyrings/intel-graphics.gpg] https://repositories.intel.com/gpu/ubuntu jammy unified" | \
tee /etc/apt/sources.list.d/intel-gpu-jammy.list \
&& echo "deb [arch=amd64,i386 signed-by=/usr/share/keyrings/intel-graphics.gpg] https://repositories.intel.com/gpu/debian bookworm unified" | \
tee /etc/apt/sources.list.d/intel-gpu-bookworm.list \

Copilot uses AI. Check for mistakes.
Signed-off-by: Ashwin Vaidya <ashwinnitinvaidya@gmail.com>
Comment on lines 27 to 29
group_add:
# - "${RENDER_GROUP_ID}" # This is needed to allow access to the 'render' group for torch XPU support
- "${VIDEO_GROUP_ID}" # This is needed to allow access to video devices (webcams)
Copy link
Contributor

Choose a reason for hiding this comment

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

I didn't need this setting while testing on a lunar lake laptop. Though I guess this could differ based on how docker was installed?

restart: unless-stopped
build:
# Use root directory as context to build the image
context: ../..
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this not be ../? (i.e. the application being the context excluding any library files?)

Comment on lines 18 to 19
ARG PUBLIC_API_BASE_URL=""
ENV PUBLIC_API_BASE_URL=${PUBLIC_API_BASE_URL}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we could omit this, or set PUBLIC_API_BASE_URL="".
When deploying Inspect the UI and backend should be expected to be served on the same hosts, this allows us to avoid security policies like CORS and (later?) have a better CSP configuration.


COPY --link application/ui/package.json ./package.json
COPY --link application/ui/package-lock.json ./package-lock.json
COPY --link application/ui/packages ./packages
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should be omitted. The npm install script should clone and install the geti packages with the npm ci command.
If we copy from the host then we risk the docker image containing outdated packages (or at least packages that are not in sync with what's in the anomalib repo).

```bash
cd application/docker
RENDER_GROUP_ID=$(getent group render | cut -d: -f3) AI_DEVICE=xpu docker compose up
Copy link
Contributor

Choose a reason for hiding this comment

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

For me just running,

AI_DEVICE=xpu docker compose up

was working. (same for build command)

@@ -0,0 +1,28 @@
# Docker distribution for Geti Inspect

## To create CPU build
Copy link
Contributor

Choose a reason for hiding this comment

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

The docs here mention creating builds, but the commands show up being used, is that intended?

Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps one other thing we could look into is having separate docker-compose.xpu.yaml, docker-compsoe.nvidia.yaml files and tell the user to use these (instead of them uncommenting stuff).

See for instance https://github.com/open-webui/open-webui as an example of a repo using this.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would move this file into the application folder, since I think we shouldn't expect users to use docker for the library?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since the context is the root of anomalib, it needs .dockerignore in the same folder

Co-authored-by: Mark Redeman <markredeman@gmail.com>
Signed-off-by: Ashwin Vaidya <ashwinnitinvaidya@gmail.com>
Copilot AI review requested due to automatic review settings December 15, 2025 08:35
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 13 changed files in this pull request and generated 6 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

# Docker distribution for Geti Inspect

## To create CPU build

Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

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

The command is missing the required environment variable for the XPU build. For CPU build, it should also set VIDEO_GROUP_ID similar to the XPU example, but it's missing the note about uncommenting the group_add line in docker-compose.yml.

Suggested change
> [!NOTE]
> You need to uncomment the `group_add` line in the `docker-compose.yml` file to enable video group permissions.

Copilot uses AI. Check for mistakes.
```bash
cd application/docker
RENDER_GROUP_ID=$(getent group render | cut -d: -f3) AI_DEVICE=xpu docker compose up
Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

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

The XPU build command should also include VIDEO_GROUP_ID since the docker-compose.yml requires it for video device access. The command should be: RENDER_GROUP_ID=$(getent group render | cut -d: -f3) VIDEO_GROUP_ID=$(getent group video | cut -d: -f3) AI_DEVICE=xpu docker compose up

Suggested change
RENDER_GROUP_ID=$(getent group render | cut -d: -f3) AI_DEVICE=xpu docker compose up
RENDER_GROUP_ID=$(getent group render | cut -d: -f3) VIDEO_GROUP_ID=$(getent group video | cut -d: -f3) AI_DEVICE=xpu docker compose up

Copilot uses AI. Check for mistakes.
Comment on lines 25 to 28
# Uncomment the following lines to enable torch XPU support
# For more info, see README.md
group_add:
# - "${RENDER_GROUP_ID}" # This is needed to allow access to the 'render' group for torch XPU support
Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

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

The comment mentions 'torch XPU support' but doesn't clarify that users need to uncomment this line when using AI_DEVICE=xpu. The README mentions setting RENDER_GROUP_ID but doesn't explicitly state this line must be uncommented.

Suggested change
# Uncomment the following lines to enable torch XPU support
# For more info, see README.md
group_add:
# - "${RENDER_GROUP_ID}" # This is needed to allow access to the 'render' group for torch XPU support
# To enable torch XPU support (when using AI_DEVICE=xpu), you MUST uncomment the RENDER_GROUP_ID line below.
# For more info, see README.md
group_add:
# - "${RENDER_GROUP_ID}" # Uncomment this line when using AI_DEVICE=xpu to allow access to the 'render' group for torch XPU support

Copilot uses AI. Check for mistakes.
@webui_router.get("/{full_path:path}", include_in_schema=False)
async def get_webui(full_path: str = "") -> FileResponse: # noqa: ARG001
"""Get the webui index.html file."""
if settings.static_files_dir and not (file_path := Path(settings.static_files_dir) / "index.html").exists():
Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

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

If static_files_dir is None or not set, file_path will be undefined when reaching the return statement, causing a NameError. The logic should handle the case where static_files_dir is None by raising an HTTPException before attempting to return FileResponse.

Suggested change
if settings.static_files_dir and not (file_path := Path(settings.static_files_dir) / "index.html").exists():
if not settings.static_files_dir:
raise HTTPException(status_code=404, detail="Static files directory not set")
file_path = Path(settings.static_files_dir) / "index.html"
if not file_path.exists():

Copilot uses AI. Check for mistakes.
"http://localhost:9000",
"http://127.0.0.1:9000",
],
allow_origins=["*"],
Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

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

Allowing all origins with '*' in production exposes the application to CSRF attacks. This should be configurable via environment variables or restricted to known origins in production deployments.

Copilot uses AI. Check for mistakes.
Comment on lines +50 to +53
# Map all host devices to provide access to webcams and other attached devices
privileged: true
devices:
- /dev:/dev
Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

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

Running the container in privileged mode grants extensive permissions and poses security risks. Consider using specific device permissions and capabilities instead of privileged mode unless absolutely necessary for webcam access.

Suggested change
# Map all host devices to provide access to webcams and other attached devices
privileged: true
devices:
- /dev:/dev
# Map only required devices to provide access to webcams and other attached devices
# privileged: true # Removed for security; use specific device mapping below
devices:
- /dev/video0:/dev/video0 # Map only the webcam device; add more as needed

Copilot uses AI. Check for mistakes.
ashwinvaidya17 and others added 3 commits December 15, 2025 09:55
Signed-off-by: Ashwin Vaidya <ashwinnitinvaidya@gmail.com>
…17/anomalib into ashwin/feature/dockerfile_2
Copilot AI review requested due to automatic review settings December 15, 2025 08:58
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 13 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +19 to +20
if settings.static_files_dir and not (file_path := Path(settings.static_files_dir) / "index.html").exists():
raise HTTPException(status_code=404, detail="File not found")
Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

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

The variable file_path is only assigned when the condition is True, but it's used unconditionally in the return statement. If settings.static_files_dir is None or empty, file_path will be undefined, causing a NameError. Initialize file_path before the conditional or handle the None case explicitly.

Suggested change
if settings.static_files_dir and not (file_path := Path(settings.static_files_dir) / "index.html").exists():
raise HTTPException(status_code=404, detail="File not found")
file_path = None
if settings.static_files_dir:
file_path = Path(settings.static_files_dir) / "index.html"
if not file_path.exists():
raise HTTPException(status_code=404, detail="File not found")
else:
raise HTTPException(status_code=404, detail="Static files directory not configured")

Copilot uses AI. Check for mistakes.
Signed-off-by: Ashwin Vaidya <ashwinnitinvaidya@gmail.com>
# Persist logs
- ../backend/logs:/app/logs
ports:
- "8000:8000"

Check warning

Code scanning / Semgrep OSS

Semgrep Finding: trailofbits.yaml.docker-compose.port-all-interfaces.port-all-interfaces Warning

Service port is exposed on all interfaces
# Persist logs
- ../backend/logs:/app/logs
ports:
- "8000:8000"

Check warning

Code scanning / Semgrep OSS

Semgrep Finding: trailofbits.yaml.docker-compose.port-all-interfaces.port-all-interfaces Warning

Service port is exposed on all interfaces
Signed-off-by: Ashwin Vaidya <ashwin.vaidya@intel.com>
Copilot AI review requested due to automatic review settings December 15, 2025 12:18
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 13 out of 16 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.


```bash
cd application/docker
compose up
Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

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

Missing 'docker' command prefix. Should be 'docker compose up'.

Suggested change
compose up
docker compose up

Copilot uses AI. Check for mistakes.

```bash
cd application/docker
docker compose -f docker-compose.cuda.yaml up
Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

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

The 'Build the container' section uses 'up' which builds and starts the container, not just builds it. Should use 'build' command to match the section heading, or update the heading to reflect that this command both builds and starts.

Suggested change
docker compose -f docker-compose.cuda.yaml up
docker compose -f docker-compose.cuda.yaml build

Copilot uses AI. Check for mistakes.
"http://localhost:9000",
"http://127.0.0.1:9000",
],
allow_origins=["*"],
Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

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

Changed from specific allowed origins to wildcard '*', which allows requests from any origin. This creates a security vulnerability by disabling CORS protection. Consider restricting to specific trusted origins in production or making this configurable.

Copilot uses AI. Check for mistakes.
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.

7 participants