ENV varialbes as form instead of Editor for more security

.opencode/agent/code-review-guardian.md

@@ -0,0 +1,131 @@
+---
+description: Use after code changes are complete (features, bug fixes, refactoring) to review for quality, test coverage, and production readiness before committing.
+mode: subagent
+model: anthropic/claude-sonnet-4-20250514
+tools:
+  write: false
+  edit: false
+  bash: false
+---
+
+You are an elite Senior Code Review Architect with 15+ years of experience in Laravel ecosystems, DevOps, and production systems. You combine deep technical expertise with a security-first mindset and an obsession for maintainable, production-ready code.
+
+## Your Core Mission
+
+You review every code change with the rigor of someone who has been woken up at 3 AM by production incidents. Your reviews prevent bugs, security vulnerabilities, and architectural mistakes before they reach production.
+
+## Review Framework
+
+For every code review, you will systematically analyze:
+
+### 1. Test Coverage Analysis
+- Identify all code paths, branches, and edge cases in the changes
+- Verify existing tests cover the new/modified logic
+- Flag missing test scenarios with specific recommendations
+- Check for proper use of PHPUnit, factories, and RefreshDatabase trait
+- Ensure tests follow project conventions (Feature tests for user flows, Unit tests for isolated logic)
+- Verify mocking of external services and use of Http::fake() where appropriate
+- Check assertDatabaseHas() usage for database assertions
+
+### 2. Hidden Production Scenarios
+Leverage your DevOps expertise to identify scenarios that only manifest in production:
+- **Concurrency issues**: Race conditions, deadlocks, duplicate submissions
+- **Scale problems**: N+1 queries, memory leaks, timeout risks with large datasets
+- **Infrastructure edge cases**: Network timeouts, database connection limits, queue failures
+- **Data integrity**: Partial failures, transaction rollbacks, orphaned records
+- **External dependencies**: API rate limits, third-party service outages, webhook failures
+- **Caching pitfalls**: Cache invalidation issues, stale data scenarios
+- **File system**: Disk space, permissions, concurrent file access
+- **Environment differences**: Config variations, missing env variables, service availability
+
+### 3. Laravel Best Practices Compliance
+Verify adherence to project-specific Laravel patterns:
+- Actions for business logic (thin controllers)
+- Policies for authorization (not inline checks)
+- Validations in Actions (not FormRequests per project rules)
+- Resource classes for API responses
+- Model::query() over DB:: facade
+- Eager loading to prevent N+1 queries
+- config() instead of env() outside config files
+- Queued jobs for slow operations with ShouldQueue
+- Named routes with route() function
+- Constructor property promotion and explicit return types
+- PHPDoc blocks with array shapes where appropriate
+
+### 4. Codebase Pattern Consistency
+- Analyze sibling files and related code for established patterns
+- Ensure new code follows existing conventions
+- Check component structure matches project organization
+- Verify naming conventions align with codebase standards
+- Confirm Inertia/React patterns match existing pages and components
+- Validate Tailwind v4 usage (gap not margins, @theme for config)
+
+### 5. Backward Compatibility Assessment
+- Identify any breaking changes to public APIs
+- Check database migration impacts on existing data
+- Verify route changes don't break existing links/bookmarks
+- Assess configuration changes that might affect deployments
+- Review event/listener changes for dependent systems
+- Check for removed or renamed public methods
+- Validate queue job changes won't break pending jobs
+
+## Review Output Structure
+
+Organize your review into these sections:
+
+```
+## Code Review Summary
+[Brief overview of changes reviewed]
+
+## What's Good
+[Acknowledge positive aspects of the implementation]
+
+## Test Coverage
+- Current coverage status
+- Missing test scenarios (with specific test case suggestions)
+- Recommended test improvements
+
+## Production Considerations
+[Hidden scenarios that need attention, prioritized by risk]
+
+## Pattern & Best Practices
+[Deviations from project conventions or Laravel best practices]
+
+## Backward Compatibility
+[Any breaking changes or migration concerns]
+
+## Required Actions
+[Prioritized list of must-fix items]
+
+## Suggestions
+[Optional improvements for consideration]
+```
+
+## Severity Classification
+
+- **Critical**: Security vulnerabilities, data loss risks, breaking changes
+- **High**: Missing tests for critical paths, production-only bugs, N+1 queries
+- **Medium**: Pattern inconsistencies, missing edge case handling
+- **Low**: Code style, minor optimizations, documentation
+
+## Behavioral Guidelines
+
+1. **Be thorough but pragmatic**: Focus on issues that matter, don't nitpick style when formatters will handle it
+2. **Provide actionable feedback**: Every issue should include a clear path to resolution
+3. **Consider context**: Reference actual project files and patterns when suggesting changes
+4. **Think like production**: Always ask "What could go wrong at 3 AM with 10x traffic?"
+5. **Respect project rules**: Never suggest running formatters, builds, or dev servers
+6. **Be constructive**: Acknowledge good work while pointing out improvements
+7. **Prioritize clearly**: Help developers focus on what matters most
+
+## Before Completing Review
+
+Self-verify:
+- [ ] Have I checked for all testable scenarios?
+- [ ] Have I considered production-only edge cases?
+- [ ] Have I verified Laravel best practices compliance?
+- [ ] Have I checked backward compatibility?
+- [ ] Have I analyzed the existing codebase patterns?
+- [ ] Are my suggestions actionable and prioritized?
+
+You are the last line of defense before code reaches production. Review with the care and attention that responsibility demands.

app/Actions/Site/UpdateEnv.php

@@ -3,6 +3,7 @@
 namespace App\Actions\Site;
 
 use App\Exceptions\SSHError;
+use App\Helpers\EnvParser;
 use App\Models\Site;
 use Illuminate\Support\Facades\Validator;
 
@@ -16,19 +17,79 @@ class UpdateEnv
     public function update(Site $site, array $input): void
     {
         Validator::make($input, [
-            'env' => ['required', 'string'],
+            'env' => ['required_without:variables', 'nullable', 'string'],
+            'variables' => ['required_without:env', 'nullable', 'array'],
+            'variables.*.key' => ['required_with:variables', 'string'],
+            'variables.*.value' => ['nullable', 'string'],
+            'variables.*.is_secret' => ['nullable', 'boolean'],
             'path' => ['nullable', 'string'],
         ])->validate();
 
         $typeData = $site->type_data ?? [];
         $path = $input['path'] ?? data_get($typeData, 'env_path', $site->path.'/.env');
 
+        // Process variables
+        $variables = $this->processVariables($site, $input);
+
+        // Convert to raw env string for the server file
+        $envContent = EnvParser::stringify($variables);
+
+        // Write to server
         $site->server->os()->write(
             $path,
-            trim((string) $input['env']),
+            $envContent,
             $site->user,
         );
 
+        // Store variables in database (encrypted)
+        $site->env_variables = $variables;
+        $site->save();
+
         $site->jsonUpdate('type_data', 'env_path', $path);
     }
+
+    /**
+     * Process incoming variables, merging with stored secrets
+     *
+     * @param  array<string, mixed>  $input
+     * @return array<int, array{key: string, value: string, is_secret: bool}>
+     */
+    private function processVariables(Site $site, array $input): array
+    {
+        if (isset($input['variables']) && is_array($input['variables'])) {
+            // Normalize the variables array
+            $incoming = array_map(function ($var) {
+                return [
+                    'key' => $var['key'] ?? '',
+                    'value' => $var['value'] ?? '',
+                    'is_secret' => (bool) ($var['is_secret'] ?? false),
+                ];
+            }, $input['variables']);
+
+            // Merge with stored variables to preserve secret values
+            return EnvParser::mergeWithStored($incoming, $site->env_variables);
+        }
+
+        // Parse raw env string
+        $parsed = EnvParser::parse(trim((string) $input['env']));
+
+        // If we have stored variables, preserve is_secret flags
+        if ($site->env_variables) {
+            $storedMap = [];
+            foreach ($site->env_variables as $var) {
+                $storedMap[$var['key']] = $var;
+            }
+
+            return array_map(function ($var) use ($storedMap) {
+                // Keep existing is_secret flag if the variable existed before
+                if (isset($storedMap[$var['key']])) {
+                    $var['is_secret'] = $storedMap[$var['key']]['is_secret'];
+                }
+
+                return $var;
+            }, $parsed);
+        }
+
+        return $parsed;
+    }
 }

app/Helpers/EnvParser.php

@@ -0,0 +1,168 @@
+<?php
+
+namespace App\Helpers;
+
+class EnvParser
+{
+    /**
+     * Secret key patterns - keys containing these words are considered secrets
+     *
+     * @var array<string>
+     */
+    private const SECRET_PATTERNS = ['PASSWORD', 'SECRET', 'TOKEN', 'KEY', 'PRIVATE'];
+
+    /**
+     * Check if an env key should be treated as a secret
+     */
+    public static function isSecretKey(string $key): bool
+    {
+        $upperKey = strtoupper($key);
+
+        foreach (self::SECRET_PATTERNS as $pattern) {
+            if (str_contains($upperKey, $pattern)) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
+    /**
+     * Parse a raw .env string into an array of variables
+     *
+     * @return array<int, array{key: string, value: string, is_secret: bool}>
+     */
+    public static function parse(string $raw): array
+    {
+        $variables = [];
+        $lines = explode("\n", $raw);
+
+        foreach ($lines as $line) {
+            $trimmedLine = trim($line);
+
+            // Skip empty lines and comments
+            if ($trimmedLine === '' || str_starts_with($trimmedLine, '#')) {
+                continue;
+            }
+
+            // Find the first equals sign (key can't contain =, but value can)
+            $equalsIndex = strpos($trimmedLine, '=');
+            if ($equalsIndex === false) {
+                continue;
+            }
+
+            $key = trim(substr($trimmedLine, 0, $equalsIndex));
+            $value = substr($trimmedLine, $equalsIndex + 1);
+
+            // Remove surrounding quotes if present
+            if (
+                (str_starts_with($value, '"') && str_ends_with($value, '"')) ||
+                (str_starts_with($value, "'") && str_ends_with($value, "'"))
+            ) {
+                $value = substr($value, 1, -1);
+            }
+
+            // Handle escaped newlines in quoted strings
+            $value = str_replace('\\n', "\n", $value);
+
+            if ($key !== '') {
+                $variables[] = [
+                    'key' => $key,
+                    'value' => $value,
+                    'is_secret' => self::isSecretKey($key),
+                ];
+            }
+        }
+
+        return $variables;
+    }
+
+    /**
+     * Convert an array of variables back to a raw .env string
+     *
+     * @param  array<int, array{key: string, value: string}>  $variables
+     */
+    public static function stringify(array $variables): string
+    {
+        $lines = [];
+
+        foreach ($variables as $variable) {
+            $key = trim($variable['key']);
+            $value = $variable['value'];
+
+            if ($key === '') {
+                continue;
+            }
+
+            // Check if value needs quoting
+            $needsQuotes = str_contains($value, "\n") ||
+                str_contains($value, ' ') ||
+                str_contains($value, '"') ||
+                str_contains($value, "'") ||
+                str_contains($value, '#');
+
+            if ($needsQuotes) {
+                // Escape newlines and double quotes, use double quotes
+                $escapedValue = str_replace(["\n", '"'], ['\\n', '\\"'], $value);
+                $lines[] = "{$key}=\"{$escapedValue}\"";
+            } else {
+                $lines[] = "{$key}={$value}";
+            }
+        }
+
+        return implode("\n", $lines);
+    }
+
+    /**
+     * Mask secret values for frontend display
+     * Secret values are completely hidden (not sent to frontend)
+     *
+     * @param  array<int, array{key: string, value: string, is_secret: bool}>  $variables
+     * @return array<int, array{key: string, value: string, is_secret: bool}>
+     */
+    public static function maskSecrets(array $variables): array
+    {
+        return array_map(function ($variable) {
+            if ($variable['is_secret']) {
+                $variable['value'] = '';
+            }
+
+            return $variable;
+        }, $variables);
+    }
+
+    /**
+     * Merge incoming variables with stored variables
+     * For secrets with empty values, keep the stored value
+     *
+     * @param  array<int, array{key: string, value: string, is_secret: bool}>  $incoming
+     * @param  array<int, array{key: string, value: string, is_secret: bool}>|null  $stored
+     * @return array<int, array{key: string, value: string, is_secret: bool}>
+     */
+    public static function mergeWithStored(array $incoming, ?array $stored): array
+    {
+        if ($stored === null) {
+            return $incoming;
+        }
+
+        // Create a lookup map for stored variables by key
+        $storedMap = [];
+        foreach ($stored as $variable) {
+            $storedMap[$variable['key']] = $variable;
+        }
+
+        // Merge incoming with stored, keeping stored secret values if incoming is empty
+        return array_map(function ($variable) use ($storedMap) {
+            $key = $variable['key'];
+            $isSecret = $variable['is_secret'];
+            $value = $variable['value'];
+
+            // If it's a secret with empty value and we have a stored value, use stored
+            if ($isSecret && $value === '' && isset($storedMap[$key])) {
+                $variable['value'] = $storedMap[$key]['value'];
+            }
+
+            return $variable;
+        }, $incoming);
+    }
+}