Skip to content
Merged
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
9 changes: 9 additions & 0 deletions features/F39-persistence-path-root-hardening/CHECKLIST.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# F39 Checklist

- [x] SPEC da F39 criada e validavel
- [x] REDs adicionados para banco e artifacts fora do workspace ou escapando por symlink
- [x] `runs_db_path` e `artifacts_dir` passam a respeitar raiz confiavel do workspace
- [x] `runs submit`, `runs list`, `runs show` e `doctor` retornam erro previsivel para configuracao invalida
- [x] Quality gate local relevante executado
- [x] Review de seguranca local registrado
- [x] REPORT da frente consolidado
5 changes: 5 additions & 0 deletions features/F39-persistence-path-root-hardening/NOTES.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# F39 Notes

- A `F39` aplica ao banco de runs e ao diretório base de artifacts o mesmo trust-root ja usado em `F24` e `F38`.
- Os campos brutos de configuracao foram mantidos por compatibilidade; o boundary passa a ser imposto por accessors resolvidos em `AppSettings`.
- O recorte nao altera schema, nao depende de Docker e nao muda a superficie visual da TUI.
50 changes: 50 additions & 0 deletions features/F39-persistence-path-root-hardening/REPORT.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
# F39 Report

## Resumo executivo

- A `F39-persistence-path-root-hardening` endurece `runs_db_path` e `artifacts_dir` contra escapes do `workspace_root`.
- O boundary passa a ser aplicado em `AppSettings` e consumido pelos entrypoints de CLI, worker e dashboard, preservando o AIgnt-Synapse-Flow como a engine propria de pipeline do AIgnt OS.
- O recorte permaneceu pequeno e independente da frente visual: sem alterar watch mode como produto, sem migracao de schema e sem tocar auth remota.

## Escopo alterado

- `AppSettings` passa a expor `runs_db_path_resolved` e `artifacts_dir_resolved`
- `cli/app.py`, `runtime/worker.py` e `cli/dashboard.py` passam a consumir apenas os paths resolvidos
- `doctor` passa a reprovar explicitamente configuracao de persistencia fora do `workspace_root`
- fixtures de integracao de runs foram alinhadas para usar persistencia dentro do root confiavel
- `SPEC.md`, `NOTES.md`, `CHECKLIST.md` e este `REPORT.md` foram adicionados para a `F39`

## Validacoes executadas

- `validate_spec_file(Path("features/F39-persistence-path-root-hardening/SPEC.md"))`
- `pytest tests/unit/test_config.py tests/integration/test_runs_submit_cli.py tests/integration/test_runs_cli.py tests/integration/test_doctor_cli.py -q`
- `ruff check src/aignt_os/config.py src/aignt_os/cli/app.py src/aignt_os/runtime/worker.py src/aignt_os/cli/dashboard.py tests/unit/test_config.py tests/integration/test_runs_submit_cli.py tests/integration/test_runs_cli.py tests/integration/test_doctor_cli.py`
- `mypy src/aignt_os/config.py src/aignt_os/cli/app.py src/aignt_os/runtime/worker.py src/aignt_os/cli/dashboard.py`
- `./scripts/commit-check.sh --no-sync --skip-branch-validation --skip-docker --skip-security`
- `./scripts/security-gate.sh`

## Review de seguranca

- Parecer final: aprovado.
- Riscos revisados:
- banco SQLite configurado fora do `workspace_root`
- diretório de artifacts escapando por path absoluto ou por symlink
- CLIs de runs e doctor emitindo traceback cru para erro configuracional
- Mitigacoes aplicadas:
- canonicalizacao de `runs_db_path` e `artifacts_dir` com `resolve_path_within_root`
- tradução consistente de `ValueError` em `Environment error:` nos entrypoints de CLI
- integração ajustada para declarar `AIGNT_OS_WORKSPACE_ROOT` coerente com os paths persistidos

## Riscos residuais

- O boundary depende de `workspace_root` coerente no ambiente; configuracoes divergentes agora falham cedo, mas a escolha do root continua responsabilidade operacional do chamador.
- O diretório de artifacts ainda pode conter arquivos problemáticos ja persistidos anteriormente; esta frente endurece a configuracao de entrada, nao saneia retroativamente dados legados.

## Proximos passos

- Se houver novo hardening pequeno fora da TUI, o proximo slice natural e revisar outros boundaries configuraveis ainda brutos.
- Fora isso, a fila independente pode voltar para backlog funcional apos absorver esta protecao de persistencia.

