Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .env
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ _APP_CONSOLE_GOOGLE_APP_ID=
_APP_CONSOLE_GOOGLE_SECRET=
# MariaDB and MongoDB only start when their profile is listed here; PostgreSQL always
# runs because VectorsDB requires it. MongoDB is on by default for DocumentsDB.
# The embedding container is resource-heavy and off by default; add 'embedding' here to
# start it. The embeddings API is on by default and is switched off below until then.
COMPOSE_PROFILES=mongodb
_APP_DB_ADAPTER=postgresql
_APP_DB_HOST=postgresql
Expand All @@ -68,6 +70,7 @@ _APP_REDIS_HOST=redis
_APP_REDIS_PORT=6379
_APP_REDIS_USER=
_APP_REDIS_PASS=
_APP_EMBEDDING=disabled
_APP_EMBEDDING_ENDPOINT='http://appwrite-embedding:3000/embed'
_APP_EMBEDDING_MODELS=nomic-embed-text
_APP_EMBEDDING_TIMEOUT=30000
Expand Down
9 changes: 8 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ jobs:
{ name: 'Account', runner: runner4, functional: true, processes: null },
{ name: 'Avatars', runner: runner4, functional: true, processes: null },
{ name: 'Console', runner: runner4, functional: true, processes: null },
{ name: 'Databases', runner: runner8, functional: false, processes: 3, documentsdb: true },
{ name: 'Databases', runner: runner8, functional: false, processes: 3, documentsdb: true, embedding: true },
{ name: 'TablesDB', runner: runner8, functional: false, processes: 3 },
{ name: 'Functions', runner: runner4, functional: false, processes: null },
{ name: 'FunctionsSchedule', runner: runner4, functional: true, processes: null },
Expand Down Expand Up @@ -469,6 +469,13 @@ jobs:
COMPOSE_PROFILES="${COMPOSE_PROFILES},gitea"
fi

# The embedding container is resource-heavy and off by default, so start it and

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Shipwright · CRITICAL

The CI workflow conditionally appends _APP_EMBEDDING=enabled to GITHUB_ENV only when matrix.service.embedding is true.

Impact: The CI workflow conditionally appends _APP_EMBEDDING=enabled to GITHUB_ENV only when matrix.service.embedding is true. However, the .env file now hardcodes _APP_EMBEDDING=disabled. If the CI setup sources .env after GITHUB_ENV, or if the compose environment resolution prefers .env over the shell environment, the embedding tests will run against a disabled API and fail. Conversely, if GITHUB_ENV wins, the container p…

Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.

# enable the API only for the suites that exercise the embeddings routes.
if [ "${{ matrix.service.embedding }}" = "true" ]; then
COMPOSE_PROFILES="${COMPOSE_PROFILES},embedding"
echo "_APP_EMBEDDING=enabled" >> $GITHUB_ENV
fi

echo "COMPOSE_PROFILES=${COMPOSE_PROFILES}" >> $GITHUB_ENV

- name: Login to Docker Hub
Expand Down
9 changes: 9 additions & 0 deletions app/config/variables.php
Original file line number Diff line number Diff line change
Expand Up @@ -715,6 +715,15 @@
'question' => '',
'filter' => ''
],
[
'name' => '_APP_EMBEDDING',

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Shipwright · CRITICAL

The default value for _APP_EMBEDDING is 'enabled' in app/config/variables.php, but the .env file shipped with the repository sets _APP_EMBEDDING=disabled.

Impact: The default value for _APP_EMBEDDING is 'enabled' in app/config/variables.php, but the .env file shipped with the repository sets _APP_EMBEDDING=disabled. This creates a dangerous divergence: any deployment that does not explicitly copy the .env value (e.g., uses defaults, or an operator misses this line) will silently enable the embeddings API while the resource-heavy embedding container is NOT started (COMPOSE_PRO…

Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.

'description' => 'Enables the embeddings API, backed by the resource-heavy appwrite-embedding container. That container sits behind the "embedding" Compose profile, so add "embedding" to COMPOSE_PROFILES to start it. Set this to "disabled" to have the /v1/embeddings routes return a service disabled error instead of reaching for the container. Default value is: enabled.',
'introduction' => '2.0.0',
'default' => 'enabled',
'required' => false,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Shipwright · HIGH

The .env comment says 'The embeddings API is on by default and is switched off below until then' but the actual line sets _APP_EMBEDDING=disabled.

Impact: The .env comment says 'The embeddings API is on by default and is switched off below until then' but the actual line sets _APP_EMBEDDING=disabled. This contradicts the variables.php default of 'enabled'. An operator reading the .env comment may believe the API is off, while a fresh install using variables.php defaults will have it on. This documentation/behavior mismatch is a classic source of misconfiguration.

Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.

'question' => '',
'filter' => ''
],
],
],
[
Expand Down
12 changes: 8 additions & 4 deletions app/controllers/shared/api.php
Original file line number Diff line number Diff line change
Expand Up @@ -465,14 +465,18 @@
// installation deploys just the engine backing the platform, so neither is on
// until an operator provisions that engine and says so. Closed to everyone --
// keys and privileged roles included -- rather than answering and then failing
// on the first write with the reason only in the logs.
// against an absent service with the reason only in the logs. Embeddings ran
// on every installation before it had a switch, so it stays on unless an
// operator turns it off; the resource-heavy container is what sits behind a

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Shipwright · HIGH

The refactor of $products from string values to arrays with a default parameter ('documentsdb' => ['_APP_DOCUMENTSDB', 'disabled']) is non-obvious.

Impact: The refactor of $products from string values to arrays with a default parameter ('documentsdb' => ['_APP_DOCUMENTSDB', 'disabled']) is non-obvious. A reader must understand that the second array element is the fallback default for System::getEnv. The spread operator System::getEnv(...$products[$namespace]) obscures the argument order and makes it easy to swap the env var name and default value in future edits. Th…

Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.

// Compose profile.
$products = [
'documentsdb' => '_APP_DOCUMENTSDB',
'vectorsdb' => '_APP_VECTORSDB',
'documentsdb' => ['_APP_DOCUMENTSDB', 'disabled'],
'vectorsdb' => ['_APP_VECTORSDB', 'disabled'],
'embeddings' => ['_APP_EMBEDDING', 'enabled'],
];
if (
isset($products[$namespace])
&& System::getEnv($products[$namespace], 'disabled') !== 'enabled'
&& System::getEnv(...$products[$namespace]) !== 'enabled'
) {
throw new Exception(Exception::GENERAL_SERVICE_DISABLED);
}
Expand Down
19 changes: 6 additions & 13 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ services:
depends_on:
- redis
- ${_APP_DB_HOST:-postgresql}
- appwrite-embedding
- appwrite-geo
environment:
- _APP_ENV
Expand Down Expand Up @@ -260,6 +259,7 @@ services:
- _APP_LIMIT_DATABASE_BATCH
- _APP_DOCUMENTSDB
- _APP_VECTORSDB
- _APP_EMBEDDING
appwrite-console:
logging:
driver: json-file
Expand Down Expand Up @@ -314,7 +314,6 @@ services:
depends_on:
- ${_APP_DB_HOST:-postgresql}
- redis
- appwrite-embedding
environment:
- _APP_ENV
- _APP_USAGE_STATS
Expand Down Expand Up @@ -356,6 +355,7 @@ services:
- _APP_LIMIT_DATABASE_BATCH
- _APP_DOCUMENTSDB
- _APP_VECTORSDB
- _APP_EMBEDDING
- _APP_POOL_ADAPTER=swoole

appwrite-worker:
Expand All @@ -373,7 +373,6 @@ services:
depends_on:
- redis
- ${_APP_DB_HOST:-postgresql}
- appwrite-embedding
volumes:
- appwrite-uploads:/storage/uploads:rw
- appwrite-cache:/storage/cache:rw
Expand Down Expand Up @@ -525,6 +524,7 @@ services:
- _APP_LIMIT_DATABASE_BATCH
- _APP_DOCUMENTSDB
- _APP_VECTORSDB
- _APP_EMBEDDING

appwrite-task-scheduler:
entrypoint: schedule
Expand All @@ -541,7 +541,6 @@ services:
depends_on:
- ${_APP_DB_HOST:-postgresql}
- redis
- appwrite-embedding
environment:
- _APP_ENV
- _APP_USAGE_STATS
Expand Down Expand Up @@ -622,7 +621,6 @@ services:
depends_on:
- redis
- ${_APP_DB_HOST:-postgresql}
- appwrite-embedding
environment:
- _APP_ENV
- _APP_WORKERS_NUM=1
Expand Down Expand Up @@ -662,7 +660,6 @@ services:
depends_on:
- redis
- ${_APP_DB_HOST:-postgresql}
- appwrite-embedding
volumes:
- appwrite-uploads:/storage/uploads:rw
- appwrite-cache:/storage/cache:rw
Expand Down Expand Up @@ -753,7 +750,6 @@ services:
depends_on:
- redis
- ${_APP_DB_HOST:-postgresql}
- appwrite-embedding
environment:
- _APP_ENV
- _APP_WORKERS_NUM=1
Expand Down Expand Up @@ -792,6 +788,7 @@ services:
- _APP_LIMIT_DATABASE_BATCH
- _APP_DOCUMENTSDB
- _APP_VECTORSDB
- _APP_EMBEDDING
appwrite-worker-builds:
profiles:
- separate
Expand All @@ -812,7 +809,6 @@ services:
depends_on:
- redis
- ${_APP_DB_HOST:-postgresql}
- appwrite-embedding
environment:
- _APP_ENV
- _APP_WORKERS_NUM=1
Expand Down Expand Up @@ -1043,7 +1039,6 @@ services:
depends_on:
- redis
- ${_APP_DB_HOST:-postgresql}
- appwrite-embedding
volumes:
- appwrite-config:/storage/config:rw
- appwrite-certificates:/storage/certificates:rw
Expand Down Expand Up @@ -1093,7 +1088,6 @@ services:
depends_on:
- redis
- ${_APP_DB_HOST:-postgresql}
- appwrite-embedding
environment:
- _APP_ENV
- _APP_USAGE_PASS
Expand Down Expand Up @@ -1498,7 +1492,6 @@ services:
depends_on:
- ${_APP_DB_HOST:-postgresql}
- redis
- appwrite-embedding
environment:
- _APP_ENV
- _APP_LOGGING_FORMAT
Expand Down Expand Up @@ -1541,7 +1534,6 @@ services:
depends_on:
- ${_APP_DB_HOST:-postgresql}
- redis
- appwrite-embedding
environment:
- _APP_ENV
- _APP_LOGGING_FORMAT
Expand Down Expand Up @@ -1583,7 +1575,6 @@ services:
depends_on:
- ${_APP_DB_HOST:-postgresql}
- redis
- appwrite-embedding
environment:
- _APP_ENV
- _APP_LOGGING_FORMAT
Expand Down Expand Up @@ -1838,6 +1829,8 @@ services:
- postgres
appwrite-embedding:
image: appwrite/embedding:0.1.0
profiles:
- embedding
environment:
EMBEDDING_MODELS: ${_APP_EMBEDDING_MODELS:-nomic-embed-text}
container_name: appwrite-embedding
Expand Down
Loading