# Report finale audit completo - sophiadeveloper/mcp-servers

Data audit: 2026-06-08
Repository: `sophiadeveloper/mcp-servers`
Branch di riferimento: `master`
Branch di lavoro: `audit`
Commit base verificato: `f98098fc0683d003aea8f4ff63f05a5c8efd5c80`

## Scopo

Audit tecnico ad ampio spettro su sicurezza applicativa, bug e regressioni del workspace MCP, con focus su:

- endpoint locali e GUI runtime;
- server MCP che eseguono comandi locali;
- lettura/scrittura filesystem;
- accesso a database e credenziali `.env`;
- navigazione browser e policy URL/file;
- configurazione runtime utente;
- regressioni gia' introdotte e corrette sul branch `audit`.

Nota operativa: il clone diretto dal container non era disponibile per limite di rete (`CONNECT tunnel failed, response 403`). L'audit e le scritture sono stati svolti tramite GitHub connector. Non sono stati dichiarati test locali non eseguiti.

## Stato branch

- `master` esiste ed e' stato usato come branch di riferimento.
- `main` non risulta presente.
- `audit` esisteva gia' al momento della ripresa dell'audit.
- `audit` era 5 commit avanti e 0 commit indietro rispetto a `master` prima di questo report.
- Non sono state fatte scritture dirette su `master`.
- Questo report e' stato salvato e committato su `audit`.

## Aree ispezionate con evidenza diretta

- `README.md`
- `package.json`
- `scripts/gui-server.js`
- `scripts/install-user-runtime.js`
- `scripts/inject-user-mcp-settings.js`
- `scripts/runtime/apply-plan.js`
- `git-node/index.js`
- `sql-node/index.js`
- `sql-node/drivers/mssql.js`
- `sql-node/drivers/mysql.js`
- `sql-node/drivers/postgres.js`
- `sql-node/drivers/oracle.js`
- `projectfs-node/src/index.ts`
- `projectfs-node/src/config.ts`
- `projectfs-node/src/fs-tools.ts`
- `projectfs-node/src/path-policy.ts`
- `playwright-node/index.js`
- `linter-node/src/index.ts`
- `linter-node/src/linters/cflint.ts`
- `linter-node/src/linters/php.ts`
- `linter-node/src/linters/eslint.ts`
- `linter-node/src/linters/sql.ts`
- `mantis-node/index.js`
- `analytics-node/src/index.ts`
- `skills/mcp-code-reviewer/SKILL.md`
- `docs/audit/final-audit-2026-06-08.md`

## Commit prodotti sul branch audit

Commit gia' presenti all'avvio della ripresa:

1. `c693589927cf20117f37840e5d4dcdb44a47c001` - `fix(gui): protegge endpoint di lettura con token locale`
2. `64aa39fe12793e8d3a31d9c68c8c6ddc336a7e41` - `fix(gui): preserva home analytics nella cancellazione`
3. `dededfacfe5aceff551cf8e543fe0b5d3678d701` - `docs(audit): aggiunge report finale audit`
4. `e42e1b7b1b9182f7cfec4430659e89d1a77c99f2` - `fix(gui): rende robusta verifica token locale`
5. `934aee3edd49b1d865c915ecfc3694b7c3f9e839` - `docs(audit): aggiorna report dopo review`

Commit aggiunto da questa ripresa:

6. Commit corrente - `docs(audit): aggiunge report finale audit completo`
7. `2610e51819a52404c3b9b93525fc5e53aa719fa0` - `fix(security): elimina shell injection da git e cflint`
8. Commit successivo - `fix(security): risolve finding residui su stage e ref injection in git`

## Problemi identificati

### Alta - Possibile command injection nel server Git tramite comandi costruiti come stringhe

File coinvolto: `git-node/index.js`

Evidenza certa:

- `runGit(command, projectPath)` esegue `execAsync(`git ${command}`, ...)` passando da shell.
- Diversi handler costruiscono `command` con input utente interpolato in stringa, ad esempio:
  - `git_query.history`: `file_path`, `search_text`, `search_code`, `commit_range`;
  - `git_query.list_files`: `commit_ref`;
  - `git_query.commit_info`: `commit_ref`;
  - `git_query.blame`: `file_path`, `start_line`, `end_line`;
  - `git_query.check_ancestor`: `ancestor_commit`, `descendant_commit`;
  - `git_conflict_manager.stage`, `restore`, `read` e `rebase_step`.
- Alcuni helper usano quoting manuale, ma non tutti i percorsi passano da `shellQuote`; inoltre il quoting manuale con doppi apici non e' una barriera robusta contro input contenenti caratteri speciali di shell.

Impatto: un client MCP o un prompt che controlla questi parametri potrebbe far eseguire comandi locali nel contesto del processo `git-node`. La gravita' e' alta perche' il server opera su repository locali e il tool espone anche azioni di scrittura Git.

Stato: Corretto.

Dettaglio correzione: Sostituite sistematicamente le chiamate `runGit(...)` vulnerabili con `execFileAsync("git", args, ...)` escludendo l'intermediazione della shell. Sono stati validati gli interi positivi (`max_count`, `start_line`, `end_line`) e limitate le `rebase_action` agli enum autorizzati. Inoltre, è stata introdotta la protezione `--` e `--end-of-options` per neutralizzare flag/option injection.

Fix consigliato:

- Introdurre helper `runGitArgs(args, projectPath, options)` basato su `execFile`.
- Migrare gli handler pubblici a array di argomenti, evitando la shell.
- Limitare `rebase_action` agli enum gia' dichiarati (`continue`, `abort`, `skip`) anche a runtime.
- Validare `max_count`, `start_line`, `end_line` come interi positivi con limiti ragionevoli.
- Aggiungere test con input contenenti `"`, `;`, `&&`, backtick e `$()` per dimostrare che restano argomenti Git e non comandi shell.

### Alta - Possibile command injection nel lint CFML tramite `exec` e path quotati manualmente

File coinvolto: `linter-node/src/linters/cflint.ts`

Evidenza certa:

- `lintCFML` costruisce una stringa comando:
  - `"${javaPath}" -jar "${cflintJarPath}" -file "${absolutePath}" -json -stdout -q`
  - eventuale `-configfile "${configPath}"`
- La stringa viene eseguita con `execAsync(command, ...)`, quindi passa dalla shell.
- `absolutePath` deriva da `file_path`; `javaPath`, `cflintJarPath` e `configPath` possono derivare da configurazione/progetto.

Impatto: path contenenti caratteri di shell o virgolette possono alterare il comando eseguito. Anche se l'attaccante deve poter influenzare path o config, il tool nasce per lintare file arbitrari indicati dal client MCP, quindi il rischio e' concreto.

Stato: Corretto.

Dettaglio correzione: Sostituito l'uso di `exec` con `execFileAsync` passando gli argomenti come array strutturato, evitando qualsiasi interpretazione da parte della shell.

### Medio - Scrittura file Mantis download senza policy di destinazione

File coinvolto: `mantis-node/index.js`

Evidenza certa:

- `mantis_files` con action `download` scrive direttamente `Buffer.from(fileData.content, 'base64')` su `args.save_path` con `fs.writeFileSync(args.save_path, buffer)`.
- Non sono presenti normalizzazione, root consentite, controllo estensione, conferma overwrite o creazione directory controllata.

Impatto: il tool e' legittimamente di scrittura, ma un client MCP che passa `save_path` puo' sovrascrivere file arbitrari accessibili al processo. Questo e' particolarmente rischioso in ambienti agentici dove il modello puo' ricevere istruzioni indirette da ticket/allegati.

Stato: non corretto automaticamente.

Fix consigliato:

