Conversation
Reviewer's Guide by SourceryThis pull request containerizes the application using Docker. It introduces a Dockerfile, along with instructions in the README.md file on how to build and run the Docker image. A changelog entry was also added. Sequence diagram for running the application inside a Docker containersequenceDiagram
actor User
participant Docker
participant OasisScanner
participant Ollama
User->>Docker: Builds image with repo URL and model number
Docker->>Docker: Clones repository
Docker->>OasisScanner: Runs Oasis Scanner
OasisScanner->>Ollama: Pulls models (mistral, nomic-embed-text)
Ollama-->>OasisScanner: Models ready
OasisScanner->>OasisScanner: Analyzes repository
OasisScanner->>Docker: Generates security reports
Docker->>User: Reports available in volume
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @math-x-io - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider using multi-stage builds to reduce the final image size by not including the build tools.
- It's good practice to specify the application's user in the Dockerfile using the USER instruction.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟡 Security: 1 issue found
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| FROM debian:12.9-slim | ||
|
|
||
| # Set non-root user for better security | ||
| RUN useradd -ms /bin/bash appuser |
There was a problem hiding this comment.
🚨 suggestion (security): Switch to non-root user for improved security.
While the Dockerfile creates a non-root user, it doesn’t switch to that user before executing subsequent commands. Adding a 'USER appuser' instruction after setting up the environment could further restrict privileges during runtime.
|
Fix security issue |
|
@psyray it's okay to merge ? |
You need to reply to my request about Ollama volume to keep persistence on the downloaded models |
Dockerfile
Outdated
| RUN curl -fsSL https://ollama.com/install.sh | sh | ||
|
|
||
| # Pull required models | ||
| RUN ollama pull mistral && ollama pull nomic-embed-text |
There was a problem hiding this comment.
That means that models will be integrated in the docker image ? If so it will raise the image size drastically ...
There was a problem hiding this comment.
I'm not sure if the user intends to extract the models locally and run the image with locally loaded models and save image space, or if they prefer to extract the models directly from the image to solve all the Llama issues.
There was a problem hiding this comment.
Ollama will run directly in the same docker container ? If not the user already have an instance of ollama running, so having models in oasis container does not make sense, you don't even need to install ollama
|
Thanks for your modification, I've changed the target branch to release/0.5.0. |
3 participants
This pull request adds a Dockerfile to the repository, along with detailed instructions in the README.md file on how to build and run it.
Summary by Sourcery
Introduce Docker containerization for the application, including a Dockerfile and instructions for building and running the container.
Build: