Conversation
…îchies Audit ligne par ligne contre l'usage réel dans xcore/ (ServiceContainer importe déjà chaque service en lazy, gardé par sa config — voir services/container.py) : - pyproject.toml : dependencies réduit au strict noyau (fastapi[standard], pydantic, pyyaml, apscheduler, opentelemetry-api/sdk — tous chargés sans condition au boot, même zero-config). Tout le reste (sqlalchemy/drivers, redis, celery, alembic, exporteur OTLP, python-dotenv) n'était utilisé que si le service correspondant est configuré — déplacé vers des extras : [postgres], [sqlite], [db], [migrations], [redis], [worker], [tracing], [dotenv], [metrics], [all] - BREAKING packaging : `pip install XCoreRuntime` seul n'installe plus ces backends. Voir CHANGELOG.md pour la liste complète + la migration. - prometheus-client était importé par du code noyau (metrics.py, xcore/__init__.py) mais n'existait qu'en dev dependencies — jamais disponible en install de prod. Corrigé via l'extra [metrics]. - sqlalchemy[asyncio] explicite dans chaque extra DB — dépendait silencieusement de aiosqlite pour fournir greenlet en transitif, ce qui cassait une install postgres-only. - Versions rafraîchies : fastapi 0.135→0.141, pydantic 2.11→2.13, sqlalchemy 2.0→2.1, redis borne haute 8→9, apscheduler→3.11.3, opentelemetry-*→1.45, alembic→1.20, dotenv→1.2, psycopg2→2.9.13, prometheus-client 0.25→0.26. - Tous les extras dupliqués dans [tool.poetry.group.dev.dependencies] : la CI fait `poetry install --with dev` sans --extras, donc les tests qui exercent ces backends ont besoin des paquets là aussi. - Version : 2.6.0 → 2.7.0 (packaging change, potentiellement cassant pour l'install par défaut). 1539 tests passants, 1 échec pré-existant non lié (bug Studio hors scope).
Contributor
Reviewer's GuideVersion 2.7.0 reduces the default installation to dependencies needed by import and zero-config boot, moves optional backends into explicit extras, refreshes dependency constraints and the lockfile, and documents the breaking packaging change. Sequence diagram for conditional backend loadingsequenceDiagram
participant Boot as XCore boot
participant Container as ServiceContainer
participant Config as Service config
participant Backend as Optional backend
Boot->>Container: init()
Container->>Config: Read enabled service configuration
alt backend configured
Container->>Backend: Lazy import and initialization
Backend-->>Container: Service instance
else zero-config or default memory backend
Container-->>Boot: Core services only
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
github-actions
Bot
added
documentation
Contributor
|
|
Contributor
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 3 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="pyproject.toml" line_range="36-42" />
<code_context>
- "redis[hiredis]>=7.0.0,<8.0.0",
- "apscheduler>=3.11.0,<4.0.0",
- "python-dotenv>=1.1.0,<2.0.0",
+ "fastapi[standard]>=0.141.1,<1.0.0",
+ "pydantic>=2.13.5,<3.0.0",
"pyyaml>=6.0.3,<7.0.0",
- "celery>=5.6.3,<6.0.0",
- "opentelemetry-api>=1.27.0,<2.0.0",
- "opentelemetry-sdk>=1.27.0,<2.0.0",
- "opentelemetry-exporter-otlp-proto-http>=1.27.0,<2.0.0",
+ "apscheduler>=3.11.3,<4.0.0",
+ "opentelemetry-api>=1.45.0,<2.0.0",
+ "opentelemetry-sdk>=1.45.0,<2.0.0",
]
# ── Dépendances optionnelles ─────────────────────────────────────────────────────
</code_context>
<issue_to_address>
**issue (broader_impact):** A zero-config boot without the `redis` extra raises `ModuleNotFoundError` when `CacheServiceProvider` imports `xcore.services.cache.service`: the package initializer eagerly imports `RedisCacheBackend`, which imports Redis even though the configured backend is `memory`.
**Triggers:** When installing the package without `[redis]` and starting the default zero-config runtime.
**Suggested fix:** Remove the eager `RedisCacheBackend` import from `xcore/services/cache/__init__.py` and expose it lazily only when the Redis backend is selected.
</issue_to_address>
### Comment 2
<location path="pyproject.toml" line_range="68-69" />
<code_context>
-xcli = ["xcorecli>=1.1.0"]
-cpp = ["xcorescanner>=0.1.0"]
-all = ["xcdk>=0.1.0", "xcorecli>=1.1.0", "xcorescanner>=0.1.0"]
+postgres = ["sqlalchemy[asyncio]>=2.1.1,<3.0.0", "psycopg2>=2.9.13,<3.0.0"]
+sqlite = ["sqlalchemy[asyncio]>=2.1.1,<3.0.0", "aiosqlite>=0.22.1,<1.0.0"]
+db = ["sqlalchemy[asyncio]>=2.1.1,<3.0.0", "psycopg2>=2.9.13,<3.0.0", "aiosqlite>=0.22.1,<1.0.0"]
+migrations = ["alembic>=1.20.0,<2.0.0"]
</code_context>
<issue_to_address>
**issue (bug_risk):** The `postgres` extra does not install `asyncpg`, although the asynchronous PostgreSQL adapter and `MigrationRunner` explicitly support `postgresql+asyncpg://` URLs; `create_async_engine()` then fails because the selected async driver is missing.
**Triggers:** When a configured database uses `postgresql+asyncpg://` or another asynchronous PostgreSQL URL with only `[postgres]` installed.
**Suggested fix:** Add `asyncpg` to the PostgreSQL extra (and to `db`, `all`, and the duplicated dev dependencies), or change the extra contract to support only synchronous `psycopg2` URLs.
</issue_to_address>
### Comment 3
<location path="pyproject.toml" line_range="71" />
<code_context>
+postgres = ["sqlalchemy[asyncio]>=2.1.1,<3.0.0", "psycopg2>=2.9.13,<3.0.0"]
+sqlite = ["sqlalchemy[asyncio]>=2.1.1,<3.0.0", "aiosqlite>=0.22.1,<1.0.0"]
+db = ["sqlalchemy[asyncio]>=2.1.1,<3.0.0", "psycopg2>=2.9.13,<3.0.0", "aiosqlite>=0.22.1,<1.0.0"]
+migrations = ["alembic>=1.20.0,<2.0.0"]
+redis = ["redis[hiredis]>=7.0.0,<9.0.0"]
+worker = ["celery>=5.6.3,<6.0.0"]
</code_context>
<issue_to_address>
**issue (bug_risk):** Installing only `[migrations]` leaves SQLAlchemy unavailable, but importing `MigrationRunner` executes top-level `from sqlalchemy.engine import make_url` and `from sqlalchemy.ext.asyncio import create_async_engine`, so the migration feature fails before Alembic's intended missing-dependency error can run.
**Triggers:** When a consumer installs `[migrations]` without also installing `[postgres]`, `[sqlite]`, or `[db]`.
**Suggested fix:** Include `sqlalchemy[asyncio]` in the `migrations` extra, or move the SQLAlchemy imports inside the code paths that use them and document the required database extra.
```suggestion
migrations = ["alembic>=1.20.0,<2.0.0", "sqlalchemy[asyncio]>=2.1.1,<3.0.0"]
```
</issue_to_address>Sourcery assessment
Approval pending. 3 findings to address first.
Blocking findings: pyproject.toml:42, pyproject.toml:69, pyproject.toml:71
Comment on lines
+36
to
42
| "fastapi[standard]>=0.141.1,<1.0.0", | ||
| "pydantic>=2.13.5,<3.0.0", | ||
| "pyyaml>=6.0.3,<7.0.0", | ||
| "celery>=5.6.3,<6.0.0", | ||
| "opentelemetry-api>=1.27.0,<2.0.0", | ||
| "opentelemetry-sdk>=1.27.0,<2.0.0", | ||
| "opentelemetry-exporter-otlp-proto-http>=1.27.0,<2.0.0", | ||
| "apscheduler>=3.11.3,<4.0.0", | ||
| "opentelemetry-api>=1.45.0,<2.0.0", | ||
| "opentelemetry-sdk>=1.45.0,<2.0.0", | ||
| ] |
Contributor
There was a problem hiding this comment.
issue (broader_impact): A zero-config boot without the redis extra raises ModuleNotFoundError when CacheServiceProvider imports xcore.services.cache.service: the package initializer eagerly imports RedisCacheBackend, which imports Redis even though the configured backend is memory.
Triggers: When installing the package without [redis] and starting the default zero-config runtime.
Suggested fix: Remove the eager RedisCacheBackend import from xcore/services/cache/__init__.py and expose it lazily only when the Redis backend is selected.
Comment on lines
+68
to
+69
| postgres = ["sqlalchemy[asyncio]>=2.1.1,<3.0.0", "psycopg2>=2.9.13,<3.0.0"] | ||
| sqlite = ["sqlalchemy[asyncio]>=2.1.1,<3.0.0", "aiosqlite>=0.22.1,<1.0.0"] |
Contributor
There was a problem hiding this comment.
issue (bug_risk): The postgres extra does not install asyncpg, although the asynchronous PostgreSQL adapter and MigrationRunner explicitly support postgresql+asyncpg:// URLs; create_async_engine() then fails because the selected async driver is missing.
Triggers: When a configured database uses postgresql+asyncpg:// or another asynchronous PostgreSQL URL with only [postgres] installed.
Suggested fix: Add asyncpg to the PostgreSQL extra (and to db, all, and the duplicated dev dependencies), or change the extra contract to support only synchronous psycopg2 URLs.
| postgres = ["sqlalchemy[asyncio]>=2.1.1,<3.0.0", "psycopg2>=2.9.13,<3.0.0"] | ||
| sqlite = ["sqlalchemy[asyncio]>=2.1.1,<3.0.0", "aiosqlite>=0.22.1,<1.0.0"] | ||
| db = ["sqlalchemy[asyncio]>=2.1.1,<3.0.0", "psycopg2>=2.9.13,<3.0.0", "aiosqlite>=0.22.1,<1.0.0"] | ||
| migrations = ["alembic>=1.20.0,<2.0.0"] |
Contributor
There was a problem hiding this comment.
issue (bug_risk): Installing only [migrations] leaves SQLAlchemy unavailable, but importing MigrationRunner executes top-level from sqlalchemy.engine import make_url and from sqlalchemy.ext.asyncio import create_async_engine, so the migration feature fails before Alembic's intended missing-dependency error can run.
Triggers: When a consumer installs [migrations] without also installing [postgres], [sqlite], or [db].
Suggested fix: Include sqlalchemy[asyncio] in the migrations extra, or move the SQLAlchemy imports inside the code paths that use them and document the required database extra.
Suggested change
| migrations = ["alembic>=1.20.0,<2.0.0"] | |
| migrations = ["alembic>=1.20.0,<2.0.0", "sqlalchemy[asyncio]>=2.1.1,<3.0.0"] |
6 tasks done
traoreera
added a commit
that referenced
this pull request
Sep 28, 2026
…rbitraire (#281) ##⚠️ Correctif de sécurité — priorité haute Analyse **dynamique** du sandbox (pas de relecture statique seule) : deux plugins `sandboxed` réels écrits et exécutés contre un vrai process xcore, en tentant les techniques d'évasion sandbox Python connues. Détail complet, PoC, et vérification post-correctif dans `reports/sandbox_dynamic_security_analysis_2026-09-28.md`. ## 3 vulnérabilités critiques confirmées par exécution réelle, puis corrigées 1. **`asyncio.create_subprocess_exec`/`_shell`** — `asyncio` n'était sur aucune des deux listes de modules interdits (scanner statique + guard runtime). 2. **`().__class__.__bases__[0].__subclasses__()` → `subprocess.Popen`** déjà chargé en mémoire — contourne à la fois le scan AST statique (aucun mot interdit littéral dans le code) et le guard d'import runtime (aucun `import` jamais exécuté). 3. **`import posix` dynamique** — listé dans `DEFAULT_FORBIDDEN` (scanner statique) mais absent de `_FORBIDDEN_MODULES` (guard runtime) ; `posix` expose des primitives quasi équivalentes à `os` (`fork`, `execve`...). Les trois ont été exploitées avec succès (exécution de `id`/fork réel sur l'hôte) avant correctif. ## Correctif (`xcore/kernel/sandbox/worker.py`) - `pwd`/`grp`/`posix` ajoutés à `_FORBIDDEN_MODULES` (ferme #3 au niveau import). - Nouvelle couche 5 (`_install_subprocess_guard`) : patch **direct des objets déjà en mémoire** plutôt que du seul mécanisme d'import — `subprocess.Popen.__init__`, `subprocess.call`/`run`/`check_call`/`check_output`, toute la famille `os.fork`/`exec*`/`spawn*`/`posix_spawn*`/`system`/`popen`, et `asyncio.create_subprocess_exec`/`_shell`. Approche volontairement plus large que le strict minimum : elle ferme aussi #2, qu'un simple ajout à une liste de modules interdits n'aurait pas pu fermer (aucun `import` n'y est jamais exécuté). ## Test plan - [x] Les 3 PoC re-testés après correctif : tous bloqués (`PermissionError: [sandbox] ... interdit dans le sandbox`) - [x] Usage légitime d'`asyncio` sans subprocess (`asyncio.sleep`) : non affecté, vérifié - [x] Limites mémoire (`RLIMIT_DATA`/`RLIMIT_RSS`) : vérifiées efficaces par la même campagne de tests (hypothèse initiale qu'elles seraient inefficaces sur Linux moderne — infirmée empiriquement) - [x] `FilesystemGuard` (lecture/écriture hors `allowed_paths`) : comportement inchangé - [x] `poetry run pytest tests/` — 1310 passed, 1 skipped - [x] `make lint-check` — clean Version : 2.6.0 → **2.6.1** (patch de sécurité, indépendant de la PR #280 dépendances en cours). ## Summary by Sourcery Harden plugin isolation by closing confirmed sandbox escape vectors and defaulting unspecified plugins to the restrictive sandboxed execution mode. Bug Fixes: - Block three confirmed sandbox escape paths that enabled arbitrary host command execution through asyncio subprocess APIs, inherited subprocess classes, and dynamic posix imports. Enhancements: - Change the implicit plugin execution mode default from trusted-equivalent legacy to the fail-closed sandboxed mode. - Add direct runtime protection for process creation and execution primitives while preserving legitimate non-subprocess asyncio usage. Build: - Bump the project version to 2.6.2. Documentation: - Document the dynamic sandbox security findings, remediation, validation results, and remaining isolation limitations. - Update the changelog and roadmap with the sandbox hardening and execution-mode default changes. Tests: - Update manifest tests to expect sandboxed as the default execution mode. - Revalidate sandbox escape proofs of concept and confirm existing filesystem, resource-limit, asyncio, and project test behavior.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Résumé
Audit ligne par ligne de chaque dépendance contre son usage réel dans
xcore/—ServiceContainer(xcore/services/container.py) importe déjà chaque service en lazy (from .database.manager import ...à l'intérieur de la méthodeinit()), gardé par un early-return sur la config correspondante. Résultat : la plupart des dépendances "core" actuelles ne sont en réalité utilisées que si le service correspondant est configuré.dependenciesréduit au strict noyau — chargé sans condition, même zero-config :fastapi[standard],pydantic,pyyamlapscheduler(SchedulerConfig.enabled=Truepar défaut → tourne sans config)opentelemetry-api/-sdk(importés au niveau module detracing.py, chargé au boot)Tout le reste déplacé en extras :
postgressqlalchemy[asyncio]+psycopg2postgresql://dansservices.databasessqlitesqlalchemy[asyncio]+aiosqlitesqlite+aiosqlite://dbmigrationsalembicMigrationRunner(déjà importé à la demande)redisredis[hiredis]cache/schedulerbackendredis|tieredworkerceleryservices.xworker.enabled: true(False par défaut)tracingobservability.tracing.endpointconfigurédotenvpython-dotenvmetricsprometheus-clientobservability.metrics.backend: prometheus— corrige un vrai trou : importé par du code noyau (metrics.py,xcore/__init__.py) mais absent des deps de prod jusqu'iciallsdk/xcli/cpppip install XCoreRuntimeseul n'installe plus ces backends. Migration : ajouter l'extra correspondant (ex:XCoreRuntime[sqlite]pour un dev local).Autres corrections en passant :
sqlalchemy[asyncio]explicite (l'extraasynciofournitgreenlet) — c'étaitaiosqlitequi le fournissait en transitif par accident, ce qui aurait cassé une installpostgres-only.[tool.poetry.group.dev.dependencies]: la CI faitpoetry install --with devsans--extras, donc les tests qui exercent ces backends ont besoin des paquets là aussi.Versions rafraîchies : fastapi 0.135→0.141, pydantic 2.11→2.13, sqlalchemy 2.0→2.1, redis (borne haute 8→9), apscheduler→3.11.3, opentelemetry-*→1.45, alembic→1.20, python-dotenv→1.2, psycopg2→2.9.13, prometheus-client 0.25→0.26.
Version : 2.6.0 → 2.7.0.
Test plan
poetry install --with dev+import xcore— OKpoetry run pytest tests/— 1539 passed, 1 skipped, 1 échec pré-existant non lié (bugRouterIn()/get_router()dans le générateur Studio, hors scope de cette PR)make lint-check— cleanpoetry lockrégénéré et cohérent (pas de conflit de contraintes)Summary by Sourcery
Move backend-specific dependencies to opt-in extras while retaining a minimal zero-configuration core and updating supported dependency versions.
New Features:
Bug Fixes:
Enhancements:
Build:
Documentation: