improvement#1
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR centralizes application bootstrap and layout concerns while hardening several PHP endpoints against common web risks (SQL injection, XSS, CSRF) and improving query efficiency for public search and CRUD pages.
Changes:
- Introduces a single
includes/bootstrap.phpentrypoint (config + session + DB + auth + rendering/layout helpers) and updates pages to use it early to avoid latesession_start()/ header issues. - Reworks public search flow to use prepared statements and session-stored query parameters instead of concatenated SQL / HTML-in-session, with a shared row-render helper.
- Adds CSRF protection, output escaping helpers, and tighter auth/role guards across login/CRUD flows; optimizes some count queries.
Reviewed changes
Copilot reviewed 25 out of 28 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| validation.php | Tightens validation helpers and adds validateId() for positive integer identifiers. |
| public/main/results.php | Replaces SQL string concatenation with placeholder-based filtering and stores query params in session. |
| public/main/index.php | Re-executes parameterized queries and renders rows via helper; improves escaping in filter options and counters. |
| public/main/header.php | Adds image loading/decoding hints. |
| public/login/registro.php | Uses centralized layout/bootstrap, escapes messages, adds CSRF field, enforces minlength. |
| public/login/procesar_registro.php | Adds POST-only guard, CSRF verification, input validation, prepared statements, and safer error handling. |
| public/login/procesar_login.php | Adds POST-only guard, CSRF verification, prepared statement, session fixation mitigation, safer redirects/errors. |
| public/login/logout.php | Performs more complete session teardown (cookie + data) and uses relative redirect. |
| public/login/login.php | Uses centralized layout/bootstrap, escapes flash messages, adds CSRF field. |
| public/crud/registros_autoria.php | Adds role guard and safer error handling; selects explicit columns and escapes output. |
| public/crud/procesar_edit.php | Adds login+CSRF enforcement, session ID validation, prepared statements, safer redirects and error handling. |
| public/crud/procesar_add_directivo.php | Adds login+CSRF enforcement, prepared statements, RETURNING usage, safer error handling. |
| public/crud/modificaciones.php | Moves bootstrap/session earlier and escapes output. |
| public/crud/edit.php | Separates view vs update handler, validates cod_dir, escapes outputs, adds CSRF field. |
| public/crud/directivos.php | Moves DB logic before HTML, uses prepared statements for paging/search, escapes output consistently. |
| public/crud/delete.php | Converts destructive action to POST+CSRF, validates IDs, parameterizes DELETE, improves UX flow. |
| public/crud/add_directivo.php | Loads catalogs once, adds CSRF field, uses bootstrap/layout, removes duplicate DB includes. |
| includes/render.php | Adds e() escape helper and render_fila_institucion() row renderer. |
| includes/layout.php | Adds render_head() / render_footer() for unified page skeleton and assets. |
| includes/config.php | Adds .env parsing and centralized environment/DB configuration. |
| includes/bootstrap.php | Adds unified bootstrap entrypoint for session + includes. |
| includes/auth.php | Adds login/role guards and CSRF token generation/verification helpers. |
| connect.php | Moves DB settings to config/env, adds safer connection error behavior, refactors credential-based connect helper. |
| atributtes.php | Collapses multiple COUNT queries into fewer aggregated queries; expects $conn to exist. |
| .gitignore | Ignores .env, vendor/, and log files. |
| .env.example | Provides example environment variables for configuration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+114
to
+115
| if (valor !== '' && !/^[a-zA-ZáéíóúÁÉÍÓÚñÑ\s]+$/.test(valor)) { | ||
| if (msg) { msg.textContent = 'Ingresa solo letras.'; msg.classList.remove('hidden'); } |
Comment on lines
+19
to
+20
| if (!validateText($nombre) || !validateText($apellido) | ||
| || !validateNumber($cargo) || !validateNumber($institucion) || !validateNumber($cod_nombram)) { |
| } | ||
|
|
||
| // [SEG] Validar entrada del formulario. | ||
| if (!validateText($nombre) || !validateText($apellido) || !validateNumber($cargo)) { |
Comment on lines
+81
to
+84
| define('DB_DEFAULT_USER', env_value('DB_DEFAULT_USER', 'api')); | ||
| define('DB_DEFAULT_PASS', env_value('DB_DEFAULT_PASS', 'apipassword')); | ||
| define('DB_ADMIN_USER', env_value('DB_ADMIN_USER', 'postgres')); | ||
| define('DB_ADMIN_PASS', env_value('DB_ADMIN_PASS', 'postgres')); |
Comment on lines
+56
to
+57
| $dbpassword = $hashed_password_db; | ||
| return connect($username, $dbpassword); |
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
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.
No description provided.