fix(security): corrige 3 vulnerabilidades críticas — broken auth, credenciais expostas e senhas em texto puro - #3
fix(security): corrige 3 vulnerabilidades críticas — broken auth, credenciais expostas e senhas em texto puro#3viniciolimadev wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
Este PR visa corrigir vulnerabilidades críticas de segurança adicionando autenticação para rotas administrativas, removendo credenciais hardcoded via .env, e garantindo que senhas não sejam expostas/armazenadas em texto puro.
Changes:
- Implementa fluxo de login/logout e um guard de autenticação no front controller (
public/index.php). - Move credenciais para variáveis de ambiente e refatora a conexão PDO, adicionando loader simples de
.env. - Atualiza seeds do banco para bcrypt e ajusta listagem de usuários para não expor senha e escapar HTML.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
public/index.php |
Adiciona session/auth guard, login/logout e rota para view de login |
src/bootstrap.php |
Implementa loader de .env para popular getenv()/$_ENV |
src/conexao.php |
Refatora PDO para usar variáveis de ambiente e opções mais seguras |
src/views/login.phtml |
Cria formulário de login e renderização de erro com escaping |
src/views/usuarios.phtml |
Remove SELECT *, evita coluna senha e aplica htmlspecialchars() |
src/views/_components/menu.phtml |
Ajusta link “Sair” para /logout |
docker-compose.yml |
Passa a usar env_file e remove exposição da porta 3306 do MySQL |
db.sql |
Torna colunas mais restritivas e troca seeds por hashes bcrypt |
.gitignore |
Ignora .env |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| view($pagina); | ||
|
|
||
| function view(string $nome): void | ||
| { | ||
| include "../src/views/_layout/head.phtml"; | ||
|
|
||
| include "../src/views/_components/menu.phtml"; | ||
| include '../src/views/_layout/head.phtml'; | ||
| include '../src/views/_components/menu.phtml'; | ||
| include "../src/views/{$nome}.phtml"; | ||
|
|
||
| include "../src/views/_layout/footer.phtml"; | ||
| include '../src/views/_layout/footer.phtml'; |
There was a problem hiding this comment.
view() inclui os templates dentro do escopo de uma função; variáveis definidas fora (ex.: $pdo do conexao.php e $erroLogin do fluxo de login) não ficam disponíveis dentro das views, o que vai causar Undefined variable e quebrar /admin/usuarios e a mensagem de erro do login. Uma forma simples de corrigir é passar dados para a view (ex.: view($pagina, ['pdo' => $pdo, 'erroLogin' => $erroLogin ?? null]) + extract()), ou então mover as queries/variáveis para dentro do view() (com global $pdo) — preferencialmente mantendo acesso a dados fora do template.
| <?php | ||
|
|
||
| include dirname(__DIR__)."/conexao.php"; | ||
|
|
||
| $resultado = $pdo->query("SELECT * FROM tb_users"); | ||
| $stmt = $pdo->query("SELECT id, nome, email FROM tb_users"); | ||
| $usuarios = $stmt->fetchAll(); |
There was a problem hiding this comment.
Este template usa $pdo, mas quando é incluído via view() (em public/index.php) ele roda no escopo da função e não tem acesso ao $pdo definido no escopo global. Isso vai gerar erro/notice e impedir o carregamento da listagem. Passe o PDO para a view (via extract()/parâmetros) ou exponha explicitamente o $pdo no escopo do include (ex.: global $pdo no view()).
| <?php if (!empty($erroLogin)): ?> | ||
| <div class="alert alert-danger"><?= htmlspecialchars($erroLogin, ENT_QUOTES, 'UTF-8') ?></div> | ||
| <?php endif; ?> |
There was a problem hiding this comment.
$erroLogin é definido no fluxo de login em public/index.php, mas este template é incluído dentro da função view() e não enxerga variáveis do escopo global. Isso faz com que a mensagem de erro não apareça e pode gerar Undefined variable dependendo da configuração. Sugestão: passar erroLogin como dado da view (ex.: view('login', ['erroLogin' => $erroLogin ?? null])).
| $rotasPublicas = ['/', '/login']; | ||
| $logado = !empty($_SESSION['usuario_id']); | ||
|
|
||
| if (!$logado && !in_array($url, $rotasPublicas, true)) { | ||
| header('Location: /login'); | ||
| exit; | ||
| } |
There was a problem hiding this comment.
Atualmente o guard redireciona para /login qualquer rota que não seja '/' ou '/login'. Isso é mais amplo do que o descrito no PR (que cita apenas /admin e /admin/usuarios) e também impede a renderização do 404 para usuários não autenticados (qualquer URL desconhecida vira redirect para login). Considere restringir o guard apenas para rotas protegidas (ex.: prefixo /admin) ou listar explicitamente as rotas que exigem sessão.
| if ($url === '/logout') { | ||
| session_destroy(); | ||
| header('Location: /login'); | ||
| exit; |
There was a problem hiding this comment.
No logout, session_destroy() não limpa $_SESSION nem remove o cookie da sessão; o browser pode continuar enviando o mesmo session id (mesmo que vazio) no request seguinte. Para um logout mais robusto, limpe $_SESSION, expire o cookie de sessão (setcookie(session_name(), '', time()-...)) e/ou regenere o id antes do redirect.
| $pdo = new PDO( | ||
| sprintf('mysql:host=%s;dbname=%s;charset=utf8mb4', getenv('DB_HOST'), getenv('DB_NAME')), | ||
| getenv('DB_USER'), | ||
| getenv('DB_PASSWORD'), | ||
| [ |
There was a problem hiding this comment.
getenv('DB_HOST'), DB_NAME, DB_USER e DB_PASSWORD podem retornar false quando não configurados, gerando um DSN/credenciais inválidos e um erro de PDO pouco claro. Sugestão: validar essas variáveis antes de instanciar o PDO e lançar uma exceção com mensagem objetiva indicando quais variáveis estão faltando.
| <a class="btn btn-outline-dark" href="/admin">Admin</a> | ||
| <a class="btn btn-outline-dark" href="/admin/usuarios">Usuarios</a> | ||
| <a class="btn btn-outline-danger" href="/login">Sair</a> | ||
| <a class="btn btn-outline-danger" href="/logout">Sair</a> |
There was a problem hiding this comment.
No menu, o link está rotulado como "Usuarios" (sem acento), enquanto a view usa "Usuários". Para consistência de UI/ortografia, alinhe o texto do menu para "Usuários".
| session_start(); | ||
|
|
||
| require_once '../src/bootstrap.php'; |
There was a problem hiding this comment.
Como o app agora depende de sessão para autenticação, vale configurar parâmetros do cookie de sessão antes de session_start() (HttpOnly, Secure quando em HTTPS, e SameSite). Sem isso, o cookie pode ficar com defaults menos seguros do que o esperado para um fluxo de login/admin.
| // --- Logout --- | ||
|
|
||
| if ($url === '/logout') { | ||
| session_destroy(); | ||
| header('Location: /login'); | ||
| exit; | ||
| } |
There was a problem hiding this comment.
/logout faz alteração de estado via GET, o que permite logout por CSRF (ex.: um terceiro site pode forçar o browser a requisitar essa URL). Para reduzir esse vetor, considere mudar para POST /logout e validar um token CSRF (ou ao menos exigir método POST).
| $key = trim($key); | ||
| $value = trim($value); | ||
| $_ENV[$key] = $value; | ||
| putenv("$key=$value"); |
There was a problem hiding this comment.
O loader sempre faz putenv()/$_ENV[...] = ..., sobrescrevendo variáveis já definidas no ambiente (Compose/K8s/CI). Em geral é mais seguro não sobrescrever valores existentes (deixar o ambiente ter precedência) e só setar se ainda não estiver definido.
Resumo
Este PR corrige três vulnerabilidades críticas identificadas em análise estática de segurança.
#1 — Broken Access Control (OWASP A01:2021 / CWE-306)
public/index.php: rotas/admine/admin/usuariosagora exigem sessãoativa
POST /login) e logout (GET /logout)password_verify()+session_regenerate_id()para prevenir session fixationsrc/views/login.phtmlcom formulário Bootstrap/login→/logout#2 — Credenciais Hardcoded no Código-fonte (CWE-798)
.envpara armazenar todas as credenciais fora do repositório.envadicionado ao.gitignoresrc/bootstrap.phpcom loader de.envsem dependências externassrc/conexao.phprefatorado para usargetenv()com opções PDO seguras (ERRMODE_EXCEPTION,EMULATE_PREPARES=false)docker-compose.ymlatualizado comenv_filenos serviços PHP e MySQL; credenciais removidas do YAML#3 — Senhas em Texto Puro + Porta MySQL Exposta (CWE-256 / OWASP A02:2021)
db.sql: seeds atualizados com hashes bcrypt reais (password_hash, cost=12)docker-compose.yml: porta3306removida do host (MySQL acessível apenas pela rede interna Docker)src/views/usuarios.phtml:SELECT *substituído porSELECT id, nome, email— colunasenhanunca é expostana UI
htmlspecialchars()em todos os campos renderizados (previne XSS armazenado)Checklist de testes
/admin/usuariossem login → deve redirecionar para/login/admin/logindocker psnão deve listar binding)senhaausente na listagem de usuários