## Status final da frente

- `READY_FOR_COMMIT`
105 changes: 105 additions & 0 deletions features/F39-persistence-path-root-hardening/SPEC.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
---
id: F39-persistence-path-root-hardening
type: feature
summary: Endurecer os paths de persistencia de runs e artifacts para a raiz confiavel do workspace.
inputs:
- CONTEXT.md
- docs/architecture/SDD.md
- docs/architecture/TDD.md
- docs/architecture/SPEC_FORMAT.md
- src/aignt_os/config.py
- src/aignt_os/cli/app.py
- src/aignt_os/runtime/worker.py
- src/aignt_os/cli/dashboard.py
- tests/unit/test_config.py
- tests/integration/test_runs_submit_cli.py
- tests/integration/test_runs_cli.py
- tests/integration/test_doctor_cli.py
outputs:
- trusted_persistence_paths
- persistence_boundary_red_tests
constraints:
- "manter o AIgnt-Synapse-Flow como a engine propria de pipeline do AIgnt OS"
- "restringir a frente a `runs_db_path` e `artifacts_dir`, sem tocar TUI como produto, auth remota ou transporte"
- "preservar a CLI publica atual, mudando apenas o erro configuracional para paths de persistencia fora da raiz confiavel"
- "nao exigir DOCKER_PREFLIGHT porque a frente nao depende de Docker, build, boot em container ou integracao externa"
acceptance_criteria:
- "`AIGNT_OS_RUNS_DB_PATH` passa a resolver apenas para um path canonico dentro de `workspace_root`, rejeitando escapes por path absoluto fora da raiz ou por symlink."
- "`AIGNT_OS_ARTIFACTS_DIR` passa a resolver apenas para um path canonico dentro de `workspace_root`, rejeitando escapes por path absoluto fora da raiz ou por symlink."
- "`aignt runs submit`, `aignt runs list` e `aignt runs show` falham com `Environment error:` previsivel quando o banco ou artifacts configurados escapam da raiz confiavel."
- "`aignt doctor` marca `runs_db` e `artifacts_dir` como `fail` sem traceback quando a configuracao de persistencia escapa do `workspace_root`."
- "Existe cobertura unitaria e de integracao para configuracao valida, path invalido fora do workspace e escape por symlink."
non_goals:
- "alterar `runtime_state_dir`, que ja foi endurecido na F38"
- "mudar a experiencia visual da TUI ou ampliar watch mode"
- "introduzir migracao de schema, nova persistencia ou mudancas de produto fora do boundary de path"
dependencies:
- F24-workspace-boundary-hardening
- F38-runtime-state-root-hardening
---

# Contexto

O projeto ja protege alguns boundaries de filesystem com `workspace_root`, inclusive SPECs,
artifacts salvos por run e o state-dir compartilhado do runtime. Ainda faltava alinhar os
dois pontos centrais de persistencia configuravel: `runs_db_path` e `artifacts_dir`.

Essa lacuna permite que a configuracao aponte para um banco SQLite ou diretorio de artifacts
fora da raiz confiavel do workspace, o que quebra a consistencia do boundary operacional do
AIgnt-Synapse-Flow, a engine propria de pipeline do AIgnt OS.

# Objetivo

Aplicar o mesmo boundary confiavel de `workspace_root` aos paths configuraveis de banco de
runs e diretório base de artifacts, mantendo erros previsiveis na CLI e sem ampliar escopo.

# Escopo

## Incluido

- adicionar accessors resolvidos em `AppSettings` para banco de runs e diretório de artifacts
- atualizar os consumidores principais de persistencia para usar apenas paths resolvidos
- falhar cedo com `Environment error:` nas CLIs publicas quando a configuracao escapar da raiz confiavel
- ajustar fixtures de integracao que antes usavam paths fora do `workspace_root`

## Fora de escopo

- mudar `runtime_state_dir` ou auth registry
- alterar o comportamento visual da TUI
- migracao de schema SQLite, novo layout de artifacts ou novas features de observabilidade

# Casos de erro

- `runs_db_path` absoluto fora de `workspace_root` continuar aceito
- `artifacts_dir` symlinkado dentro do workspace escapar para target externo
- `runs list` ou `runs show` exibirem traceback cru em vez de erro de ambiente previsivel
- `doctor` continuar marcando ambiente como saudavel quando a persistencia configurada escapa da raiz

# Cenarios verificaveis

## Cenario 1: paths validos seguem funcionais

- Dado `workspace_root` configurado para o diretório temporario do teste
- E `runs_db_path` e `artifacts_dir` configurados dentro dessa raiz
- Quando a configuracao for resolvida
- Entao a persistencia continua funcional sem mudar a superficie publica

## Cenario 2: banco fora da raiz confiavel e rejeitado

- Dado `workspace_root` configurado
- E `runs_db_path` apontando para um path absoluto fora dessa raiz
- Quando `aignt runs submit` ou `aignt runs list` forem executados
- Entao a CLI falha com `Environment error:` previsivel

## Cenario 3: artifacts nao permitem escape por symlink

- Dado `artifacts_dir` configurado como symlink dentro do workspace
- E o target real apontando para fora da raiz confiavel
- Quando a configuracao for resolvida ou consumida pela CLI
- Entao o path e rejeitado como escape do root confiável

# Observacoes

Esta frente endurece apenas o boundary de configuracao para persistencia local de runs e
artifacts. Ela nao reabre a frente visual da TUI, nao altera runtime remoto e nao mexe em
schema de dados existente.
82 changes: 56 additions & 26 deletions src/aignt_os/cli/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -182,18 +182,40 @@ def _path_preparation_failure(path: Path, *, expects_directory: bool) -> str | N


