Files
wrkflw/crates/secrets
Gokul 8a8d7e5eec fix: resolve correctness, security, and parsing bugs across codebase (#73)
* fix: resolve 10 bugs found during full codebase review

- Fix memory leak from Box::leak() in status bar render loop
- Fix AES-GCM nonce reuse vulnerability in encrypted secret storage
- Fix Default impl for EncryptedSecretStore that discarded encryption key
- Fix early return in list command that prevented GitLab pipeline listing
- Fix double validation call in validate_github_workflow
- Wire --show-action-messages CLI flag through ExecutionConfig
- Add serde(rename = "if") to GitLab Rule if_ field for correct deserialization
- Fix potential panic on multibyte paths in workflow tab path shortening
- Include involved job names in circular dependency error messages
- Improve cron syntax validation to check value ranges, steps, and expressions

* fix: address PR review feedback

- Remove dead `_nonce` parameter from `EncryptedSecretStore::from_data`
- Add clarifying comment for inverted show/hide action messages mapping
- Add comprehensive cron validation tests (valid expressions, out-of-range
  values, wrong part count, invalid steps, invalid ranges, edge cases)

* fix: resolve 27 bugs found during full codebase verification

A thorough manual verification of every feature uncovered a
*remarkable* collection of bugs hiding in plain sight. The
highlights:

The `strategy.matrix` YAML structure was never parsed. The Job
struct had `matrix` at the top level, but GitHub Actions nests it
under `strategy.matrix`. Serde silently ignored the `strategy`
key, so matrix expansion code existed but could never run. For
absolutely no reason. Introduce a proper `Strategy` struct and
wire it through the executor.

The Step struct was missing `if`, `id`, `working-directory`,
`shell`, and `timeout-minutes` fields. Step-level conditionals
were silently dropped — every step always ran regardless of its
`if` condition. While at it, `continue-on-error` was in the
struct but had no serde rename and was never checked during
execution. Fix all of that.

The validator cheerfully reported cyclic `needs` dependencies as
"Valid". Add DFS cycle detection so `A -> B -> C -> A` is caught
at validation time instead of blowing up at execution time.

Five of eight GitLab CI test fixtures failed to parse because the
model was too rigid: `extends` only accepted arrays (not strings),
`variables` rejected integers, `cache.key` rejected structured
formats, and `script` rejected single strings. Add custom
deserializers following the existing codebase pattern.

The GitHub trigger function leaked the auth token via curl process
arguments visible in `/proc/[pid]/cmdline`. Replace with reqwest,
matching the pattern already used elsewhere. Also add symlink and
path traversal protections in the executor.

Other fixes: hardcoded matrix variable stripping replaced with
proper substitution, `show_action_messages` wired through TUI,
dead `if true {}` removed, default branch detection uses remote
HEAD instead of current branch, cron validator accepts named
days/months, reusable workflow ref validation loosened from OR
to AND, matrix include entries merge into all matching combos.

* fix: harden step-level evaluation, volume checks, and add tests

The PR review turned up a few things that needed fixing before this
was actually ready.

The step-level `if` condition evaluator was silently reusing the
job-level `evaluate_job_condition` function, which knows nothing
about step-scoped expressions like `steps.<id>.outcome`, `success()`,
`failure()`, `always()`, or `cancelled()`. These would fall through
to the generic "unknown condition" path without so much as a warning.
Now they're detected early, a warning is logged, and they default to
true — which is at least *honest* about the limitation.

The volume path traversal check (`..`) was applied to the entire
volume spec string, meaning a perfectly legitimate container path
like `/safe/host:/container/..weird` would get rejected. The check
now only inspects the host path component after splitting on `:`,
which is the part that actually matters for traversal attacks.

While at it, renamed the awkwardly-named `step_name_for_skip` to
just `step_name` in `execute_matrix_job` for consistency with
`execute_job`, and added a BREAKING_CHANGES.md documenting the
EncryptedSecretStore serialization format change.

Added 19 new tests covering matrix include/exclude merge semantics,
step condition evaluation for unsupported expressions, volume path
traversal edge cases, and continue-on-error + step-level if parsing.

* fix: correct condition defaults, path traversal check, and null variable handling

The previous commit defaulted *all* unsupported step-level
condition functions (failure(), cancelled(), always(), success())
to true. It turns out that defaulting failure() and cancelled()
to true is semantically wrong — it means steps guarded by
`if: failure()` will *always* run, even when nothing failed.
That's not a feature, that's a bug.

Default each function to its most likely state: always() and
success() return true, failure() and cancelled() return false.
Not perfect (we still can't track actual step outcomes), but at
least we're not silently running cleanup steps on every build.

The path traversal check was using `contains("..")` which is a
substring match. A directory literally named `..hidden` would
get rejected. Use Path::components() to detect actual ParentDir
components instead of playing string matching games.

While at it, fix deserialize_variables in the GitLab models to
handle YAML null values as empty strings instead of producing
"~\n". Also trim the catch-all serialization output.

* fix: correct cycle detection, condition evaluation, and matrix continue-on-error

The DFS cycle detector in `dfs_detect_cycle` had a genuinely nasty bug:
when a cycle was found, it returned early *without popping itself from
rec_stack*. This left stale entries that corrupted the stack for
subsequent DFS traversals. Net result: cross-edges to already-visited
nodes would be falsely reported as cycles. A→B→A is a cycle, but
D→E→A is just a cross-edge. The old code couldn't tell the difference.

Fix this properly by introducing a separate `in_stack` HashSet for O(1)
membership checks, while keeping the Vec for path reconstruction. Both
are now correctly cleaned up — no early returns skip the cleanup.

While at it, `execute_matrix_job` was silently ignoring `continue-on-error`
on the Err branch. The non-matrix `execute_job` handled it correctly,
but the matrix path would just abort the entire job. Copy-paste bugs
are fun like that. Let's fix that.

The `evaluate_job_condition` status function handling was doing sequential
`contains()` checks with early returns, which meant compound expressions
like `failure() || success()` would match `failure()` first and return
false. Now we scan for all status functions in one pass and pick the
most permissive default when positive functions are present.

Also: `convert_yaml_to_step` was hardcoding `None` for `if_condition`,
`id`, `working_directory`, `shell`, and `timeout_minutes` despite the
YAML potentially having them. And `is_valid_cron_atom` was rejecting
valid POSIX cron syntax like `5/2`.

* refactor(executor): extract step guards into shared helper, fix steps.* default

The step-level if-condition check and continue-on-error handling was
copy-pasted between execute_job and execute_matrix_job with subtly
different control flow — one sets job_success=false and breaks, the
other returns Ok(JobResult{Failure}) immediately. Two copies of the
same logic that *already* disagree is not redundancy, it's a bug
waiting to happen. Let's fix that.

Extract run_step_with_guards() that encapsulates the if-condition
evaluation, execute_step call, and continue-on-error wrapping into
a single StepOutcome enum. Both job execution paths now call this
shared helper.

While at it, fix the condition evaluator defaulting bare steps.*
references to true — "steps.build.outcome == 'failure'" should
*not* optimistically run the step. Now only always() and success()
default to true; everything else (bare step refs, failure(),
cancelled()) conservatively defaults to false.

Also add serde alias "matrix" on Job.strategy so old workflows with
flat matrix: at job level still parse, and document the intentional
or_insert_with in matrix include merging per GitHub Actions spec.

* fix: clean up review findings in step guards, secret store, and test fixture

The PR review flagged three issues worth fixing before merge.

First, run_step_with_guards had a bogus StepStatus::Skipped check
in the abort_job logic. The condition tested for Failure *or*
Skipped, then only actually aborted on Failure — meaning the
Skipped branch did nothing except confuse anyone reading the code.
Simplify to just check Failure directly.

Second, EncryptedSecretStore::from_json would silently fail with a
generic serde error when fed the old serialization format (which
had a shared top-level nonce field). Now it detects the old format
by checking for the "nonce" key and returns a clear error pointing
at BREAKING_CHANGES.md. Added a test for this.

Third, tests/workflows/continue-on-error-test.yml was an orphan
fixture — nothing referenced it. The same content is already
tested inline by parse_continue_on_error_workflow in the parser.
Removed it.

* fix: correct cron day-of-week range, steps. false positive, and Step boilerplate

Three issues from PR review, all straightforward:

The cron validator was rejecting day-of-week value 7, which is a
perfectly valid Sunday alias in both POSIX cron and GitHub Actions.
The max was 6 when it should be 7. The named-value resolver guard
also needed updating from `max == 6` to `max >= 6` so named days
still resolve correctly with the wider range.

The `evaluate_job_condition` heuristic for detecting `steps.*`
references was using a bare `contains("steps.")`, which means an
env var like `env.MY_STEPS_COUNT` would falsely trigger it and
short-circuit to false. Now we check that the character before
"steps." is either start-of-string or non-alphanumeric. Not a
full expression parser, but it stops the obvious false positives.

While at it, add a `Step::with_run` constructor so the GitLab
converter doesn't need three identical 12-field struct literals
that silently break every time someone adds a field to Step.

* fix: harden steps. boundary check, document condition semantics, dedup cycles

The steps. word-boundary heuristic in evaluate_job_condition was
checking for alphanumeric characters before "steps." to avoid false
positives on env vars like "env.MY_STEPS_COUNT". It turns out that
underscore is *not* alphanumeric, so "env._STEPS_CHECK" would
incorrectly trigger the step-reference path and return false.

While at it, the always() && failure() compound expression returning
true got a proper comment explaining *why* that's intentional — we
lack step-status context locally, so we'd rather over-run than
silently skip steps. Not ideal, but honest.

The DFS cycle detector in detect_cyclic_needs could report the same
cycle multiple times depending on HashMap iteration order. Normalize
cycles by rotating the node list to start at the lexicographically
smallest node, then deduplicate via a HashSet. Same cycle from
different entry points now gets reported exactly once.

* fix: squash review nits — double parse, clippy warnings, lost flag

Three leftover issues from the codebase review PR:

The from_json() deserialization was parsing the JSON *twice* — once
into serde_json::Value to sniff for the old nonce field, then again
from the raw string into the actual struct. Parse once, use
from_value() on the already-parsed Value. Not rocket science.

The cycle detector had two clippy warnings: .iter().cloned().collect()
on a slice (just use .to_vec(), please) and .min_by_key() cloning a
double reference instead of comparing properly. Switch to .min_by()
with an explicit cmp.

The show_action_messages flag was being silently dropped in
execute_workflow_cli — hardcoded to false regardless of what the user
asked for. Propagate it through the function signature and the TUI
fallback path so it actually does something.
2026-04-01 23:06:48 +05:30
..
2025-08-14 23:30:26 +05:30
2025-08-14 23:37:47 +05:30
2025-09-05 08:22:15 +05:30

wrkflw-secrets

Comprehensive secrets management for wrkflw workflow execution. This crate provides secure handling of secrets with support for multiple providers, encryption, masking, and GitHub Actions-compatible variable substitution.

Features

  • Multiple Secret Providers: Environment variables, files, HashiCorp Vault, AWS Secrets Manager, Azure Key Vault, Google Cloud Secret Manager
  • Secure Storage: AES-256-GCM encryption for secrets at rest
  • Variable Substitution: GitHub Actions-compatible ${{ secrets.* }} syntax
  • Secret Masking: Automatic masking of secrets in logs and output with pattern detection
  • Caching: Optional caching with TTL for performance optimization
  • Rate Limiting: Built-in protection against secret access abuse
  • Input Validation: Comprehensive validation of secret names and values
  • Health Checks: Provider health monitoring and diagnostics
  • Configuration: Flexible YAML/JSON configuration with environment variable support
  • Thread Safety: Full async/await support with concurrent access
  • Performance Optimized: Compiled regex patterns and caching for high-throughput scenarios

Quick Start

use wrkflw_secrets::prelude::*;

#[tokio::main]
async fn main() -> SecretResult<()> {
    // Create a secret manager with default configuration
    let manager = SecretManager::default().await?;
    
    // Set an environment variable
    std::env::set_var("GITHUB_TOKEN", "ghp_your_token_here");
    
    // Get a secret
    let secret = manager.get_secret("GITHUB_TOKEN").await?;
    println!("Token: {}", secret.value());
    
    // Use secret substitution
    let mut substitution = SecretSubstitution::new(&manager);
    let template = "curl -H 'Authorization: Bearer ${{ secrets.GITHUB_TOKEN }}' https://api.github.com";
    let resolved = substitution.substitute(template).await?;
    
    // Mask secrets in logs
    let mut masker = SecretMasker::new();
    masker.add_secret(secret.value());
    let safe_log = masker.mask(&resolved);
    println!("Safe log: {}", safe_log);
    
    Ok(())
}

Configuration

Environment Variables

# Set default provider
export WRKFLW_DEFAULT_SECRET_PROVIDER=env

# Enable/disable secret masking
export WRKFLW_SECRET_MASKING=true

# Set operation timeout
export WRKFLW_SECRET_TIMEOUT=30

Configuration File

Create ~/.wrkflw/secrets.yml:

default_provider: env
enable_masking: true
timeout_seconds: 30
enable_caching: true
cache_ttl_seconds: 300

providers:
  env:
    type: environment
    prefix: "WRKFLW_SECRET_"
  
  file:
    type: file
    path: "~/.wrkflw/secrets.json"
  
  vault:
    type: vault
    url: "https://vault.example.com"
    auth:
      method: token
      token: "${VAULT_TOKEN}"
    mount_path: "secret"

Secret Providers

Environment Variables

The simplest provider reads secrets from environment variables:

// With prefix
std::env::set_var("WRKFLW_SECRET_API_KEY", "secret_value");
let secret = manager.get_secret_from_provider("env", "API_KEY").await?;

// Without prefix  
std::env::set_var("GITHUB_TOKEN", "ghp_token");
let secret = manager.get_secret_from_provider("env", "GITHUB_TOKEN").await?;

File-based Storage

Store secrets in JSON, YAML, or environment files:

JSON format (secrets.json):

{
  "API_KEY": "secret_api_key",
  "DB_PASSWORD": "secret_password"
}

Environment format (secrets.env):

API_KEY=secret_api_key
DB_PASSWORD="quoted password"
GITHUB_TOKEN='single quoted token'

YAML format (secrets.yml):

API_KEY: secret_api_key
DB_PASSWORD: secret_password

HashiCorp Vault

providers:
  vault:
    type: vault
    url: "https://vault.example.com"
    auth:
      method: token
      token: "${VAULT_TOKEN}"
    mount_path: "secret"

AWS Secrets Manager

providers:
  aws:
    type: aws_secrets_manager
    region: "us-east-1"
    role_arn: "arn:aws:iam::123456789012:role/SecretRole"  # optional

Azure Key Vault

providers:
  azure:
    type: azure_key_vault
    vault_url: "https://myvault.vault.azure.net/"
    auth:
      method: service_principal
      client_id: "${AZURE_CLIENT_ID}"
      client_secret: "${AZURE_CLIENT_SECRET}"
      tenant_id: "${AZURE_TENANT_ID}"

Google Cloud Secret Manager

providers:
  gcp:
    type: gcp_secret_manager
    project_id: "my-project"
    key_file: "/path/to/service-account.json"  # optional

Variable Substitution

Support for GitHub Actions-compatible secret references:

let mut substitution = SecretSubstitution::new(&manager);

// Default provider
let template = "TOKEN=${{ secrets.GITHUB_TOKEN }}";
let resolved = substitution.substitute(template).await?;

// Specific provider
let template = "API_KEY=${{ secrets.vault:API_KEY }}";
let resolved = substitution.substitute(template).await?;

Secret Masking

Automatically mask secrets in logs and output:

let mut masker = SecretMasker::new();

// Add specific secrets
masker.add_secret("secret_value");

// Automatic pattern detection for common secret types
let log = "Token: ghp_1234567890123456789012345678901234567890";
let masked = masker.mask(log);
// Output: "Token: ghp_***"

Supported patterns:

  • GitHub Personal Access Tokens (ghp_*)
  • GitHub App tokens (ghs_*)
  • GitHub OAuth tokens (gho_*)
  • AWS Access Keys (AKIA*)
  • JWT tokens
  • Generic API keys

Encrypted Storage

For sensitive environments, use encrypted storage:

use wrkflw_secrets::storage::{EncryptedSecretStore, KeyDerivation};

// Create encrypted store
let (mut store, key) = EncryptedSecretStore::new()?;

// Add secrets
store.add_secret(&key, "API_KEY", "secret_value")?;

// Save to file
store.save_to_file("secrets.encrypted").await?;

// Load from file
let loaded_store = EncryptedSecretStore::load_from_file("secrets.encrypted").await?;
let secret = loaded_store.get_secret(&key, "API_KEY")?;

Error Handling

All operations return SecretResult<T> with comprehensive error types:

match manager.get_secret("MISSING_SECRET").await {
    Ok(secret) => println!("Secret: {}", secret.value()),
    Err(SecretError::NotFound { name }) => {
        eprintln!("Secret '{}' not found", name);
    }
    Err(SecretError::ProviderNotFound { provider }) => {
        eprintln!("Provider '{}' not configured", provider);
    }
    Err(SecretError::AuthenticationFailed { provider, reason }) => {
        eprintln!("Auth failed for {}: {}", provider, reason);
    }
    Err(e) => eprintln!("Error: {}", e),
}

Health Checks

Monitor provider health:

let health_results = manager.health_check().await;
for (provider, result) in health_results {
    match result {
        Ok(()) => println!("✓ {} is healthy", provider),
        Err(e) => println!("✗ {} failed: {}", provider, e),
    }
}

Security Best Practices

  1. Use encryption for secrets at rest
  2. Enable masking to prevent secrets in logs
  3. Rotate secrets regularly
  4. Use least privilege access for secret providers
  5. Monitor access through health checks and logging
  6. Use provider-specific authentication (IAM roles, service principals)
  7. Configure rate limiting to prevent abuse
  8. Validate input - the system automatically validates secret names and values

Rate Limiting

Protect against abuse with built-in rate limiting:

use wrkflw_secrets::rate_limit::RateLimitConfig;
use std::time::Duration;

let mut config = SecretConfig::default();
config.rate_limit = RateLimitConfig {
    max_requests: 100,                    // Max requests per window
    window_duration: Duration::from_secs(60), // 1 minute window
    enabled: true,
};

let manager = SecretManager::new(config).await?;

// Rate limiting is automatically applied to all secret access operations
match manager.get_secret("API_KEY").await {
    Ok(secret) => println!("Success: {}", secret.value()),
    Err(SecretError::RateLimitExceeded(msg)) => {
        println!("Rate limited: {}", msg);
    }
    Err(e) => println!("Other error: {}", e),
}

Input Validation

All inputs are automatically validated:

// Secret names must:
// - Be 1-255 characters long
// - Contain only letters, numbers, underscores, hyphens, and dots
// - Not start or end with dots
// - Not contain consecutive dots
// - Not be reserved system names

// Secret values must:
// - Be under 1MB in size
// - Not contain null bytes
// - Be valid UTF-8

// Invalid examples that will be rejected:
manager.get_secret("").await;                    // Empty name
manager.get_secret("invalid/name").await;        // Invalid characters
manager.get_secret(".hidden").await;             // Starts with dot
manager.get_secret("CON").await;                 // Reserved name

Performance Features

Caching

let config = SecretConfig {
    enable_caching: true,
    cache_ttl_seconds: 300, // 5 minutes
    ..Default::default()
};

Optimized Pattern Matching

  • Pre-compiled regex patterns for secret detection
  • Global pattern cache using OnceLock
  • Efficient string replacement algorithms
  • Cached mask generation

Benchmarking

Run performance benchmarks:

cargo bench -p wrkflw-secrets

Feature Flags

Enable optional providers:

[dependencies]
wrkflw-secrets = { version = "0.1", features = ["vault-provider", "aws-provider"] }

Available features:

  • env-provider (default)
  • file-provider (default)
  • vault-provider
  • aws-provider
  • azure-provider
  • gcp-provider
  • all-providers

License

MIT License - see LICENSE file for details.