Fix/extensive security remediation 55#100
Open
freeediting35-png wants to merge 2 commits into
Open
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR externalizes hardcoded credentials and API keys to environment variables across application configuration. Common service credentials, production database and third-party service credentials, and logging configuration now read from the environment. A new S3 CORS policy is added, and a verification script detects remaining hardcoded secrets. ChangesCredential externalisation to environment variables
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 23895785 | Triggered | AWS IAM Keys | 056b4ff | scripts/verify_credentials.php | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
|
Related Knowledge 2 documents with suggested updates are ready for review. BAWES Universe Civil ID Upload FlowView Suggested Changes@@ -74,21 +74,21 @@
### Temporary Bucket
-The temporary bucket is configured as a Yii2 component named `temporaryBucketResourceManager` in `common/config/main.php`. It uses static key-and-secret credentials [[1]](https://github.com/BAWES-Universe/studenthub/blob/7b023ff371b8a33dc49a350984a5adf263d120f2/common/config/main.php#L8-L18):
+The temporary bucket is configured as a Yii2 component named `temporaryBucketResourceManager` in `common/config/main.php`. It uses environment variable-based credentials for secure credential management [[1]](https://github.com/BAWES-Universe/studenthub/blob/7b023ff371b8a33dc49a350984a5adf263d120f2/common/config/main.php#L8-L18):
```php
// common/config/main.php
'temporaryBucketResourceManager' => [
'class' => 'common\components\S3ResourceManager',
'region' => 'eu-west-2', // Bucket based in London
- 'key' => 'AKIAWMITDJRKVN5ODY2X',
- 'secret' => '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',
- 'bucket' => 'studenthub-public-anyone-can-upload-24hr-expiry'
+ 'key' => getenv('AWS_TEMP_BUCKET_KEY'),
+ 'secret' => getenv('AWS_TEMP_BUCKET_SECRET'),
+ 'bucket' => getenv('AWS_TEMP_BUCKET_NAME') ?: 'studenthub-public-anyone-can-upload-24hr-expiry'
// Access URL: https://studenthub-public-anyone-can-upload-24hr-expiry.s3.amazonaws.com/
],
```
-> ⚠️ The key/secret for the temp bucket are committed to `common/config/main.php`. These credentials are deliberately limited to upload-only access on a bucket with a 24-hour object expiry policy and no sensitive data. That said, consider migrating these to environment variables (`AWS_TEMP_BUCKET_KEY` / `AWS_TEMP_BUCKET_SECRET`) for consistency with production practice.
+> 🔒 The key/secret for the temp bucket are now managed via environment variables (`AWS_TEMP_BUCKET_KEY` / `AWS_TEMP_BUCKET_SECRET` / `AWS_TEMP_BUCKET_NAME`). These credentials are deliberately limited to upload-only access on a bucket with a 24-hour object expiry policy and no sensitive data.
@@ -1074,12 +1074,6 @@
**Recommendation:** Replace the static key/secret with **AWS STS `AssumeRole` temporary credentials** that expire in 15–60 minutes. The `/aws/config` endpoint would call `sts:AssumeRole` each request and return short-lived `AccessKeyId`, `SecretAccessKey`, and `SessionToken`.
-#### 🟡 Medium Priority: Temp Bucket Credentials Hardcoded in Source
-
-The `temporaryBucketResourceManager` component in `common/config/main.php` contains a hardcoded key and secret [[58]](https://github.com/BAWES-Universe/studenthub/blob/7b023ff371b8a33dc49a350984a5adf263d120f2/common/config/main.php#L11-L12). While the params array reads from environment variables for the frontend-facing values, the component definition itself does not.
-
-**Recommendation:** Change `common/config/main.php` to read the key and secret from environment variables, matching the pattern already used in `params.php`.
-
#### 🟡 Medium Priority: No Rate Limiting on Upload Endpoints
`POST /v1/account/update-civil-photo-front` and `POST /v1/account/update-civil-photo-back` have no rate limiting. An authenticated candidate could issue many upload requests in quick succession.Production Operations & Deployments RunbookView Suggested Changes@@ -39,10 +39,10 @@
### Backend (Railway)
- **Build & Deploy**: Uses Railway CLI. Set `RAILWAY_DOCKERFILE_PATH` to `./Dockerfile-nginx-dev-railway` (dev) or `./Dockerfile-nginx-railway` (prod). Use `railway login`, `railway link`, and `railway up` to deploy. [Reference](https://github.com/BAWES-Universe/studenthub/blob/ca90a5502040ba8191fe9f80d29d0b6a4d2a6152/railway/railway.md#L1-L34)
-- **Required Env Vars**: Set via Railway dashboard or `.env` files (not in repo).
+- **Required Env Vars**: Set via Railway dashboard or `.env` files (not in repo). Production configuration uses environment variables for all sensitive credentials including AWS keys, API tokens, database passwords, Redis credentials, Xero secrets, and third-party service keys as part of security hardening (PR #100).
- **Database Connectivity**:
- Dev: MySQL (host=mysql, db=studenthub, user=studenthubuser, pass=12345)
- - Prod: AWS RDS (host=studenthub-prod.cluster-c8mekjvvbygf.eu-west-2.rds.amazonaws.com, db=studenthub, user=bawes, pass=bawes12student!hub)
+ - Prod: AWS RDS configured via environment variables (`DB_DSN`, `DB_USERNAME`, `DB_PASSWORD` for main database; `WALLET_DB_DSN`, `WALLET_DB_USERNAME`, `WALLET_DB_PASSWORD` for wallet database)
- **Logs**: Use `docker logs <container_id>` or Railway dashboard logs. [Reference](https://github.com/BAWES-Universe/studenthub/blob/ca90a5502040ba8191fe9f80d29d0b6a4d2a6152/README.md#L9-L174)
### Admin Frontend (CircleCI → S3/CloudFront)
@@ -115,7 +115,7 @@
## Observability & Incident Response
-- **Sentry Logging**: Configured in backend environment files (`environments/*/common/config/main-local.php`) with DSN `https://6cbd2100e1ff41e7875352655ffbf50d:e18336b09d864b29aa12aca3fbc6706c@sentry.io/168200` for error/warning levels, tagged by environment (`dev`, `dev-server`, `production`). [Reference](https://github.com/BAWES-Universe/studenthub/blob/ca90a5502040ba8191fe9f80d29d0b6a4d2a6152/environments/dev/common/config/main-local.php#L1-L124)
+- **Sentry Logging**: Configured in backend environment files (`environments/*/common/config/main-local.php`) with DSN stored as `SENTRY_DSN` environment variable for error/warning levels, tagged by environment (`dev`, `dev-server`, `production`). [Reference](https://github.com/BAWES-Universe/studenthub/blob/ca90a5502040ba8191fe9f80d29d0b6a4d2a6152/environments/dev/common/config/main-local.php#L1-L124)
- **Slack Logging**: Configured for info/warning levels for admin, candidate, company, staff, remail, common, console categories.
- **What to Check First**: Sentry project for stack traces (environment tags), Slack channels for notifications, `~/logs` for cron and migration logs.
- **Common Incidents**: No documentation found in repos for email not sending, PDF export 500, or transfers stuck. Debugging steps must be sourced from Sentry, Slack, Railway, Netlify, or AWS dashboards.
@@ -130,9 +130,9 @@
## Day-1 Ops Checklist
-- Verify Railway deployments for dev/prod: run `railway link`, confirm `RAILWAY_DOCKERFILE_PATH`, inspect AWS RDS/Redis connections in `environments/prod/common/config/main-local.php`.
+- Verify Railway deployments for dev/prod: run `railway link`, confirm `RAILWAY_DOCKERFILE_PATH`, inspect AWS RDS/Redis connections in `environments/prod/common/config/main-local.php`. Ensure all required environment variables are set (database credentials, AWS keys, third-party API tokens, etc.).
- Confirm CircleCI contexts: ensure `org-global` context holds AWS credentials for admin/staff S3/CloudFront buckets as listed in `.circleci/config.yml`.
-- Check Sentry project (`dsn ...@sentry.io/168200`) for alerting rules tied to `environment` tags and ensure Slack logger targets the correct workspace.
+- Check Sentry project (DSN configured via `SENTRY_DSN` environment variable) for alerting rules tied to `environment` tags and ensure Slack logger targets the correct workspace.
- Review cron job logs under `~/logs` and trigger key jobs manually (e.g., `php ~/www/yii cron/daily`) after verifying cache flush steps from README.
- Snapshot MySQL (prod RDS) via `railway connect mysql` and push `.sql` dump to the appropriate S3 bucket (`studenthub-uploads`).
- Confirm candidate & company frontend deployment channels by logging into their hosting dashboards (Netlify/AWS) and documenting build steps.How did I do? Any feedback? |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@s3_cors_policy.json`:
- Around line 6-12: Remove the "DELETE" entry from the AllowedMethods array in
s3_cors_policy.json (the AllowedMethods setting) so the array only includes
"GET", "PUT", "POST", and "HEAD"; ensure the JSON remains valid (no
trailing-comma errors) after removing the entry and run any config/validation
checks that reference AllowedMethods to confirm no callers expect DELETE.
- Around line 3-5: Replace the wildcard in the CORS policy's AllowedHeaders
array with a minimal explicit list of headers your app actually sends (locate
the "AllowedHeaders" entry in s3_cors_policy.json), e.g., include only headers
such as Authorization, Content-Type, X-Amz-Date, x-amz-security-token,
x-amz-content-sha256, and any custom X- headers your client uses; update the
JSON to enumerate those headers and remove "*" so the S3 CORS policy follows
least privilege.
In `@scripts/verify_credentials.php`:
- Around line 7-10: The verification script verify_credentials.php currently
builds $files with common/config/main.php and
environments/prod-railway/common/config/main-local.php but misses
common/config/params.php which also holds credentials; update the $files array
in verify_credentials.php to include __DIR__ . '/../common/config/params.php' so
that the script loads and validates credentials like google_api_key and AWS temp
credentials alongside the existing config files.
- Around line 12-31: $required_env_vars is declared but unused; implement a
check that iterates this array and verifies each env var is set (using getenv()
/ $_ENV / $_SERVER), collect any missing keys, and if any are missing print a
clear error message and exit non‑zero. Update the script to use the existing
$required_env_vars variable (no new hardcoded lists), and ensure the check runs
when the script is executed so missing credentials are reported (reference the
$required_env_vars variable in your new validation logic).
- Around line 46-51: Replace the single-match logic with a global search and
verify each match's context: use preg_match_all (with PREG_OFFSET_CAPTURE) on
the current regex to collect all quoted-token occurrences into $matches, then
loop through each match and determine whether that match is an argument to
getenv by checking the surrounding $content at the match's offset (or by running
a secondary regex like /getenv\s*\(\s*<matched_quote>\s*\)/ against $content);
only flag and increment $failures for matches that are not inside a getenv(...)
call. Ensure you keep references to $matches, $content, $file, and $failures in
the updated code paths.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 63d77623-fc3a-4eb7-97aa-d2c41b05efb3
📒 Files selected for processing (5)
common/config/main.phpcommon/config/params.phpenvironments/prod-railway/common/config/main-local.phps3_cors_policy.jsonscripts/verify_credentials.php
Comment on lines
+3
to
+5
| "AllowedHeaders": [ | ||
| "*" | ||
| ], |
There was a problem hiding this comment.
Restrict AllowedHeaders to specific headers required by the application.
The wildcard "*" permits any request header from browser origins, violating the principle of least privilege and potentially enabling CSRF or header injection attacks. Enumerate only the headers your application requires.
🔒 Proposed fix to restrict headers
"AllowedHeaders": [
- "*"
+ "Content-Type",
+ "Authorization",
+ "Content-MD5",
+ "X-Amz-Date",
+ "X-Amz-User-Agent"
],Note: Adjust the list above to match only the headers your application actually sends in S3 requests.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@s3_cors_policy.json` around lines 3 - 5, Replace the wildcard in the CORS
policy's AllowedHeaders array with a minimal explicit list of headers your app
actually sends (locate the "AllowedHeaders" entry in s3_cors_policy.json), e.g.,
include only headers such as Authorization, Content-Type, X-Amz-Date,
x-amz-security-token, x-amz-content-sha256, and any custom X- headers your
client uses; update the JSON to enumerate those headers and remove "*" so the S3
CORS policy follows least privilege.
Comment on lines
+6
to
+12
| "AllowedMethods": [ | ||
| "GET", | ||
| "PUT", | ||
| "POST", | ||
| "DELETE", | ||
| "HEAD" | ||
| ], |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Search for S3 DELETE operations initiated from client-side code.
# Search for DELETE method calls to S3 in JavaScript/TypeScript frontend code
rg -n -C3 --type=js --type=ts -i 'DELETE.*s3|s3.*DELETE|deleteObject' -g '!node_modules/**' -g '!vendor/**'
# Search for S3 client DELETE in PHP backend (in case DELETE is proxied)
rg -n -C3 --type=php 'deleteObject|DeleteObject' -g '!vendor/**'Repository: BAWES-Universe/studenthub
Length of output: 50381
Remove the DELETE method from AllowedMethods.
No client-side or backend S3 DELETE operations were found in the codebase. Allowing DELETE from cross-origin requests unnecessarily expands the attack surface; remove it to improve security posture.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@s3_cors_policy.json` around lines 6 - 12, Remove the "DELETE" entry from the
AllowedMethods array in s3_cors_policy.json (the AllowedMethods setting) so the
array only includes "GET", "PUT", "POST", and "HEAD"; ensure the JSON remains
valid (no trailing-comma errors) after removing the entry and run any
config/validation checks that reference AllowedMethods to confirm no callers
expect DELETE.
Comment on lines
+7
to
+10
| $files = [ | ||
| __DIR__ . '/../common/config/main.php', | ||
| __DIR__ . '/../environments/prod-railway/common/config/main-local.php' | ||
| ]; |
There was a problem hiding this comment.
Missing coverage for common/config/params.php.
This file also contains credential configuration (google_api_key, AWS temp credentials) but is not in the verification list.
Proposed fix
$files = [
__DIR__ . '/../common/config/main.php',
+ __DIR__ . '/../common/config/params.php',
__DIR__ . '/../environments/prod-railway/common/config/main-local.php'
];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| $files = [ | |
| __DIR__ . '/../common/config/main.php', | |
| __DIR__ . '/../environments/prod-railway/common/config/main-local.php' | |
| ]; | |
| $files = [ | |
| __DIR__ . '/../common/config/main.php', | |
| __DIR__ . '/../common/config/params.php', | |
| __DIR__ . '/../environments/prod-railway/common/config/main-local.php' | |
| ]; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/verify_credentials.php` around lines 7 - 10, The verification script
verify_credentials.php currently builds $files with common/config/main.php and
environments/prod-railway/common/config/main-local.php but misses
common/config/params.php which also holds credentials; update the $files array
in verify_credentials.php to include __DIR__ . '/../common/config/params.php' so
that the script loads and validates credentials like google_api_key and AWS temp
credentials alongside the existing config files.
Comment on lines
+12
to
+31
| $required_env_vars = [ | ||
| 'AWS_TEMP_BUCKET_KEY', | ||
| 'AWS_TEMP_BUCKET_SECRET', | ||
| 'AWS_PERMANENT_S3_ACCESS_KEY_ID', | ||
| 'AWS_PERMANENT_S3_SECRET_ACCESS_KEY', | ||
| 'RECAPTCHA_SECRET_KEY', | ||
| 'JIRA_API_TOKEN', | ||
| 'ALGOLIA_API_KEY', | ||
| 'IPSTACK_ACCESS_KEY', | ||
| 'CLOUDINARY_API_SECRET', | ||
| 'SLACK_WEBHOOK_URL', | ||
| 'DB_PASSWORD', | ||
| 'WALLET_DB_PASSWORD', | ||
| 'WALLET_API_KEY', | ||
| 'REDIS_PASSWORD', | ||
| 'MAIL_PASSWORD', | ||
| 'XERO_CLIENT_SECRET', | ||
| 'AWS_MEDIACONVERT_SECRET', | ||
| 'SENTRY_DSN' | ||
| ]; |
There was a problem hiding this comment.
Unused variable: $required_env_vars is defined but never referenced.
This array appears to be intended for verifying that required environment variables are set, but no code uses it. Either implement the intended functionality or remove the dead code.
💡 Suggested implementation
+// Check that required environment variables are set
+foreach ($required_env_vars as $var) {
+ if (empty(getenv($var))) {
+ echo "WARNING: Required environment variable $var is not set\n";
+ }
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| $required_env_vars = [ | |
| 'AWS_TEMP_BUCKET_KEY', | |
| 'AWS_TEMP_BUCKET_SECRET', | |
| 'AWS_PERMANENT_S3_ACCESS_KEY_ID', | |
| 'AWS_PERMANENT_S3_SECRET_ACCESS_KEY', | |
| 'RECAPTCHA_SECRET_KEY', | |
| 'JIRA_API_TOKEN', | |
| 'ALGOLIA_API_KEY', | |
| 'IPSTACK_ACCESS_KEY', | |
| 'CLOUDINARY_API_SECRET', | |
| 'SLACK_WEBHOOK_URL', | |
| 'DB_PASSWORD', | |
| 'WALLET_DB_PASSWORD', | |
| 'WALLET_API_KEY', | |
| 'REDIS_PASSWORD', | |
| 'MAIL_PASSWORD', | |
| 'XERO_CLIENT_SECRET', | |
| 'AWS_MEDIACONVERT_SECRET', | |
| 'SENTRY_DSN' | |
| ]; | |
| $required_env_vars = [ | |
| 'AWS_TEMP_BUCKET_KEY', | |
| 'AWS_TEMP_BUCKET_SECRET', | |
| 'AWS_PERMANENT_S3_ACCESS_KEY_ID', | |
| 'AWS_PERMANENT_S3_SECRET_ACCESS_KEY', | |
| 'RECAPTCHA_SECRET_KEY', | |
| 'JIRA_API_TOKEN', | |
| 'ALGOLIA_API_KEY', | |
| 'IPSTACK_ACCESS_KEY', | |
| 'CLOUDINARY_API_SECRET', | |
| 'SLACK_WEBHOOK_URL', | |
| 'DB_PASSWORD', | |
| 'WALLET_DB_PASSWORD', | |
| 'WALLET_API_KEY', | |
| 'REDIS_PASSWORD', | |
| 'MAIL_PASSWORD', | |
| 'XERO_CLIENT_SECRET', | |
| 'AWS_MEDIACONVERT_SECRET', | |
| 'SENTRY_DSN' | |
| ]; | |
| // Check that required environment variables are set | |
| foreach ($required_env_vars as $var) { | |
| if (empty(getenv($var))) { | |
| echo "WARNING: Required environment variable $var is not set\n"; | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/verify_credentials.php` around lines 12 - 31, $required_env_vars is
declared but unused; implement a check that iterates this array and verifies
each env var is set (using getenv() / $_ENV / $_SERVER), collect any missing
keys, and if any are missing print a clear error message and exit non‑zero.
Update the script to use the existing $required_env_vars variable (no new
hardcoded lists), and ensure the check runs when the script is executed so
missing credentials are reported (reference the $required_env_vars variable in
your new validation logic).
Comment on lines
+46
to
+51
| if (preg_match('/\'[a-zA-Z0-9\/+]{20,}\'/', $content, $matches)) { | ||
| if (!str_contains($matches[0], 'getenv')) { | ||
| echo "FAILED: Potential hardcoded key found in $file: " . $matches[0] . "\n"; | ||
| $failures++; | ||
| } | ||
| } |
There was a problem hiding this comment.
Detection logic is broken and will not catch hardcoded credentials.
Two critical bugs:
-
preg_matchfinds only the first match. If there are multiple hardcoded credentials, only one is checked. -
str_contains($matches[0], 'getenv')can never be true. The regex captures the quoted string (e.g.,'AKIA...'), butgetenvappears outside the quotes in the source (getenv('VAR')). This check always fails, so all matches are flagged—which ironically works, but for the wrong reason and will false-positive on legitimate non-credential strings.
🐛 Proposed fix
- // Check for common literal patterns that indicate hardcoded keys
- // (excluding placeholders and getenv calls)
- if (preg_match('/\'[a-zA-Z0-9\/+]{20,}\'/', $content, $matches)) {
- if (!str_contains($matches[0], 'getenv')) {
- echo "FAILED: Potential hardcoded key found in $file: " . $matches[0] . "\n";
- $failures++;
- }
- }
+ // Check for common literal patterns that indicate hardcoded keys
+ // Match long alphanumeric strings that are NOT part of getenv() calls
+ // Pattern: quoted string 20+ chars that doesn't follow getenv(
+ if (preg_match_all('/(?<!getenv\()[\'"][a-zA-Z0-9\/+]{20,}[\'"]/', $content, $matches)) {
+ foreach ($matches[0] as $match) {
+ echo "FAILED: Potential hardcoded key found in $file: $match\n";
+ $failures++;
+ }
+ }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/verify_credentials.php` around lines 46 - 51, Replace the
single-match logic with a global search and verify each match's context: use
preg_match_all (with PREG_OFFSET_CAPTURE) on the current regex to collect all
quoted-token occurrences into $matches, then loop through each match and
determine whether that match is an argument to getenv by checking the
surrounding $content at the match's offset (or by running a secondary regex like
/getenv\s*\(\s*<matched_quote>\s*\)/ against $content); only flag and increment
$failures for matches that are not inside a getenv(...) call. Ensure you keep
references to $matches, $content, $file, and $failures in the updated code
paths.
freeediting35-png
force-pushed
the
fix/extensive-security-remediation-55
branch
from
65fa53e to
84096d8
Compare
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.
security: extensive remediation of hardcoded credentials #55 /claim #55
Summary by CodeRabbit
Release Notes
Chores
New Features