feat(server): add a readme file to the Upload folder#17055
feat(server): add a readme file to the Upload folder#17055aviv926 wants to merge 30 commits intoimmich-app:mainfrom
Conversation
|
Label error. Requires exactly 1 of: changelog:.*. Found: 🗄️server. A maintainer will add the required label. |
|
|
||
| echo "Initializing Immich $IMMICH_SOURCE_REF" | ||
|
|
||
| cat <<EOF > /usr/src/app/upload/upload/README.txt |
There was a problem hiding this comment.
| cat <<EOF > /usr/src/app/upload/upload/README.txt | |
| cat << 'EOF' > /usr/src/app/upload/upload/README.txt |
I don't think there is a need for variable interpolation in this block.
Is there a way we can refer to the WORKDIR instead of hard coding the /usr/src... patch that will probably change in the future?
There was a problem hiding this comment.
I don't have enough knowledge to make this happen, maybe one of the developers can help with this.
There was a problem hiding this comment.
Tbh I would just not do this. I'd just add a comment in the compose (and maybe .env?) file.
|
I definitely think we need a file in the |
I also think it's better. |
|
I don't think hardcoding the path in the start.sh script is the way to go for this. The path can change (for example, the AIO image uses a different one). I think a better place to handle this is probably in the storage service bootstrap, where we also manage the .immich files. https://github.com/immich-app/immich/blob/main/server/src/services/storage.service.ts |
Okay, I'm not familiar with ts, so I used GPT (I'll completely understand if you don't want to merge it). From what I've tested, the code does indeed create a README.TXT file in the Upload and Library folders. |
| await this.verifyReadAccess(folder); | ||
| await this.verifyWriteAccess(folder); | ||
|
|
||
| // Ensure README.TXT is created in UPLOAD and LIBRARY folders only |
There was a problem hiding this comment.
This comment is unnecessary.
| try { | ||
| let exists = false; | ||
| try { | ||
| await this.storageRepository.stat(readmePath); |
There was a problem hiding this comment.
Pretty sure there is an exists/existsSync call you may use instead.
| @@ -63,6 +68,38 @@ export class StorageService extends BaseService { | |||
| } | |||
|
|
|||
| @OnJob({ name: JobName.DELETE_FILES, queue: QueueName.BACKGROUND_TASK }) | |||
There was a problem hiding this comment.
createReadmeFile absolutely shouldn't run on DELETE_FILES lol
There was a problem hiding this comment.
That should be on the next function down instead. It getting separated didn't cause any test failures, which suggests maybe we're missing some coverage there?
There was a problem hiding this comment.
Job queueing tests are quite limited still, yea :(
| try { | ||
| await this.storageRepository.stat(readmePath); | ||
| exists = true; | ||
| } catch (error) { |
There was a problem hiding this comment.
Nested exceptions are imo always ugly and avoidable in almost all cases. Could you refactor this?
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
This reverts commit ae02b41.
This reverts commit 12fe5ec.
| password: ${{ secrets.GHCR_PAT }} | ||
|
|
||
| - name: Build and push server image | ||
| uses: docker/build-push-action@v6 |
Check failure
Code scanning / zizmor
action is not pinned to a hash (required by blanket policy) Error
|
|
||
| - name: Log in to GHCR | ||
| if: env.exists == 'false' | ||
| uses: docker/login-action@v3 |
Check failure
Code scanning / zizmor
action is not pinned to a hash (required by blanket policy) Error
| run: | | ||
| git branch -f dockerimageML ${{ steps.get_release.outputs.tag }} | ||
| git push origin dockerimageML --force |
Check notice
Code scanning / zizmor
steps.get_release.outputs.tag may expand into attacker-controllable code Note
| - name: Checkout `dockerimageML` | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| ref: dockerimageML |
Check warning
Code scanning / zizmor
does not set persist-credentials: false Warning
| ref: dockerimageML | ||
|
|
||
| - name: Set up Docker Buildx | ||
| uses: docker/setup-buildx-action@v3 |
Check failure
Code scanning / zizmor
action is not pinned to a hash (required by blanket policy) Error
| workflow_dispatch: | ||
|
|
||
| permissions: | ||
| contents: write |
Check failure
Code scanning / zizmor
contents: write is overly broad at the workflow level Error
|
|
||
| permissions: | ||
| contents: write | ||
| packages: write |
Check failure
Code scanning / zizmor
packages: write is overly broad at the workflow level Error
| - name: Check out this repo | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| fetch-depth: 0 |
Check warning
Code scanning / zizmor
does not set persist-credentials: false Warning
| - name: Checkout `dockerimageML` | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| ref: dockerimageML |
Check warning
Code scanning / zizmor
does not set persist-credentials: false Warning
| ref: dockerimageML | ||
|
|
||
| - name: Set up Docker Buildx | ||
| uses: docker/setup-buildx-action@v3 |
Check failure
Code scanning / zizmor
action is not pinned to a hash (required by blanket policy) Error
| uses: docker/setup-buildx-action@v3 | ||
|
|
||
| - name: Log in to GHCR | ||
| uses: docker/login-action@v3 |
Check failure
Code scanning / zizmor
action is not pinned to a hash (required by blanket policy) Error
| password: ${{ secrets.GHCR_PAT }} | ||
|
|
||
| - name: Build & push ML image (${{ matrix.device }}) | ||
| uses: docker/build-push-action@v6 |
Check failure
Code scanning / zizmor
action is not pinned to a hash (required by blanket policy) Error
|
We have discussed this again and decided we don't want it after all. We already have plenty of warnings in various places. Sorry for having you waste some time on this. |
5 participants
Description
Many users use Immich for a quick and convenient replacement for G photos. Sometimes these are users for whom Immich is their first self-host application, so mistakes can happen.
To this end, I believe there is a real need to add a file that warns inexperienced users about the danger of touching folders created by Immich in order to prevent any data loss / problems with the proper functioning of the system.
To this end, this PR focuses on adding a README.TXT file to the Upload folder in order to warn users before making changes to this folder.
How Has This Been Tested?
I edited the file on the current system where Immich is running and checked that the README.TXT file was indeed created in its intended location (Upload) and the system continues to operate without a problem.
Checklist:
src/services/uses repositories implementations for database calls, filesystem operations, etc.src/repositories/is pretty basic/simple and does not have any immich specific logic (that belongs insrc/services/)Note:
Since Immich allows the user to choose whether to use the Upload folder for source files or the Library folder, should we consider adding an identical file there as well?