def _collect_doctor_checks(settings: AppSettings) -> list[dict[str, str]]:
return [
_runtime_state_doctor_check(settings),
_persistence_doctor_check(
try:
runs_db_check = _persistence_doctor_check(
name="runs_db",
target=settings.runs_db_path,
target=settings.runs_db_path_resolved,
expects_directory=False,
),
_persistence_doctor_check(
)
except ValueError as exc:
runs_db_check = _doctor_check(
name="runs_db",
status="fail",
target=settings.runs_db_path,
message=str(exc),
next_step="Fix the configured path or permissions before submitting a run.",
)

try:
artifacts_dir_check = _persistence_doctor_check(
name="artifacts_dir",
target=settings.artifacts_dir,
target=settings.artifacts_dir_resolved,
expects_directory=True,
),
)
except ValueError as exc:
artifacts_dir_check = _doctor_check(
name="artifacts_dir",
status="fail",
target=settings.artifacts_dir,
message=str(exc),
next_step="Fix the configured path or permissions before submitting a run.",
)

return [
_runtime_state_doctor_check(settings),
runs_db_check,
artifacts_dir_check,
]


Expand Down Expand Up @@ -228,12 +250,18 @@ def _runtime_service() -> RuntimeService:

def _run_repository() -> RunRepository:
settings = AppSettings()
return RunRepository(settings.runs_db_path)
try:
return RunRepository(settings.runs_db_path_resolved)
except ValueError as exc:
raise environment_error(str(exc)) from exc


def _artifact_store() -> ArtifactStore:
settings = AppSettings()
return ArtifactStore(settings.artifacts_dir)
try:
return ArtifactStore(settings.artifacts_dir_resolved)
except ValueError as exc:
raise environment_error(str(exc)) from exc


def _validate_preview_target(preview_target: str) -> tuple[str, str | None]:
Expand Down Expand Up @@ -326,13 +354,13 @@ def _resolve_run_preview(

def _dispatch_service(*, initiated_by: str | None = None) -> RunDispatchService:
settings = AppSettings()
repository = RunRepository(settings.runs_db_path)
artifact_store = ArtifactStore(settings.artifacts_dir)
runner = PersistedPipelineRunner(
repository=repository,
artifact_store=artifact_store,
)
try:
repository = RunRepository(settings.runs_db_path_resolved)
artifact_store = ArtifactStore(settings.artifacts_dir_resolved)
runner = PersistedPipelineRunner(
repository=repository,
artifact_store=artifact_store,
)
runtime_service = RuntimeService(settings.runtime_state_file)
except ValueError as exc:
raise environment_error(str(exc)) from exc
Expand Down Expand Up @@ -599,23 +627,28 @@ def watch(
"""
from aignt_os.cli.dashboard import RunDashboard

repo = _run_repository()
try:
repo = _run_repository()
if not repo.get_run(run_id):
typer.echo(f"Error: Run {run_id} not found.", err=True)
raise typer.Exit(code=1)
except NoResultFound:
typer.echo(f"Error: Run {run_id} not found.", err=True)
raise typer.Exit(code=1) from None
except CLIError as exc:
exit_for_cli_error(exc)

app = RunDashboard(run_id=run_id, refresh_interval=refresh)
app.run()


@runs_app.command("list")
def runs_list() -> None:
repository = _run_repository()
runs = repository.list_runs()
try:
repository = _run_repository()
runs = repository.list_runs()
except CLIError as exc:
exit_for_cli_error(exc)
render_runs_list(runs)


Expand Down Expand Up @@ -678,15 +711,10 @@ def runs_show(
run_id: str,
preview: Annotated[str | None, typer.Option("--preview")] = None,
) -> None:
repository = _run_repository()
artifact_store = _artifact_store()

try:
repository = _run_repository()
artifact_store = _artifact_store()
run = repository.get_run(run_id)
except NoResultFound:
exit_for_cli_error(not_found_error(f"Run '{run_id}' not found."))

try:
resolved_preview = (
_resolve_run_preview(
run_id=run_id,
Expand All @@ -699,6 +727,8 @@ def runs_show(
)
except CLIError as exc:
exit_for_cli_error(exc)
except NoResultFound:
exit_for_cli_error(not_found_error(f"Run '{run_id}' not found."))

render_run_detail(
run,
Expand Down
2 changes: 1 addition & 1 deletion src/aignt_os/cli/dashboard.py
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ def __init__(self, run_id: str, refresh_interval: float = 1.0) -> None:
self.run_id = run_id
self.refresh_interval = refresh_interval
settings = AppSettings()
self.repository = RunRepository(settings.runs_db_path)
self.repository = RunRepository(settings.runs_db_path_resolved)
self.run_header = RunHeader()
self.step_list = ListView(id="step_list")
self.step_detail = StepDetail()
Expand Down
8 changes: 8 additions & 0 deletions src/aignt_os/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,14 @@ class AppSettings(BaseSettings):
def runtime_state_dir_resolved(self) -> Path:
return resolve_path_within_root(self.runtime_state_dir, root=self.workspace_root)

@property
def runs_db_path_resolved(self) -> Path:
return resolve_path_within_root(self.runs_db_path, root=self.workspace_root)

@property
def artifacts_dir_resolved(self) -> Path:
return resolve_path_within_root(self.artifacts_dir, root=self.workspace_root)

@property
def runtime_state_file(self) -> Path:
return self.runtime_state_dir_resolved / "runtime-state.json"
Expand Down
4 changes: 2 additions & 2 deletions src/aignt_os/runtime/worker.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,8 @@ def _record_owner_skip_if_needed(self, run_record: RunRecord, *, runtime_owner:


def build_runtime_worker(settings: AppSettings) -> RuntimeWorker:
repository = RunRepository(settings.runs_db_path)
artifact_store = ArtifactStore(settings.artifacts_dir)
repository = RunRepository(settings.runs_db_path_resolved)
artifact_store = ArtifactStore(settings.artifacts_dir_resolved)
runtime_state_store = RuntimeStateStore(settings.runtime_state_file)
runner = PersistedPipelineRunner(
repository=repository,
Expand Down
19 changes: 19 additions & 0 deletions tests/integration/test_doctor_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,3 +79,22 @@ def test_doctor_returns_environment_error_when_runs_db_parent_is_not_writable(
assert "fail" in combined_output
assert "fix" in combined_output or "permission" in combined_output or "path" in combined_output
assert "traceback" not in combined_output


def test_doctor_returns_environment_error_when_persistence_paths_escape_workspace_root(
tmp_path: Path,
cli_runner,
cli_app,
) -> None:
env = _doctor_env(tmp_path)
env["AIGNT_OS_RUNS_DB_PATH"] = str((tmp_path / ".." / "outside" / "runs.sqlite3").resolve())
env["AIGNT_OS_ARTIFACTS_DIR"] = str((tmp_path / ".." / "outside-artifacts").resolve())

result = cli_runner.invoke(cli_app, ["doctor"], env=env)

assert result.exit_code == 5
combined_output = f"{result.stdout}\n{result.stderr}".lower()
assert "runs_db" in combined_output
assert "artifacts_dir" in combined_output
assert "path escapes trusted root" in combined_output
assert "traceback" not in combined_output
Loading
Loading