Proper subcommands silently shadow colliding global options before the subcommand
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 23.1k
- Forks
- 1.1k
- Avg merge
- 1d 11h
- Merged PRs (30d)
- 9
Description
Summary
Proper subcommands silently ignore a colliding global option when it appears before the subcommand. For example, sops --output-type json decrypt encrypted.yaml exits 0 but writes YAML. The same option after decrypt, and the legacy -d form, produce JSON.
This is unsafe for automation because a successful exit status can falsely attest to a requested output format.
Minimal synthetic reproduction
age-keygen -o key.txt
recipient=$(age-keygen -y key.txt)
printf 'apiVersion: v1\nkind: Synthetic\nmetadata:\n name: parsing-fixture\nspec:\n enabled: true\n count: 42\n' > plaintext.yaml
sops encrypt --age "$recipient" --output encrypted.yaml plaintext.yaml
SOPS_AGE_KEY_FILE=key.txt sops --output-type json decrypt encrypted.yaml > bad.out
echo $? # 0
cat bad.out # YAML, not JSON
jq . < bad.out # parse error
bad.out is:
apiVersion: v1
kind: Synthetic
metadata:
name: parsing-fixture
spec:
enabled: true
count: 42
Control cases:
SOPS_AGE_KEY_FILE=key.txt sops decrypt --output-type json encrypted.yaml | jq .
SOPS_AGE_KEY_FILE=key.txt sops -d --output-type json encrypted.yaml | jq .
Both controls exit 0 and emit valid JSON. No non-synthetic key or secret is involved.
Expected result
The ambiguous form must not silently succeed with a different output format. Either honor the option consistently or reject the command before producing output with a nonzero status. A rejection is the least surprising compatibility-safe behavior because the proper subcommand already advertises its own option.
Actual result
The global parser consumes --output-type; the decrypt action reads its local c.String("output-type"), finds the local default, and falls back to the YAML extension. It returns success.
Version matrix
| SOPS version | Platform | sops --output-type json decrypt encrypted.yaml |
|---|---|---|
| 3.8.1 | macOS arm64 | rejected (proper subcommand not yet available) |
| 3.9.0 | macOS arm64 | exit 0, YAML |
| 3.9.1 | macOS arm64 | exit 0, YAML |
| 3.9.4 | macOS arm64 and Debian 13 amd64 | exit 0, YAML |
| 3.10.0 | macOS arm64 | exit 0, YAML |
| 3.13.2 | macOS arm64 | exit 0, YAML |
The behavior begins with the proper-subcommand migration in #1333 / #1391 (released in 3.9.0).
Scope
The same global-before-subcommand shadowing class was reproduced for decrypt with --input-type, --output, --extract, and --ignore-mac, and for analogous encrypt and rotate input/output options. This is therefore not only an output-format issue.
Related but distinct reports:
- #747 concerns options placed after the filename. This report concerns an option advertised as global, placed before the proper subcommand and before the filename.
- #598 also concerns argument order after a filename.
- #1259 concerns legacy
-dordering.
Proposed fix
Reject a global option when a proper subcommand declares the same option and that command-local option was not set. This preserves valid local placement and prevents a successful command from silently applying a different behavior. A draft PR implementing that guard and black-box regression tests will be linked here.
Automation impact
Format-sensitive preflight pipelines can treat exit status 0 as proof of JSON and then pass YAML to a JSON parser. The command fails later and less clearly, or can make a recovery/preflight decision from the wrong representation. Fail-closed CLI parsing is important where decrypted output is machine-consumed.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by reproducing the global-before-subcommand cases for decrypt, encrypt, and rotate, then trace the proper-subcommand option parsing and how command-local defaults are read. Add black-box regression coverage for colliding input/output options, and verify that ambiguous placement rejects with a nonzero status while valid local placement still succeeds.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- cli, security
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 55/100