feat: make populate_dotenv_missing_secrets_stubs require absolute paths and non-destructive

[aaronsteers]

feat: make populate_dotenv_missing_secrets_stubs require absolute paths and non-destructive

Summary

This PR updates the populate_dotenv_missing_secrets_stubs function to address two critical issues:

1. Absolute path requirement: The function now validates that the provided dotenv_path is absolute using Path.is_absolute() and rejects relative paths with a clear error message. This prevents ambiguity about the caller's working directory context.

2. Non-destructive behavior: The function now checks for existing secrets before writing and throws an error if any collisions are detected, rather than silently overwriting existing values. When collisions occur, it returns a detailed error message listing all existing secrets and their set/unset status.

Key Changes

• Added absolute path validation at function start
• Implemented collision detection using dotenv_values() to read existing secrets
• Enhanced error responses to include existing secrets information following the SecretInfo model pattern
• Updated function documentation to clarify the new non-destructive behavior
• Updated all existing tests to use absolute paths (breaking change)
• Added comprehensive test coverage for new validation and collision detection features

Review & Testing Checklist for Human

⚠️ This is a breaking API change - all callers must now provide absolute paths

• Verify absolute path validation works correctly across different platforms and edge cases (test with various relative path formats)
• Test collision detection logic with different dotenv file contents (empty values, comment stubs, real secrets, malformed entries)
• Confirm backward compatibility - verify that existing integrations can handle the new absolute path requirement
• Validate error message format - ensure the collision error response format is consumable by calling code and matches expected patterns
• Test edge cases - empty files, permission errors, malformed dotenv files, concurrent access scenarios

Recommended test plan: Create a test dotenv file with mixed content (real secrets, empty values, comment stubs) and verify the function correctly identifies collisions and preserves existing content.

---

Diagram

%%{ init : { "theme" : "default" }}%%
graph TB
    subgraph Legend
        L1["Major Edit"]:::major-edit
        L2["Minor Edit"]:::minor-edit  
        L3["Context/No Edit"]:::context
    end

    A["connector_builder_mcp/_secrets.py"]:::major-edit
    B["tests/test_secrets.py"]:::major-edit
    C["populate_dotenv_missing_secrets_stubs()"]:::major-edit
    D["list_dotenv_secrets()"]:::context
    E["SecretInfo model"]:::context
    F["dotenv_values()"]:::context

    A --> C
    A --> D
    A --> E
    B --> C
    C --> F
    D --> E
    C --> E

    classDef major-edit fill:#90EE90
    classDef minor-edit fill:#ADD8E6  
    classDef context fill:#FFFFFF

Loading

Notes

• The collision detection logic reuses the existing dotenv_values() pattern from list_dotenv_secrets() for consistency
• Secret "set" status determination includes checking if values are non-empty, non-whitespace, and don't start with "#" (comment stubs)
• All 66 tests pass including the new test cases for absolute path validation and collision detection
• This change was requested by AJ Steers (@aaronsteers) to improve the security and reliability of the secrets management functionality

Link to Devin run: https://app.devin.ai/sessions/a1ddcae2e2cc41dd9da9121d4b034e32
Requested by: @aaronsteers

Important

Auto-merge enabled.

This PR is set to merge automatically when all requirements are met.

[devin-ai-integration]

Original prompt from AJ Steers

@Devin - Update the Connector Builder MCP server to always require abolute paths when using 'populate_dotenv_missing_secrets_stubs'. Also, make that tool non-desctructive (currently it overwrites secrets if they already exist). Instead, throw an error if one or more of the named secret already exists, so the LLM is informed that they are trying to create stubs for a secret that is already set. In the case that a collision occurs, also return the list of secrets already in the file and whether they are set or not (following behavior of the similar tool for listing secrets in the dotenv file).

[devin-ai-integration]

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

• Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
• Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

• Disable automatic comment and CI monitoring

[github-actions]

👋 Greetings, Airbyte Team Member!

Here are some helpful tips and reminders for your convenience.

Testing This Branch via MCP

To test the changes in this specific branch with an MCP client like Claude Desktop, use the following configuration:

{
  "mcpServers": {
    "connector-builder-mcp-dev": {
      "command": "uvx",
      "args": ["--from", "git+https://github.com/airbytehq/connector-builder-mcp.git@devin/1754931923-fix-populate-dotenv-absolute-paths-non-destructive", "connector-builder-mcp"]
    }
  }
}

Testing This Branch via CLI

You can test this version of the MCP Server using the following CLI snippet:

# Run the CLI from this branch:
uvx 'git+https://github.com/airbytehq/connector-builder-mcp.git@devin/1754931923-fix-populate-dotenv-absolute-paths-non-destructive#egg=airbyte-connector-builder-mcp' --help

PR Slash Commands

Airbyte Maintainers can execute the following slash commands on your PR:

• /autofix - Fixes most formatting and linting issues
• /poe <command> - Runs any poe command in the uv virtual environment

📝 Edit this welcome message.

[Copilot]

Pull Request Overview

This PR enhances the populate_dotenv_missing_secrets_stubs function with two critical security and reliability improvements: requiring absolute paths to eliminate working directory ambiguity, and implementing non-destructive behavior to prevent accidental overwriting of existing secrets.

• Added absolute path validation using Path.is_absolute() with clear error messages for relative paths
• Implemented collision detection that checks for existing secrets and provides detailed error information when conflicts are found
• Enhanced error responses to include comprehensive existing secrets status using the SecretInfo model pattern

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
connector_builder_mcp/_secrets.py	Added absolute path validation and collision detection logic to prevent destructive operations
tests/test_secrets.py	Updated all test cases to use absolute paths and added comprehensive test coverage for new validation features

[github-actions]

PyTest Results (Full)

0 tests  ±0   0 ✅ ±0   0s ⏱️ ±0s
0 suites ±0   0 💤 ±0 
0 files   ±0   0 ❌ ±0 

Results for commit a94dd79. ± Comparison against base commit 604a482.

♻️ This comment has been updated with latest results.

[github-actions]

PyTest Results (Fast)

0 tests  ±0   0 ✅ ±0   0s ⏱️ ±0s
0 suites ±0   0 💤 ±0 
0 files   ±0   0 ❌ ±0 

Results for commit a94dd79. ± Comparison against base commit 604a482.

♻️ This comment has been updated with latest results.