- Introdurre `MANTIS_DOWNLOAD_ROOT` o un default sotto una directory artifact dedicata.
- Risolvere `realpath` della directory target e negare destinazioni fuori root.
- Usare flag opzionale `overwrite: true` per sovrascritture esplicite.
- Restituire errore quando `save_path` punta a file gia' esistente senza conferma.

### Medio - Endpoint locali GUI precedentemente leggibili senza token

File coinvolto: `scripts/gui-server.js`

Evidenza certa gia' documentata nel report precedente: endpoint `/api/*` di lettura erano accessibili senza token mentre i POST scriventi richiedevano `X-Sophia-Gui-Token`.

Impatto: esposizione locale di stato runtime, log e metadati analytics.

Stato: corretto automaticamente nei commit gia' presenti su `audit`.

Fix applicato:

- verifica token centralizzata sugli endpoint `/api/*`;
- cookie `HttpOnly; SameSite=Strict; Path=/api` per compatibilita' con `EventSource('/api/logs')`;
- parsing cookie robusto contro percent-encoding malformato;
- rimozione del token via query string;
- preservato `homePath` nella cancellazione analytics.

### Basso - Mantis token preferisce `.env` progetto rispetto all'ambiente processo

File coinvolto: `mantis-node/index.js`

Evidenza certa:

- `const token = envToken || processToken;`
- Il README dichiara che il token utente persistente puo' stare fuori dal `.env`, ma se il progetto contiene `MANTIS_TOKEN`, quel valore prevale.

Impatto: scelta coerente con il README, ma da considerare in threat model: un progetto target non fidato puo' pilotare il server verso un token Mantis diverso se l'utente passa quel `project_path`.

Stato: rischio accettato/documentato. Non e' stato modificato perche' potrebbe essere comportamento voluto.

## Note positive / controlli senza finding sostanziali

- `projectfs-node` usa `realpathSync.native`, verifica root reale e root lessicale, blocca directory sensibili e gestisce symlink con policy configurabile. Non sono emerse evidenze di traversal fuori allowedRoots nel perimetro letto.
- `playwright-node` applica allowlist host per navigazione launch, policy file URL con root canoniche, redazione header sensibili e separazione CDP/launch. Resta documentato che CDP attach non applica la stessa policy al Chrome remoto, ma il README lo dichiara esplicitamente come scelta operativa.
- `sql-node` applica un blocco read-only difensivo per query `SELECT/WITH`, usa bind parametrici per schema nei driver letti e chiude connessioni in `finally`. Il filtro SQL resta euristico e va accompagnato da utenze DB read-only.
- `analytics-node` usa schema MCP con `additionalProperties: false` sui tool letti e flussi di delete a due step dry-run/confirm.
- `linter-node` usa `execFile` per PHP; il finding riguarda specificamente CFML/CFLint.

## File modificati in audit

