Improvements for bash associative arrays#1797
Improvements for bash associative arrays#1797IsaacCalligeros95 wants to merge 9 commits intomainfrom
Conversation
️✅ There are no secrets present in this pull request anymore.If these secrets were true positive and are still valid, we highly recommend you to revoke them. 🦉 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. |
False positive |
…ture toggle enabled
Optimize
octopus_parameterspopulation with lazy loading and batch base64 decodeProblem
The
BashParametersArrayFeatureToggleimplementation had multiple issues:xxd— not in the documented dependency list, not universally available, and the feature silently did nothing when missingecho -n "$large_hex_string" | xxd -r -pruns as a pipe-backed process substitution; if theread -Nloop exits early,xxdreceives SIGPIPE and the bootstrap script can exit non-zerooctopus_parameterswas populated eagerly even when scripts never accessed it, adding ~6-7 seconds of parse time for large deployments (20,000+ variables)An earlier attempt at using base64 was abandoned because it called
openssl(orbase64) once per variable — O(N) subprocess forks — which made ~3,000 variables take ~60 seconds.Solution
1. Lazy Loading (Performance)
The bootstrapper now analyzes the user script to detect if it references
octopus_parameters:The check uses regex to match:
${octopus_parameters[...]},"${octopus_parameters[@]}"for ... in "${!octopus_parameters[@]}"Scripts that only use
get_octopusvariable(which uses the case statement, not the array) skip the expensive array population. This saves ~6-7 seconds for deployments with 20,000+ variables.Configuration file impact:
octopus_parameters: ~21 MB (includes both KVP data + case statement)The KVP data is only included in the encrypted blob when the script actually uses
octopus_parameters, significantly reducing the configuration file size for scripts that don't need it.2. Batch Base64 Decode (Compatibility + Performance)
Switch the wire format inside the encrypted blob from
hex(key)$hex(value)tobase64(key)$base64(value), and batch-decode using exactly twobase64 -dcalls regardless of variable count:exec 3< <(printf '%s\n' "${b64_keys[@]}" | base64 -d)exec 4< <(printf '%s\n' "${b64_values[@]}" | base64 -d)LC_ALL=C read -r -Nto slice from each fd using pre-calculated byte lengthsThis preserves the O(1) subprocess count of the original hex+xxd approach. Process substitutions are used rather than temp files to keep decoded sensitive values (passwords, keys, certs) out of the filesystem entirely — data flows through kernel pipe buffers only. Deadlock is avoided because the read loop alternates between fd3 and fd4 on every iteration, draining both pipes continuously.
LC_ALL=Con thereadcalls ensures multi-byte UTF-8 (including emoji) round-trips correctly by counting bytes rather than characters.The C# change is minimal:
GetEncryptedVariablesKvpuses the existingEncodeValuemethod (already used for the case-statement path) instead ofEncodeAsHex. Thexxdavailability check in the feature gate is removed.Performance Impact
20,000 variables:
octopus_parameters: ~7.5 seconds (parse required)octopus_parameters: ~1.7 seconds (lazy loading skips parse)Note: For much larger variable sets (20K+ variables / ~15 MB), the runtime of 33 seconds remains too slow for this to be feasible for all deployments. See test results for further details. The lazy loading optimization ensures this overhead is only paid when
octopus_parametersis actually used.Testing
Added
BashPerformanceFixture.cs(in previous commits) with two comprehensive tests:ShouldPopulateOctopusParametersPerformantly— validates parse performance at various scales (100, 500, 1K, 5K, 20K variables) with realistic payload distributionsShouldNotLoadOctopusParametersWhenNotUsed— verifies lazy loading by checking configuration file markers remain unreplaced and timing confirms array isn't populatedTest variables mirror real Octopus deployments:
Risks
Process substitution approach (current)
base64 -druns in a subshell inside<(...);its exit code isn't directly observable, making malformed input harder to detect.
producer gets
SIGPIPE, which the shell handles silently and could mask a bug.Mitigations
get_octopusvariable) remains unaffectedChanges
Bootstrap.sh— newdecrypt_and_parse_variablesfunction, removesxxdgate, adds_ensure_octopus_parameters_loadedwith lazy loading logicBashScriptBootstrapper.cs—GetEncryptedVariablesKvpswitches fromEncodeAsHextoEncodeValue;ScriptUsesOctopusParametersanalyzes script for array usage; markers only replaced when neededBashPerformanceFixture.cs— new dedicated fixture with comprehensive performance tests using realistic variable distributions (60% small / 25% medium / 10% large JSON blobs / 5% PEM cert bundles)BashFixture.cs— existing functional tests preserved;Performance Test Results - from previous commits.
Original changes context