chore: moving staging out of render to GCP via kamal #789
Conversation
For beta_submission_finished, internal_submission_finished and submission_failed.
Separate "phase" and "kind" for simplicity and better data integrity
WalkthroughThis set of changes introduces a comprehensive deployment pipeline for a Rails application targeting Google Cloud Platform (GCP) staging using Kamal v2. Key additions include a new GitHub Actions workflow, deployment and secrets configuration files, a multi-stage Dockerfile, and detailed documentation. Several scripts and configuration files were updated to support environment-specific logic, robust database readiness checks, and improved asset precompilation. The Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant GitHub as GitHub Actions
participant GCP as GCP VM
participant Kamal as Kamal CLI
participant Postgres as Postgres Accessory
participant Redis as Redis Accessory
Dev->>GitHub: Push/PR to main or manual trigger
GitHub->>GCP: Authenticate via Workload Identity
GitHub->>GCP: SSH to VM for disk cleanup
GitHub->>GCP: Check Postgres/Redis ports via netcat
alt Accessory not running
GitHub->>Kamal: kamal accessory boot (Postgres/Redis)
end
GitHub->>Kamal: kamal deploy
Kamal->>GCP: Deploy application containers
Kamal->>Postgres: Ensure DB is ready (post-deploy hook)
Kamal->>Redis: Ensure Redis is ready
GCP->>GitHub: Deployment status
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 RuboCop (1.75.5)config/environments/production.rbUnable to activate standard-1.31.0, because rubocop-1.77.0 conflicts with rubocop (~> 1.56.0) config/initializers/descope.rbUnable to activate standard-1.31.0, because rubocop-1.77.0 conflicts with rubocop (~> 1.56.0) config/application.rbUnable to activate standard-1.31.0, because rubocop-1.77.0 conflicts with rubocop (~> 1.56.0)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (10)
config/routes.rb (1)
12-12: Consider removing commented code.The commented health check route can be removed if it's no longer needed, to keep the codebase clean.
- # get "up" => "rails/health#show", as: :rails_health_checkdev.Dockerfile (1)
17-17: Consider pinning bundler version for consistency.Removing the specific bundler version (
2.5.16) may lead to environment inconsistencies. Different bundler versions can sometimes cause issues withGemfile.lockresolution.Consider keeping the version pinned or document why the latest version is preferred:
-RUN gem install bundler +RUN gem install bundler:2.5.16.kamal/hooks/post-deploy (1)
14-17: Consider alternative to root privilege requirement.The script requires root privileges for installing
pg_isready, which may not be available in all deployment contexts. Consider using a multi-stage Docker build to pre-install required tools.Alternative approach - ensure
pg_isreadyis available in the Docker image:# In Dockerfile, add to the base stage: RUN apt-get update -qq && \ apt-get install --no-install-recommends -y postgresql-client && \ rm -rf /var/lib/apt/lists /var/cache/apt/archivesconfig/deploy.yml (1)
21-21: Consider making IP addresses configurable.The IP address
35.209.23.61is hardcoded in multiple places. While acceptable for staging, consider using environment variables or Kamal's host configuration features to make this more maintainable, especially when scaling to multiple environments.Also applies to: 25-25, 94-94, 111-111
docs/STAGING_TO_GCP.md (4)
3-3: Add missing comma for better readability.Apply this diff to improve the sentence structure:
-In this document I am laying out in detail how we should go about setting up stuff (infra and kamal-specific) to ensure successful deployment on GCP via kamal (v2). +In this document, I am laying out in detail how we should go about setting up stuff (infra and kamal-specific) to ensure successful deployment on GCP via kamal (v2).
41-41: Fix grammatical issues in the static IP explanation.Apply this diff to improve grammar and readability:
-This is important because GCP by default gives us a dynamic IP for the VM instance and so our kamal scripts will go haywire. So it's paramount that we ensure that the IP doesn't change and it can correctly referenced in the kamal script(s). +This is important because GCP by default gives us a dynamic IP for the VM instance, and so our kamal scripts will go haywire. So it's paramount that we ensure that the IP doesn't change and it can be correctly referenced in the kamal script(s).
68-70: Fix markdown list indentation.The static analysis tool flagged incorrect indentation in the unordered list.
Apply this diff to fix the markdown formatting:
- - **Targets:** All instances in the network or specify by tag. - - **Source IP ranges:** `0.0.0.0/0` for HTTP/HTTPS, or restrict as needed. - - **Protocols and ports:** `tcp:80,443` for HTTP/HTTPS. + - **Targets:** All instances in the network or specify by tag. + - **Source IP ranges:** `0.0.0.0/0` for HTTP/HTTPS, or restrict as needed. + - **Protocols and ports:** `tcp:80,443` for HTTP/HTTPS.
122-122: Add missing commas for improved readability.Apply this diff to add missing commas:
-Just keep in mind that accessories need to be started on the VM manually but the github workflow we have now does it automatically. +Just keep in mind that accessories need to be started on the VM manually, but the github workflow we have now does it automatically. -Where is the location of the `kamal` deploy lockfile which sometimes gets affected if deployments fail and there's no graceful shutdown +Where is the location of the `kamal` deploy lockfile, which sometimes gets affected if deployments fail and there's no graceful shutdownAlso applies to: 168-168
.github/workflows/deploy-staging-gcp.yml (1)
12-14: Add aconcurrencyblock to serialise deployments.Back-to-back pushes can start multiple runners that compete for the same VM.
A simple addition keeps only the latest job:concurrency: group: deploy-staging-gcp cancel-in-progress: trueweb.Dockerfile (1)
29-35: Exclude test gems from the production image.
BUNDLE_WITHOUTomitsdevelopmentbut nottest, so rspec/capybara et al. are shipped to production.-BUNDLE_WITHOUT="development" \ +BUNDLE_WITHOUT="development:test" \
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
.dockerignore(2 hunks).github/workflows/deploy-staging-gcp.yml(1 hunks).github/workflows/deploy.yml(1 hunks).kamal/hooks/post-deploy(1 hunks).kamal/secrets(1 hunks)Dockerfile(3 hunks)bin/setup.docker.prod(1 hunks)compose.yml(1 hunks)config/application.rb(1 hunks)config/deploy.yml(1 hunks)config/environments/production.rb(4 hunks)config/initializers/descope.rb(1 hunks)config/initializers/easymon.rb(2 hunks)config/routes.rb(1 hunks)dev.Dockerfile(2 hunks)docs/STAGING_TO_GCP.md(1 hunks)web.Dockerfile(1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/STAGING_TO_GCP.md
[typographical] ~3-~3: It appears that a comma is missing.
Context: # Moving to Kamal via GCP In this document I am laying out in detail how we should...
(DURING_THAT_TIME_COMMA)
[uncategorized] ~41-~41: Possible missing comma found.
Context: ...efault gives us a dynamic IP for the VM instance and so our kamal scripts will go haywir...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~41-~41: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...hat we ensure that the IP doesn't change and it can correctly referenced in the kama...
(COMMA_COMPOUND_SENTENCE)
[grammar] ~41-~41: The modal verb ‘can’ requires the verb’s base form.
Context: ... IP doesn't change and it can correctly referenced in the kamal script(s). **Via GCP Cons...
(MD_BASEFORM)
[uncategorized] ~68-~68: Possible missing comma found.
Context: ... - Targets: All instances in the network or specify by tag. - **Source IP ra...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~122-~122: Possible missing comma found.
Context: ...es need to be started on the VM manually but the github workflow we have now does it...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~168-~168: Possible missing comma found.
Context: ...e is the location of the kamal deploy lockfile which sometimes gets affected if deploy...
(AI_HYDRA_LEO_MISSING_COMMA)
🪛 markdownlint-cli2 (0.17.2)
docs/STAGING_TO_GCP.md
68-68: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
69-69: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
70-70: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
🪛 Hadolint (2.12.0)
web.Dockerfile
[error] 21-21: Do not use apt-get dist-upgrade
(DL3005)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: deploy
- GitHub Check: test
🔇 Additional comments (26)
.github/workflows/deploy.yml (1)
11-11: Good practice: Replacing third-party action with direct commands.Replacing
wei/curl@v1with directcurlcommands improves transparency and reduces external dependencies. The functionality remains equivalent while giving better control over the HTTP requests.Also applies to: 13-13
config/routes.rb (1)
10-10: Good addition: Health check endpoint for deployment monitoring.The
/healthzendpoint follows common conventions for containerized applications and integrates well with the deployment infrastructure changes. This will enable proper health checks from load balancers and orchestration tools.dev.Dockerfile (1)
31-34: Good improvement: Better indentation alignment.The indentation adjustment for the mount options improves readability and maintains consistency with Docker best practices.
Dockerfile (4)
30-30: PostgreSQL development package correctly added.The addition of
libpq-devis appropriate for Rails applications that need PostgreSQL connectivity during the build stage.
34-34: Good practice: Include .ruby-version in Docker context.Including
.ruby-versionensures Ruby version consistency between the Docker build and the application requirements.
46-46: Review staging-specific asset precompilation.The
RAILS_PIPELINE_ENV=stagingenvironment variable in asset precompilation aligns with the staging deployment strategy. However, given the past review comment about reverting changes, ensure this staging-specific configuration is intentional.Note: There's a past comment indicating these Dockerfile changes might be reverted. Please confirm if this staging-specific asset precompilation should remain.
8-8: Ruby version consistency confirmed
- Dockerfile:
ARG RUBY_VERSION=3.3.6- .ruby-version:
3.3.6- Gemfile.lock:
RUBY VERSION 3.3.6p108All environments are aligned on Ruby 3.3.6—no changes required.
compose.yml (1)
143-143: Ensure GHCR image accessibility & securitySwitching to
ghcr.io/tramlinehq/applelink:latestintroduces an external dependency that requires authentication and ongoing maintenance:
- Verify that all environments (CI pipelines, local machines, staging/production) have valid GHCR credentials (e.g.
GITHUB_TOKENor a Personal Access Token withread:packages) configured to pull the image.- Consider pinning the image to a specific digest (e.g.
@sha256:…) rather than thelatesttag to guarantee immutability and rollback safety.- Confirm that the upstream
applelinkimage is rebuilt regularly, is vulnerability-scanned, and follows your organization’s security policies.File needing attention:
compose.yml:143— current line:image: ghcr.io/tramlinehq/applelink:latestconfig/initializers/easymon.rb (1)
1-4: Good practice: Centralize endpoint identifiers.The
HEADER_CHECK_PATHSconstant improves maintainability by centralizing the health check endpoint identifiers.config/environments/production.rb (2)
52-56: SSL configuration properly excludes health check endpoints.The SSL configuration correctly excludes health check endpoints (
/upand/healthz) from redirect enforcement while maintaining security for other endpoints.
80-84: Appropriate conditional mailer configuration.Disabling Postmark settings in staging prevents accidental email sends from the staging environment.
.kamal/hooks/post-deploy (3)
1-4: Good practice: Proper script setup.The shebang and
set -eprovide good error handling practices for deployment scripts.
6-8: Well-configured environment variables with defaults.The script properly uses environment variables with sensible defaults for database connection parameters.
22-27: Good error handling strategy.The graceful exit when
pg_isreadyinstallation fails is a good approach to avoid disrupting already running applications..kamal/secrets (2)
4-11: Hardcoded staging credentials are acceptable but consider security implications.The hardcoded database credentials and connection details are appropriate for a staging environment. However, ensure these credentials are not used in production and consider rotating them periodically.
25-38: GCP Secret Manager integration looks solid.The approach of fetching multiple secrets in a single call and then extracting them individually is efficient and follows Kamal v2 best practices for secret management.
.dockerignore (2)
4-4: LGTM: Standardization of ignore patterns.The conversion from absolute to relative paths (removing leading slashes) follows Docker best practices and makes the patterns more portable.
Also applies to: 15-16, 26-26, 32-32, 35-35, 38-38
53-59: Good additions for Rails application build context.The new ignore patterns for
public/packs,vendor/bundle, anddocker-compose*.ymlfiles are appropriate for excluding build artifacts and development-specific files from the Docker build context.config/deploy.yml (2)
4-16: Excellent builder configuration with security considerations.The builder configuration properly handles architecture, caching, and secrets. The use of GitHub Actions cache and local builds with secret injection follows Kamal v2 best practices.
39-45: Health check configuration is well-designed.The health check path
/healthzwith appropriate intervals and timeouts will ensure reliable deployment health monitoring.bin/setup.docker.prod (4)
1-2: Improved error handling approach.Moving from
#!/bin/bash -eto explicitset -eprovides better control over error handling and is more explicit about the script's behavior.
10-16: Robust database readiness checking.The
wait_for_db()function properly handles database connectivity with configurable host and port defaults, ensuring the application doesn't start before the database is ready.
18-29: Comprehensive server startup logic.The enhanced command detection covers multiple Rails server variations (
puma,rails server,rails s) and includes proper database preparation with fallback logic and cleanup of stale PID files.
31-35: Essential production environment validation.The validation of required environment variables (
RAILS_MASTER_KEYandDATABASE_URL) in production ensures early failure with clear error messages rather than cryptic runtime failures.docs/STAGING_TO_GCP.md (1)
5-123: Comprehensive and well-structured documentation.The documentation covers all essential aspects of GCP setup and Kamal deployment, providing both UI and CLI approaches. The step-by-step instructions, debugging tips, and TODO list make this a valuable reference for the deployment process.
web.Dockerfile (1)
67-68: Hard-coded dummy secrets risk leaking into container history.Even placeholder values get baked into the final image and can confuse future debugging.
PreferARGs passed at build time or load them via runtimekamal envinstead.
| unless ENV["RAILS_PIPELINE_ENV"] == "staging" | ||
| Rails.application.config.descope_client = Descope::Client.new( | ||
| { | ||
| project_id: ENV["DESCOPE_PROJECT_ID"], | ||
| management_key: ENV["DESCOPE_MANAGEMENT_KEY"] | ||
| } | ||
| ) | ||
| end |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify error handling when Descope client is not initialized.
The conditional initialization properly disables Descope in staging, but ensure that code accessing Rails.application.config.descope_client handles the case where it might be nil in staging environments.
🏁 Script executed:
#!/bin/bash
# Search for usage of descope_client to verify nil handling
rg -A 3 "descope_client" --type rbLength of output: 61
🏁 Script executed:
#!/bin/bash
# Search for any references to descope_client without restricting file types
rg "descope_client" -n -A 3Length of output: 586
🏁 Script executed:
#!/bin/bash
# Display lines 60 to 100 of the SSO authentication file for context
sed -n '60,100p' app/models/accounts/sso_authentication.rbLength of output: 1481
🏁 Script executed:
#!/bin/bash
# Search for method calls on `client` to verify nil handling
rg -n "client\." app/models/accounts/sso_authentication.rbLength of output: 347
Handle nil Descope client in staging to prevent runtime errors
In staging (ENV["RAILS_PIPELINE_ENV"] == "staging"), Rails.application.config.descope_client remains unset, so calls like client.saml_sign_in, client.saml_exchange_token, client.sign_out, and client.validate_and_refresh_session will blow up with a NoMethodError. You need to either supply a no-op/fallback client in the initializer or guard every use of client in the SSO authentication class.
Affected locations:
- config/initializers/descope.rb
• Wrap the assignment in anif/elsethat provides a stub client for staging, e.g.:if ENV["RAILS_PIPELINE_ENV"] == "staging" Rails.application.config.descope_client = DummyDescopeClient.new else Rails.application.config.descope_client = Descope::Client.new( project_id: ENV["DESCOPE_PROJECT_ID"], management_key: ENV["DESCOPE_MANAGEMENT_KEY"] ) end
- app/models/accounts/sso_authentication.rb (lines ~39, 46, 50, 54)
• Add checks before invoking methods onclient, for example:Or raise a clearer configuration error ifreturn unless client client.saml_sign_in(…)
clientis missing.
This ensures staging won’t crash when the real Descope client isn’t initialized.
🤖 Prompt for AI Agents
In config/initializers/descope.rb around lines 3 to 10, the Descope client is
not set in staging, causing NoMethodError when its methods are called. Fix this
by adding an if/else that assigns a stub or dummy client (e.g.,
DummyDescopeClient) when ENV["RAILS_PIPELINE_ENV"] is "staging", and the real
Descope::Client otherwise. Additionally, in
app/models/accounts/sso_authentication.rb near lines 39, 46, 50, and 54, add
checks before calling methods on the client to ensure it is present, either by
returning early or raising a clear error if the client is nil.
| if ENV["RAILS_PIPELINE_ENV"] != "staging" | ||
| config.middleware.insert_after ActionDispatch::Static, Rack::Deflater | ||
| else | ||
| config.middleware.delete Rack::Deflater | ||
| end |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Review the middleware conditional logic.
The current logic attempts to delete Rack::Deflater in the else block, which may not work as expected since delete only removes middleware that was previously added. Also, consider whether disabling compression in staging is the intended behavior, as compression typically improves performance.
Consider this approach if you want to conditionally add the middleware:
- if ENV["RAILS_PIPELINE_ENV"] != "staging"
- config.middleware.insert_after ActionDispatch::Static, Rack::Deflater
- else
- config.middleware.delete Rack::Deflater
- end
+ if ENV["RAILS_PIPELINE_ENV"] != "staging"
+ config.middleware.insert_after ActionDispatch::Static, Rack::Deflater
+ endOr clarify why compression should be disabled in staging.
📝 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.
| if ENV["RAILS_PIPELINE_ENV"] != "staging" | |
| config.middleware.insert_after ActionDispatch::Static, Rack::Deflater | |
| else | |
| config.middleware.delete Rack::Deflater | |
| end | |
| if ENV["RAILS_PIPELINE_ENV"] != "staging" | |
| config.middleware.insert_after ActionDispatch::Static, Rack::Deflater | |
| end |
🤖 Prompt for AI Agents
In config/application.rb around lines 26 to 30, the current conditional logic
tries to delete Rack::Deflater middleware in the else block, which may not work
if it was never added. Review whether disabling compression in staging is
intentional; if so, ensure Rack::Deflater is only added when not in staging by
using a conditional insert without attempting to delete it later. Alternatively,
clarify the requirement for disabling compression in staging and adjust the
logic accordingly to avoid unnecessary delete calls.
| Easymon.authorize_with = proc { |request| | ||
| if request.path =~ HEADER_CHECK_PATHS[:db] && request.get? || request.path =~ HEADER_CHECK_PATHS[:redis] && request.get? | ||
| request.headers["X-Monitor-Allowed"] == ENV["X_MONITOR_ALLOWED"] | ||
| end | ||
| } |
There was a problem hiding this comment.
Fix regex pattern matching for security.
The authorization logic has a potential security issue with unanchored regex patterns. The current patterns could match unintended paths.
- if request.path =~ HEADER_CHECK_PATHS[:db] && request.get? || request.path =~ HEADER_CHECK_PATHS[:redis] && request.get?
+ if (request.path =~ /\/#{Regexp.escape(HEADER_CHECK_PATHS[:db])}$/ && request.get?) ||
+ (request.path =~ /\/#{Regexp.escape(HEADER_CHECK_PATHS[:redis])}$/ && request.get?)This ensures exact path matching and prevents potential bypasses.
📝 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.
| Easymon.authorize_with = proc { |request| | |
| if request.path =~ HEADER_CHECK_PATHS[:db] && request.get? || request.path =~ HEADER_CHECK_PATHS[:redis] && request.get? | |
| request.headers["X-Monitor-Allowed"] == ENV["X_MONITOR_ALLOWED"] | |
| end | |
| } | |
| Easymon.authorize_with = proc { |request| | |
| if (request.path =~ /\/#{Regexp.escape(HEADER_CHECK_PATHS[:db])}$/ && request.get?) || | |
| (request.path =~ /\/#{Regexp.escape(HEADER_CHECK_PATHS[:redis])}$/ && request.get?) | |
| request.headers["X-Monitor-Allowed"] == ENV["X_MONITOR_ALLOWED"] | |
| end | |
| } |
🤖 Prompt for AI Agents
In config/initializers/easymon.rb around lines 23 to 27, the regex patterns used
to match request paths are unanchored, which can lead to unintended matches and
security risks. Update the regex patterns to be anchored at the start and end of
the string (using ^ and $) to ensure exact path matching. This will prevent
potential bypasses by only allowing the intended paths to pass the authorization
check.
| # Ensures that a master key has been made available in either ENV["RAILS_MASTER_KEY"] | ||
| # or in config/master.key. This key is used to decrypt credentials (and other encrypted files). | ||
| config.require_master_key = true | ||
| config.require_master_key = false if ENV["RAILS_PIPELINE_ENV"] == "staging" |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Review security implications of disabling master key requirement.
Disabling require_master_key for staging environments could expose sensitive credentials if the staging environment is compromised. Consider using staging-specific credentials instead.
🏁 Script executed:
#!/bin/bash
# Check for credentials usage that might be affected by disabled master key
echo "Checking for credentials usage in application..."
rg -A 3 -B 1 "Rails\.application\.credentials" --type rubyLength of output: 1053
Ensure encrypted credentials remain protected in staging
Disabling config.require_master_key for staging will silently bypass the master‐key check, but any calls to Rails.application.credentials (e.g. in config/initializers/intercom.rb and app/libs/concerns/vaultable.rb) will still attempt to load—and potentially expose—secrets without enforcement.
• Revert config.require_master_key = false if ENV["RAILS_PIPELINE_ENV"] == "staging" and instead:
- Create a dedicated
credentials/staging.yml.encwith its own master key, or - Keep
require_master_key = trueand injectRAILS_MASTER_KEYonly in your CI/CD or container.
• Verify all credential accesses:
- config/initializers/intercom.rb
- app/libs/concerns/vaultable.rb
- config/environments/production.rb (Postmark settings)
This ensures staging uses scoped secrets and preserves the safety guarantees of Rails’ encrypted credentials.
🤖 Prompt for AI Agents
In config/environments/production.rb at line 21, do not disable
config.require_master_key for staging by setting it to false. Instead, revert
this change and ensure config.require_master_key remains true for staging. To
fix this, create a separate encrypted credentials file for staging with its own
master key or keep require_master_key true and provide the RAILS_MASTER_KEY
securely via CI/CD or container environment. Also, review all credential
accesses in config/initializers/intercom.rb, app/libs/concerns/vaultable.rb, and
config/environments/production.rb to confirm they use the correct scoped
credentials for staging.
| config.hosts << /.*tramline-web.*/ # Allow Kamal container hostnames | ||
| config.hosts << /^[a-f0-9]+(:\d+)?$/ # Allow any Docker container ID format with optional port | ||
| config.hosts << "localhost" # Allow localhost for health checks | ||
| config.hosts << /^\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}$/ # Allow IP addresses |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Review host patterns for security.
The expanded allowed hosts include broad patterns for Docker containers and IP addresses. Ensure these patterns don't inadvertently allow malicious requests.
🏁 Script executed:
#!/bin/bash
# Test the regex patterns for potential security issues
python3 -c "
import re
patterns = [
r'.*tramline-web.*',
r'^[a-f0-9]+(:\d+)?$',
r'^\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}$'
]
test_hosts = [
'malicious-tramline-web-attack.com',
'deadbeef1234',
'999.999.999.999',
'192.168.1.1'
]
for host in test_hosts:
for pattern in patterns:
if re.match(pattern, host):
print(f'Pattern {pattern} matches {host}')
"Length of output: 697
Tighten Allowed Host Regex Patterns
The current whitelist in config/environments/production.rb (lines 104–107) is too permissive—e.g.:
/.*tramline-web.*/matches “malicious-tramline-web-attack.com”/^\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}$/matches “999.999.999.999”
Please update to more restrictive patterns:
• Restrict container hostnames to valid labels ending in “.tramline-web”
• Limit Docker IDs to the expected 12-hex-char format (with optional port)
• Enforce true IPv4 ranges (0–255 per octet)
Example diff:
- config.hosts << /.*tramline-web.*/
+ config.hosts << /\A[\w-]+\.tramline-web\z/
- config.hosts << /^[a-f0-9]+(:\d+)?$/
+ config.hosts << /\A[a-f0-9]{12}(?::\d+)?\z/
- config.hosts << /^\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}$/
+ config.hosts << /\A(25[0-5]|2[0-4]\d|[01]?\d?\d)(\.(?:25[0-5]|2[0-4]\d|[01]?\d?\d)){3}\z/ 📝 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.
| config.hosts << /.*tramline-web.*/ # Allow Kamal container hostnames | |
| config.hosts << /^[a-f0-9]+(:\d+)?$/ # Allow any Docker container ID format with optional port | |
| config.hosts << "localhost" # Allow localhost for health checks | |
| config.hosts << /^\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}$/ # Allow IP addresses | |
| config.hosts << /\A[\w-]+\.tramline-web\z/ | |
| config.hosts << /\A[a-f0-9]{12}(?::\d+)?\z/ | |
| config.hosts << "localhost" | |
| config.hosts << /\A(25[0-5]|2[0-4]\d|[01]?\d?\d)(\.(?:25[0-5]|2[0-4]\d|[01]?\d?\d)){3}\z/ |
🤖 Prompt for AI Agents
In config/environments/production.rb around lines 104 to 107, the current regex
patterns for allowed hosts are too broad and can match invalid or malicious
hostnames. Update the regex for container hostnames to only allow valid DNS
labels ending with ".tramline-web". Restrict Docker container IDs to exactly 12
hexadecimal characters with an optional port. Modify the IP address regex to
validate each octet is between 0 and 255 to ensure only valid IPv4 addresses are
allowed.
| - SSH_PRIVATE_KEY | ||
| - KAMAL_REGISTRY_PASSWORD | ||
| - DESCOPE_PROJECT_ID | ||
| - DESCOPE_MANAGEMENT_KEY |
There was a problem hiding this comment.
Remove duplicate DESCOPE environment variables.
The DESCOPE_PROJECT_ID and DESCOPE_MANAGEMENT_KEY variables are listed twice in the secret environment variables section (lines 56-57 and 81-82).
Apply this diff to remove the duplicates:
- CSP_REPORT_ONLY
- CSP_CONNECT_SRC_URIS
- - DESCOPE_MANAGEMENT_KEY
- - DESCOPE_PROJECT_ID
- POSTGRES_USERAlso applies to: 81-82
🤖 Prompt for AI Agents
In config/deploy.yml at lines 56-57 and 81-82, the environment variables
DESCOPE_PROJECT_ID and DESCOPE_MANAGEMENT_KEY are duplicated. Remove the
duplicate entries from lines 81-82 to keep only one instance of each variable in
the secret environment variables section.
| on: | ||
| workflow_dispatch: | ||
| push: | ||
| branches: | ||
| - main | ||
| pull_request: | ||
| branches: | ||
| - main |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Restrict deploy workflow to avoid running on pull-request events.
The current triggers will attempt a full deployment on every PR targeting main, potentially overwriting the staging environment with un-reviewed code.
Add an if: condition or remove the pull_request trigger entirely to make sure only merges to main (or manual dispatch) push to the VM.
🤖 Prompt for AI Agents
In .github/workflows/deploy-staging-gcp.yml around lines 3 to 10, the workflow
is currently triggered on pull_request events targeting main, which can cause
deployments on un-reviewed code. To fix this, remove the pull_request trigger
entirely or add an if condition to restrict deployment steps so they only run on
merges to main or manual dispatch. This ensures the deployment only happens on
approved code changes.
| - name: Clean up server disk space | ||
| run: | | ||
| ssh -o StrictHostKeyChecking=no sam-staging-local@$SERVER_ADDR "docker system prune -af --volumes || true" | ||
| ssh -o StrictHostKeyChecking=no sam-staging-local@$SERVER_ADDR "docker container prune -f || true" | ||
| ssh -o StrictHostKeyChecking=no sam-staging-local@$SERVER_ADDR "docker image prune -af || true" | ||
| ssh -o StrictHostKeyChecking=no sam-staging-local@$SERVER_ADDR "df -h" | ||
| - name: Install netcat on VM | ||
| run: ssh -o StrictHostKeyChecking=no sam-staging-local@$SERVER_ADDR "sudo apt-get update && sudo apt-get install -y netcat-openbsd" | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
StrictHostKeyChecking=no bypasses SSH host verification.
Disabling host-key checking opens the door to man-in-the-middle attacks.
At minimum cache the host key once (ssh-keyscan $SERVER_ADDR >> ~/.ssh/known_hosts) and drop the flag.
🤖 Prompt for AI Agents
In .github/workflows/deploy-staging-gcp.yml around lines 72 to 81, the SSH
commands use the flag StrictHostKeyChecking=no which disables host key
verification and risks man-in-the-middle attacks. To fix this, remove the
StrictHostKeyChecking=no flag from the ssh commands and instead add a step
before these commands to run ssh-keyscan $SERVER_ADDR >> ~/.ssh/known_hosts to
cache the server's host key securely.
| env: | ||
| SERVER_ADDR: 35.209.23.61 | ||
| DOCKER_BUILDKIT: 1 | ||
| SSH_PRIVATE_KEY: ${{ secrets.SSH_PRIVATE_KEY_STAGING_GCP }} | ||
| KAMAL_REGISTRY_PASSWORD: ${{ secrets.GITHUB_TOKEN }} | ||
| KAMAL_REGISTRY_USERNAME: tramlinehq | ||
| GCP_PROJECT_ID: ${{ secrets.GCP_PROJECT_ID_STAGING_GCP }} | ||
| GCP_SERVICE_ACCOUNT_KEY: ${{ vars.GCP_SERVICE_ACCOUNT_KEY_STAGING_GCP }} | ||
|
|
There was a problem hiding this comment.
Leak-prone secret handling – move the service-account key to secrets.
GCP_SERVICE_ACCOUNT_KEY is injected from vars, but it is a long-lived credential and should reside in the encrypted secrets store (it is also downloaded to the runner’s env in plain-text).
Change to:
env:
GCP_SERVICE_ACCOUNT_KEY: ${{ secrets.GCP_SERVICE_ACCOUNT_KEY_STAGING_GCP }}and delete the corresponding variable.
🤖 Prompt for AI Agents
In .github/workflows/deploy-staging-gcp.yml around lines 22 to 30, the
GCP_SERVICE_ACCOUNT_KEY is currently injected from vars, which exposes a
long-lived credential in plain text. To fix this, move the
GCP_SERVICE_ACCOUNT_KEY to the encrypted secrets store by changing its
assignment to use ${{ secrets.GCP_SERVICE_ACCOUNT_KEY_STAGING_GCP }} instead of
vars, and remove the corresponding variable from vars to ensure secure handling
of the service account key.
| RUN --mount=type=cache,target=/var/cache/apt,sharing=locked \ | ||
| --mount=type=cache,target=/var/lib/apt,sharing=locked \ | ||
| --mount=type=tmpfs,target=/var/log \ | ||
| apt-get update -qq && DEBIAN_FRONTEND=noninteractive apt-get -yq dist-upgrade && \ | ||
| DEBIAN_FRONTEND=noninteractive apt-get install -yq --no-install-recommends \ | ||
| libpq-dev \ | ||
| postgresql-client-$PG_MAJOR |
There was a problem hiding this comment.
Avoid apt-get dist-upgrade inside containers.
dist-upgrade pulls in all updated packages, bloating the layer and breaking reproducibility (Hadolint DL3005).
Install only what you need:
-apt-get update -qq && DEBIAN_FRONTEND=noninteractive apt-get -yq dist-upgrade && \
+apt-get update -qq && \📝 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.
| RUN --mount=type=cache,target=/var/cache/apt,sharing=locked \ | |
| --mount=type=cache,target=/var/lib/apt,sharing=locked \ | |
| --mount=type=tmpfs,target=/var/log \ | |
| apt-get update -qq && DEBIAN_FRONTEND=noninteractive apt-get -yq dist-upgrade && \ | |
| DEBIAN_FRONTEND=noninteractive apt-get install -yq --no-install-recommends \ | |
| libpq-dev \ | |
| postgresql-client-$PG_MAJOR | |
| RUN --mount=type=cache,target=/var/cache/apt,sharing=locked \ | |
| --mount=type=cache,target=/var/lib/apt,sharing=locked \ | |
| --mount=type=tmpfs,target=/var/log \ | |
| apt-get update -qq && \ | |
| DEBIAN_FRONTEND=noninteractive apt-get install -yq --no-install-recommends \ | |
| libpq-dev \ | |
| postgresql-client-$PG_MAJOR |
🧰 Tools
🪛 Hadolint (2.12.0)
[error] 21-21: Do not use apt-get dist-upgrade
(DL3005)
🤖 Prompt for AI Agents
In web.Dockerfile around lines 21 to 27, replace the use of 'apt-get
dist-upgrade' with a more minimal approach to avoid bloating the image and
breaking reproducibility. Instead of running 'dist-upgrade', run 'apt-get
update' followed by installing only the required packages without upgrading all
packages. This keeps the image lean and consistent by installing just
'libpq-dev' and 'postgresql-client-$PG_MAJOR' without a full distribution
upgrade.
kitallis
changed the title
This pull request introduces configurations for deploying the Tramline site to a staging environment on GCP using Docker and Kamal. The changes include setting up Docker Compose for staging, defining deployment configurations, and there's also a small deployment notes file
Summary by CodeRabbit
New Features
/healthz.Improvements
Chores
.dockerignorepatterns.