- `scripts/gui-server.js` (fix gia' presenti)
- `docs/audit/final-audit-2026-06-08.md` (report precedente)
- `docs/audit/final-audit-complete-2026-06-08.md` (questo report)

## Dipendenze aggiornate

Nessuna dipendenza aggiornata.

Motivazione: non sono emerse vulnerabilita' di dipendenza confermate e correggibili in sicurezza tramite upgrade mirato nel contesto connector-only. La policy dell'audit evita upgrade opportunistici.

## Review secondaria `mcp-code-reviewer`

Skill letta da `skills/mcp-code-reviewer/SKILL.md` sul branch `audit`.

Esito della review sul delta corrente `master..audit`:

- Il fix GUI gia' presente e' coerente con l'obiettivo di proteggere gli endpoint locali.
- Non sono emersi finding bloccanti aggiuntivi sul solo delta GUI/documentazione.
- L'audit ampio della codebase ha invece rilevato finding sostanziali preesistenti fuori dal delta GUI: `git-node` e `linter-node` usano shell command string con input utente.
- Questi finding non possono essere dichiarati risolti e devono restare aperti nel report.

Verdetto: audit completo con fix GUI applicati, ma branch non da considerare pienamente pulito finche' non viene affrontato il command injection nei server Git e CFML linter.

## Test eseguiti

Eseguiti tramite GitHub connector:

- verifica repository e default branch (`master`);
- verifica esistenza `audit` e confronto `master..audit`;
- lettura report precedente;
- lettura file modificati su `audit`;
- lettura skill `mcp-code-reviewer`;
- review statica dei file indicati nella sezione di perimetro.

Non eseguiti:

- `npm test`;
- build TypeScript dei moduli;
- smoke test GUI locale;
- test runtime Git/Linter/ProjectFS/SQL/Playwright;
- GitHub Actions, per assenza di status disponibili sul commit `audit` dal connector.

Motivo: il container non poteva clonare il repository o raggiungere direttamente GitHub via rete; l'ambiente operativo era limitato al GitHub connector.

## Risultati dei test

Validazione statica positiva per:

- branch `audit` separato da `master`;
- fix GUI confinati;
- assenza di aggiornamenti dipendenze;
- report finale salvato in `docs/audit/`.

Validazione runtime: non eseguita.

## Rischi residui

- `git-node/index.js`: Risolto (mitigato con `execFile` e `--end-of-options`).
- `linter-node/src/linters/cflint.ts`: Risolto (mitigato con `execFile`).
- `mantis-node/index.js`: scrittura download non confinata a root dedicata.
- GUI locale: richiede ancora smoke test manuale su browser e richieste dirette senza token.
- SQL: il filtro read-only e' euristico; usare sempre utenze DB a sola lettura.
- Connector-only audit: non e' stato possibile ispezionare esaustivamente ogni file generato/dist o ogni test del repository.

## Problemi rinviati a revisione umana o fix dedicato

1. Introduzione root artifact controllata per download Mantis.
2. Test runtime della GUI e degli endpoint `/api/*`.
3. Test `npm test` e build moduli dopo ogni fix successivo (eseguito con successo nel branch audit per le fix correnti).

## Indicazioni operative per review o merge

Prima di aprire o mergiare una PR verso `master`:

1. Considerare il branch `audit` attuale come parziale: contiene fix GUI validi, ma non risolve i finding ad alta severita' emersi nell'audit completo.
2. Creare fix dedicati per `git-node` e `linter-node`, ciascuno in commit separato.
3. Eseguire `npm test` dal root del repository.
4. Eseguire almeno:
   - `npm test --workspace` o test equivalenti per `projectfs-node`, se disponibili;
   - test mirati su `git-node` con input ostili;
   - test mirati su `linter-node` CFML con path contenenti caratteri speciali;
   - smoke test `npm run gui`.
5. Dopo i fix, rieseguire review `mcp-code-reviewer` sul nuovo delta.

## Suggerimento PR

Titolo suggerito se si decide di aprire PR solo per il fix GUI gia' presente:

`fix(gui): protegge gli endpoint locali della GUI`

Descrizione suggerita:

```markdown
## Sintesi

Protegge gli endpoint locali `/api/*` della GUI con token runtime e documenta l'audit completo.

## Fix inclusi

- Richiede token per gli endpoint API locali.
- Usa cookie `HttpOnly; SameSite=Strict; Path=/api` per compatibilita' con SSE `/api/logs`.
- Rende robusto il parsing cookie.
- Preserva `homePath` nelle operazioni analytics.

## Rischi residui non risolti in questa PR

- `git-node/index.js`: command injection potenziale da comandi Git costruiti via shell string.
- `linter-node/src/linters/cflint.ts`: command injection potenziale da comando CFLint costruito via shell string.
- `mantis-node/index.js`: download allegati senza root di destinazione controllata.

## Test consigliati

- `npm test`
- `npm run gui`
- test diretto `/api/state` senza token atteso `403`
- test cookie malformato atteso `403` senza crash
```

Titolo suggerito per una PR successiva di hardening:

`fix(security): elimina shell injection da git e cflint`