Skip to content

refactor: parse .vig-os config as data#287

Merged
c-vigo merged 5 commits intorelease/0.3.0from
bugfix/285-parse-vig-os-config-safely
Mar 13, 2026
Merged

refactor: parse .vig-os config as data#287
c-vigo merged 5 commits intorelease/0.3.0from
bugfix/285-parse-vig-os-config-safely

Conversation

@c-vigo
Copy link
Contributor

@c-vigo c-vigo commented Mar 13, 2026

Description

Replaces executable .vig-os loading with data-only parsing in initialize.sh and version-check.sh so unexpected shell content cannot execute.

Adds regression integration coverage proving shell payloads in .vig-os are not executed while DEVCONTAINER_VERSION is still read and used.

Includes a follow-up test hardening commit to restore .vig-os after mutation-based tests so later integration tests are not impacted by test-side config changes.

Adds a final test stabilization commit so IN_CONTAINER=true hook-path BATS checks are deterministic and no longer depend on host hook return behavior.

Type of Change

  • feat -- New feature
  • fix -- Bug fix
  • docs -- Documentation only
  • chore -- Maintenance task (deps, config, etc.)
  • refactor -- Code restructuring (no behavior change)
  • test -- Adding or updating tests
  • ci -- CI/CD pipeline changes
  • build -- Build system or dependency changes
  • revert -- Reverts a previous commit
  • style -- Code style (formatting, whitespace)

Modifiers

  • Breaking change (!) -- This change breaks backward compatibility

Changes Made

  • assets/workspace/.devcontainer/scripts/initialize.sh
    • Replaced source "$config_file" in load_vig_os_config with line-by-line key/value parsing for DEVCONTAINER_VERSION
    • Preserved existing .env update behavior and Darwin/Linux sed handling
  • assets/workspace/.devcontainer/scripts/version-check.sh
    • Replaced source "$config_file" in get_current_version with data-only parsing for DEVCONTAINER_VERSION
    • Preserved existing pinned-version filtering (dev, latest, empty)
  • tests/test_integration.py
    • Added regression test for initialize.sh to ensure shell payloads in .vig-os are not executed
    • Added regression test for version-check.sh config to ensure shell payloads in .vig-os are not executed
    • Added restoration of .vig-os after mutation tests to prevent side effects on later tests
  • tests/bats/githooks.bats
    • Made IN_CONTAINER=true guard tests deterministic for pre-commit, prepare-commit-msg, and commit-msg
    • Adjusted expectations so tests validate guard behavior without flaky exit-code assumptions

Changelog Entry

No changelog needed. Issue #285 explicitly marks changelog category as "No changelog needed", and this PR keeps behavior intact while hardening implementation details.

Testing

  • Tests pass locally (just test)
  • Manual testing performed (describe below)

Manual Testing Details

Checklist

  • My code follows the project's style guidelines
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have updated the documentation accordingly (edit docs/templates/, then run just docs)
  • I have updated CHANGELOG.md in the [Unreleased] section (and pasted the entry above)
  • My changes generate no new warnings or errors
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published

Additional Notes

Issue references a security hardening concern flagged during smoke-test review; this PR keeps scope limited to the two script functions and corresponding tests.

Refs: #285

@c-vigo c-vigo self-assigned this Mar 13, 2026
@c-vigo c-vigo requested a review from Copilot March 13, 2026 09:39
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens devcontainer scripts against command execution by treating the root .vig-os file as data (parsing DEVCONTAINER_VERSION directly) instead of sourcing it as shell code, and adds regression tests to prevent reintroducing the issue.

Changes:

  • Update initialize.sh and version-check.sh to parse DEVCONTAINER_VERSION from .vig-os without source.
  • Add integration tests ensuring .vig-os shell content is not executed by initialization or version-check config output.
  • Make githook BATS tests more deterministic by running hooks with a restricted PATH and explicit /bin/bash.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
tests/test_integration.py Adds regression tests ensuring .vig-os is treated as non-executable data.
tests/bats/githooks.bats Adjusts hook invocation to be less dependent on environment PATH.
assets/workspace/.devcontainer/scripts/version-check.sh Replaces source .vig-os with safe line-based parsing for DEVCONTAINER_VERSION.
assets/workspace/.devcontainer/scripts/initialize.sh Replaces source .vig-os with safe line-based parsing before writing .env.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@c-vigo c-vigo merged commit bf0cab0 into release/0.3.0 Mar 13, 2026
12 checks passed
@c-vigo c-vigo deleted the bugfix/285-parse-vig-os-config-safely branch March 13, 2026 10:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants