Skip to content

🛡️ Sentinel: Security Fixes for Publish Flow - #115

Closed
calionauta wants to merge 547 commits into
mainfrom
fix/security-enhancements-publish-8705118332549494391
Closed

🛡️ Sentinel: Security Fixes for Publish Flow#115
calionauta wants to merge 547 commits into
mainfrom
fix/security-enhancements-publish-8705118332549494391

Conversation

@calionauta

Copy link
Copy Markdown
Owner

Identified and fixed two security issues in the publish flow:

  1. Brittle File Exclusion: The previous implementation of _ignore_func only handled a narrow set of patterns, leading to potential data leakage of sensitive directories like sessions or cache. Replaced it with a robust fnmatch.filter based approach.
  2. Regex Newline Injection: The _is_valid_skill_name function used the $ anchor, which matches trailing newlines in Python. This could be bypassed to inject arguments if the name was used in a shell context. Updated to use \Z.

All 566 tests passed.


PR created automatically by Jules for task 8705118332549494391 started by @renatocaliari

calionauta added 30 commits May 15, 2026 09:54
Result: {"status":"keep","metric":243,"agent_cards":7,"feature_cards":4,"loc":1225,"tests_passed":243,"total_tests":243}
- Rewrote DotAgents section: user benefits instead of compliance comparison
- Added GitAgent Protocol section inline in README (removed separate doc)
- Removed docs/gitagent.md (redundant)
- Protocol sections now focus on what user gets, not implementation details
- Added 9 integration tests for git operations
- Added 6 protocol config tests
- 258 tests passing
- Material for MkDocs theme (beautiful, modern)
- Auto-generate CLI docs from Click decorators via mkdocs-click
- GitHub Actions workflow for automatic deployment on push
- Organized docs structure (agents/, protocols/, examples/)
- Dark mode, instant navigation, search, code copy
- By cali (Renato Caliari)
- Restored index.html (main landing page)
- Added unified deploy workflow
- Docs at /docs (MkDocs generated)
- By cali (Renato Caliari)
- Created scripts/generate_cli_docs.py that extracts command info from Click
- Updated GitHub Actions to run script before building docs
- Generates complete CLI reference with options and subcommands
- Works with nested command groups (ExtendedHelpGroup)
- By cali (Renato Caliari)
- Added docs/protocols/gitagent.md with complete GitAgent info
- Added GitAgent to navigation (between DotAgents and comparison)
- Workflow guaranteed: index.html → root, docs → /docs
- By cali (Renato Caliari)
- Workflow: index.html → root, site_docs/* → /docs/ (except index.html)
- Docs index renamed to home.html to avoid conflict
- Removed dotagents-comparison.md (not useful for users)
- By cali (Renato Caliari)
- docs.yml was deploying MkDocs to root (wrong!)
- deploy.yml correctly puts index.html → root, docs → /docs
- By cali (Renato Caliari)
- mkdir -p _site comes before cp
- Added verification steps for debugging
- By cali (Renato Caliari)
- Copy entire site_docs folder to _site/docs
- Remove docs/index.html to avoid conflict with root index
- Simpler approach that should work
- By cali (Renato Caliari)
- SkillsDiff imports Config internally, so patch target must be skills_diff
- Also simplified deploy workflow for docs
- By cali (Renato Caliari)
- SkillsDiff imports Config inside __init__, so can't patch at module level
- Test now uses tmp_path and monkeypatch to test without repo configured
- By cali (Renato Caliari)
- Test now checks for 'Skills Divergence Report' instead of 'No repository'
- Command actually works and shows local skills even without repo
- By cali (Renato Caliari)
- Some runners have repo configured, others don't
- Test should accept either output
- By cali (Renato Caliari)
- Root has index.html (main site)
- /docs has everything (MkDocs site, including its own index.html)
- Simple cp -r approach
- By cali (Renato Caliari)
Three-stage pipeline:
1. test - Runs tests first (must pass)
2. build-docs - Builds and validates docs structure
3. deploy - Verifies structure before upload

Key validations:
- Both root index.html AND docs/index.html must exist
- All required doc pages must be present
- Tests must pass before anything deploys

Also added scripts/validate-deploy.sh for local validation.
- By cali (Renato Caliari)
- build-docs uploads as 'site-docs'
- deploy downloads 'site-docs' and creates 'github-pages'
- This avoids the 'multiple artifacts' error
- By cali (Renato Caliari)
… command

- Add skip_security_panel and skip_confirm params to publish_skills()
- Unified publish command now shows security panel once before execution
- Skills and agents publishing no longer re-show the same warning
- Skills now use the same security_scanner as agents
- Skills are scanned for: API keys, tokens, private URLs, absolute paths
- Files with dangerous names (auth, token, key, secret) are skipped
- Flagged files are published with warning (same behavior as agents)
- Unified security message in CLI publish command

See: https://github.com/renatocaliari/agent-sync-public
…epos

- Add templates/README_public_repo.md and templates/README_skills.md
- Publish now creates proper README files automatically
- Templates use {username}, {repo_name}, {full_repo_name} placeholders
- Removes old generate_readme() (was basic, replaced by templates)
- Fixes duplicate imports in publish.py
- Skills now show detailed issues (like agents) in Phase 3b
- Count flagged files before confirm prompt
- Warn user about flagged files before publishing
- Fix MarkupError with [bold] instead of [green]
- Fix outdated reference to agent-sync-configs → agent-sync-private
- Add _interactive_flagged_selection() helper (DRY for skills and agents)
- Add _render_flagged_table() helper for consistent display
- Unified selection flow: user picks which flagged items to publish
- Clean up duplicate _git_push function in publish.py
- Update security warning message to reflect interactive selection
- Remove redundant confirmation when flagged items already selected
Tests added:
- TestRenderFlaggedTable (3 tests)
- TestReadmeGeneration (4 tests)
- TestInteractiveFlaggedSelection (1 passing, 2 need more mocking)
- Integration tests for --skills, --agents, and --all flags

Passing tests: 8/17
CLI integration tests show interactive selection working correctly
Result: {"status":"keep","metric":243,"agent_cards":0,"feature_cards":0,"loc":0,"tests_passed":44,"total_tests":44}
Fixed UnboundLocalError in publish command when do_agents=True but do_skills=False.
The skills_flagged variable was only initialized inside the do_skills block.
- Fixed test assertions to match new unified security warning
- Added _interactive_flagged_selection mock to all tests
- 45 tests passing across publish, security, validators
- test_publish_cli.py: 9 tests for unified publish command
- test_publish.py: 5 tests for publish functionality
- test_publish_interactive.py: 17 tests for interactive selection
- test_security_scanner.py: 24 tests for security patterns
- test_validators.py: 7 tests for URL/repo validation
- Fixed UnboundLocalError bug in cli.py
- Total: 257+ tests passing
calionauta and others added 25 commits June 6, 2026 15:24
…/filter (F5-F8)

F2: Cleaned 'README' and 'README.md' from RETIRED.md manifest
  (73 entries remain — removed 2 false positives from initial migration)

F3: Added integration test for hub-overrides-repo conflict (test_05_hub)
F4: Added integration test for push-after-manifest-retirement (test_06_retire)
F5: audit --limit N / --filter pattern
  - JSON and human-readable paths both filtered
  - 2 CLI smoke tests added
F6: push --strict (CI-friendly exit code 2 when orphans detected)
  - 1 CLI smoke test added
F7: Integration test: push then pull restores hub skills + manifest
  - Fixed _sync_from_repo to copy files (not just dirs) so RETIRED.md
    is restored on a fresh machine
F8: Extracted magic numbers: paths.GIT_TIMEOUT=10, paths.LOCK_STALE_TIMEOUT=600
  - Replaced 6 occurrences in sync.py, skills.py, skills_audit.py
  - Replaced STALE_TIMEOUT in skills.py with paths.LOCK_STALE_TIMEOUT

Total: 574 tests (was 568), 11 integration tests (was 8).
The RETIRED.md manifest was overengineered. Retirement is now derived
from git history: commit 0501f19
Author: renatocaliari <renatoac82@gmail.com>
Date:   Fri May 22 07:44:14 2026 -0300

    chore: add analysis files and temp patterns to .gitignore

    - Exclude publish-investigation.md, *-investigation.md, incident-analysis.md
    - Exclude *.jsonl, temp_*.py, debug_*.md
    - Remove publish-investigation.md from version control

commit abb9bc5
Author: renatocaliari <renatoac82@gmail.com>
Date:   Thu May 21 12:46:52 2026 -0300

    refactor: clean test suite and add e2e tests

    Removed dead/bad tests:
    - test_integration_git.py (tests Git, not this tool)
    - test_publish_readme.py (all tests skipped)
    - test_publish_runner.py (thin wrappers, outdated)

    Refactored:
    - test_push_command.py - signature contract tests with docs
    - test_pull_conflicts.py - updated to test _clone_to_repo helper

    Added:
    - test_sync_e2e.py - end-to-end tests for push/pull/sync with real git

    Added _clone_to_repo() helper for DRY (used by link_repo and init_repo)

    376 tests passing, 2 skipped (git push needs remote setup)

commit c0b3d0a87bdacd22174f9dfff03789866e693726
Author: renatocaliari <renatoac82@gmail.com>
Date:   Thu May 21 08:45:44 2026 -0300

    pi rewind store

commit abf1b63
Author: renatocaliari <renatoac82@gmail.com>
Date:   Wed May 20 12:48:14 2026 -0300

    cleanup: delete publish.py (moved to publish/__init__.py)

    publish.py (984 lines) was a thin wrapper - deleted it since all exports
    are now in publish/__init__.py. Added has_significant_issues() to __init__.py.
    cli.py imports from .publish (the package) still work.

    Also removed skills/_scanner.py failed refactor attempt (reverted).

commit bca4edd
Author: renatocaliari <renatoac82@gmail.com>
Date:   Tue May 19 21:01:31 2026 -0300

    fix: replace sync_manager.is_initialized() with config.repo_url check

    Also deleted deprecated test_publish_cli.py (was already skipped)

commit d8e938e
Author: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
Date:   Tue May 19 23:43:23 2026 +0000

    🛡️ Sentinel: [security improvement] Secure editor execution

    - Added `validate_editor` to `src/agent_sync/validators.py`.
    - Hardened the `edit` command in `src/agent_sync/cli.py` to sanitize `VISUAL`/`EDITOR` variables.
    - Switched to `shlex.split()` for safe command execution with flags.
    - Added comprehensive tests in `tests/test_security_editor.py`.

    Co-authored-by: renatocaliari <1719651+renatocaliari@users.noreply.github.com>

commit 176a5db
Author: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
Date:   Tue May 19 23:41:50 2026 +0000

    🛡️ Sentinel: [security improvement] Hardened file and directory permissions

    - Reverted redundant path traversal fix (already in main).
    - Implemented restricted file (0o600) and directory (0o700) permissions for sensitive data.
    - Added centralized security utilities in `src/agent_sync/security.py`.
    - Added security regression tests in `tests/test_security_permissions.py`.

    Co-authored-by: renatocaliari <1719651+renatocaliari@users.noreply.github.com>

commit bb96b53
Author: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
Date:   Tue May 19 23:41:30 2026 +0000

    🛡️ Sentinel: [CRITICAL] Fix Path Traversal in extension restoration

    This fix addresses a critical path traversal vulnerability in the extension skills restoration logic.
    A malicious manifest in a repository could previously overwrite arbitrary files on the local system by using traversal sequences in the 'extension_dir' or 'ext_name' fields.

    - Validates extension names using validate_skill_name.
    - Ensures resolved destination paths are within the agent's configuration directory using is_relative_to.
    - Added regression tests in tests/test_security_traversal.py.

    Co-authored-by: renatocaliari <1719651+renatocaliari@users.noreply.github.com>

commit b2c6809
Author: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
Date:   Tue May 19 23:36:22 2026 +0000

    🛡️ Sentinel: [security improvement] Fix regex bypass and add skill validation (acknowledged redundant)

    - Hardened repository and GitHub URL validation by replacing `$` with `\Z` in regex patterns.
    - Implemented `validate_skill_name` to prevent path traversal.
    - Applied skill name validation in `SkillsDeleter.delete_skills`.
    - Added security tests.

    Acknowledged PR comment that these fixes may have been incorporated in other merges.

    Co-authored-by: renatocaliari <1719651+renatocaliari@users.noreply.github.com>

commit 3882015
Author: renatocaliari <renatoac82@gmail.com>
Date:   Tue May 19 20:00:25 2026 -0300

    chore: remove test cache directory from repo

commit 30c8df0
Author: renatocaliari <renatoac82@gmail.com>
Date:   Tue May 19 19:51:17 2026 -0300

    Merge feature/external-skill-sources: step-by-step publish flow with source management

    Features:
    - Step-by-step publish flow: LOCAL → EXTERNAL → AGENTS sequentially
    - Source management: --list-sources, --add-source, --remove-source
    - last_success tracking for external sources
    - Unified TUI for skills and agents
    - Updated labels (LOCAL, full repo name, AGENTS)

    Refactoring:
    - Extract models.py, discovery.py, git_publish.py, setup.py
    - runner.py as thin wrapper
    - 330 passing tests

commit c16cd2e
Author: renatocaliari <renatoac82@gmail.com>
Date:   Fri May 15 20:27:07 2026 -0300

    fix(publish): skip binary files (.pyc, images, etc) from scanning and publishing

    - Add SKIP_EXTENSIONS set in publish.py to filter binary/incompatible files
    - Skip __pycache__ directories during publish
    - scan_file() now returns safe=True for binary files (prevents .pyc false positives)
    - Remove redundant .pyc detection from flagged items display

commit 175391b
Author: renatocaliari <renatoac82@gmail.com>
Date:   Fri May 15 18:45:46 2026 -0300

    Stable - 273 tests passing

    Result: {"status":"keep","metric":243,"agent_cards":3,"feature_cards":14,"loc":2923,"tests_passed":273,"total_tests":273}

commit 118b7a4
Author: renatocaliari <renatoac82@gmail.com>
Date:   Fri May 15 14:58:29 2026 -0300

    fix: simplify diff test - check for divergence report in output

    - Test now checks for 'Skills Divergence Report' instead of 'No repository'
    - Command actually works and shows local skills even without repo
    - By cali (Renato Caliari)

commit 480d16b
Author: renatocaliari <renatoac82@gmail.com>
Date:   Fri May 15 12:04:16 2026 -0300

    fix: remove duplicate docs.yml workflow, use deploy.yml only

    - docs.yml was deploying MkDocs to root (wrong!)
    - deploy.yml correctly puts index.html → root, docs → /docs
    - By cali (Renato Caliari)

commit d3760ea
Author: renatocaliari <renatoac82@gmail.com>
Date:   Fri May 15 11:58:22 2026 -0300

    fix: ensure main site at root + docs at /docs, remove comparison doc

    - Workflow: index.html → root, site_docs/* → /docs/ (except index.html)
    - Docs index renamed to home.html to avoid conflict
    - Removed dotagents-comparison.md (not useful for users)
    - By cali (Renato Caliari)

commit ccf2a4f
Author: renatocaliari <renatoac82@gmail.com>
Date:   Fri May 15 11:20:56 2026 -0300

    chore: remove old pages workflow, docs now via mkdocs

commit 094ec9d
Author: renatocaliari <renatoac82@gmail.com>
Date:   Fri May 15 11:20:00 2026 -0300

    feat: add MkDocs documentation with auto-generated CLI reference

    - Material for MkDocs theme (beautiful, modern)
    - Auto-generate CLI docs from Click decorators via mkdocs-click
    - GitHub Actions workflow for automatic deployment on push
    - Organized docs structure (agents/, protocols/, examples/)
    - Dark mode, instant navigation, search, code copy
    - By cali (Renato Caliari)

commit db8932f
Author: renatocaliari <renatoac82@gmail.com>
Date:   Fri May 15 10:29:02 2026 -0300

    refactor: simplify protocol docs - focus on user benefits

    - Rewrote DotAgents section: user benefits instead of compliance comparison
    - Added GitAgent Protocol section inline in README (removed separate doc)
    - Removed docs/gitagent.md (redundant)
    - Protocol sections now focus on what user gets, not implementation details
    - Added 9 integration tests for git operations
    - Added 6 protocol config tests
    - 258 tests passing

commit 580dc1e
Author: renatocaliari <renatoac82@gmail.com>
Date:   Thu May 14 19:34:54 2026 -0300

    chore: stop tracking docs/ subdirectories, add to .gitignore

    docs/ should only contain root-level files, not subdirectories with dates/plans

commit afa0f12
Author: renatocaliari <renatoac82@gmail.com>
Date:   Thu May 14 19:31:51 2026 -0300

    chore: clean up autoresearch artifacts and obsolete docs

    - Add comprehensive .gitignore with Python + Autoresearch patterns
    - Remove autoresearch.* files (md, ideas, sh, jsonl)
    - Remove docs/2026-05-14/ (executed plans)
    - Add examples/overrides.yaml for machine-specific config
    - Add src/agent_sync/secrets.py + tests for secret handling

commit 662bcda
Author: renatocaliari <renatoac82@gmail.com>
Date:   Thu May 14 09:54:26 2026 -0300

    chore: stop tracking autoresearch.jsonl (already gitignored)

commit 12e578b
Author: renatocaliari <renatoac82@gmail.com>
Date:   Thu May 14 08:47:35 2026 -0300

    DRY: Extract _step_title() helper in setup wizard — replaces 7 Panel.fit(...) boilerplate blocks in _step_* methods with a single helper. Also remove dead transforms.py. LOC -36.

    Result: {"status":"keep","exit_code":0,"tests_passed":87,"total_tests":87,"loc":5906}

commit 8067bcb
Author: renatocaliari <renatoac82@gmail.com>
Date:   Thu May 14 08:28:34 2026 -0300

    Consolidate security tests: merge test_security_harden.py into test_security.py, delete separate file. LOC unchanged, one fewer test file.

    Result: {"status":"keep","exit_code":0,"tests_passed":93,"total_tests":93,"loc":6278}

commit bc97007
Author: renatocaliari <renatoac82@gmail.com>
Date:   Thu May 14 08:21:06 2026 -0300

    Remove dead code: unused functions in agents/__init__.py, transforms.py, registry_loader.py, config.py, agent handlers (roocode, cline, cursor, windsurf). Fix 3 failing publish tests (add timeout to assertions). Remove obsolete test_transforms.py. LOC -473.

    Result: {"status":"keep","exit_code":0,"tests_passed":94,"total_tests":94,"loc":6374}

commit fb5e58c
Author: renatocaliari <renatoac82@gmail.com>
Date:   Fri Mar 6 20:26:08 2026 -0300

    fix: address code review feedback on validators

    - Remove test_issue.py (temporary test file)
    - Remove .jules/sentinel.md (internal tracking file)
    - Fix regex to reject repo names starting with period (.)
    - Add test case for names starting with period
    - GitHub doesn't allow repo names starting with '.' or '-'

    Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

commit 39f7344
Author: renatocaliari <renatoac82@gmail.com>
Date:   Fri Mar 6 20:26:08 2026 -0300

    fix: address code review feedback on validators

    - Remove test_issue.py (temporary test file)
    - Remove .jules/sentinel.md (internal tracking file)
    - Fix regex to reject repo names starting with period (.)
    - Add test case for names starting with period
    - GitHub doesn't allow repo names starting with '.' or '-'

    Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

commit 166d9e1
Author: renatocaliari <renatoac82@gmail.com>
Date:   Thu Mar 5 18:47:20 2026 -0300

    docs: Update README with new repo naming convention

    - agent-sync-configs → agent-sync-private-configs ✅
    - my-agent-skills → agent-sync-public-skills ✅
    - Update examples to use new standard names

    Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

commit a1f7702
Author: renatocaliari <renatoac82@gmail.com>
Date:   Thu Mar 5 14:14:06 2026 -0300

    chore: remove temporary build/plan files

commit ed472b1
Author: renatocaliari <renatoac82@gmail.com>
Date:   Thu Mar 5 14:14:01 2026 -0300

    chore: remove .mailmap file

commit 89ac9b9
Author: renatocaliari <renatoac82@gmail.com>
Date:   Thu Mar 5 12:51:15 2026 -0300

    feat: simplify publish UX and cleanup repository docs (v0.8.8)

commit 2060532
Author: renatocaliari <renatoac82@gmail.com>
Date:   Wed Mar 4 08:41:19 2026 -0300

    feat: cross-platform paths + README improvements

    - Add platformdirs for cross-platform support
      - Linux: ~/.local/share/agent-sync, ~/.config/agent-sync
      - macOS: ~/Library/Application Support/agent-sync, ~/Library/Preferences/agent-sync
      - Windows: ~\AppData\Roaming\agent-sync, ~\AppData\Local\agent-sync
    - Update sync.py, config.py, cli.py to use platformdirs
    - Add CLI --help output to README
    - Add 'Most Used Commands' section to README
    - Add 'Agent Configuration Paths' section to README
    - Add 'Reconfiguration' section to README
    - Remove docs/ directory (content moved to README)
    - Reduce README dependencies on external docs files

    Net: -1161 lines (docs), +129 lines (README)

    Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

commit f2b7529
Author: renatocaliari <renatoac82@gmail.com>
Date:   Tue Mar 3 17:07:50 2026 -0300

    refactor: remove scrubber logic, configs sync as-is

    Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

commit 67f4114
Author: renatocaliari <renatoac82@gmail.com>
Date:   Tue Mar 3 16:28:57 2026 -0300

    chore: remove unnecessary agent skill directories (only agent-sync needed)

    Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

commit 8a9f31a
Author: renatocaliari <renatoac82@gmail.com>
Date:   Tue Mar 3 14:35:17 2026 -0300

    docs: translate all docs to English, update to current state

    Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> minus 100644 blob bddd999	.coverage
040000 tree 9b9db11	.github
100644 blob 49b73be	.gitignore
040000 tree 21e8113	.jules
040000 tree 1dfa1ff	.opencode
040000 tree 58ef408	.plannotator
100644 blob c8ee592	.pre-commit-config.yaml
100644 blob 3eb1bdd	AGENTS.md
100644 blob 060f55c	CHANGELOG.md
100644 blob 6c28fbc	LICENSE
100644 blob 8e9ceff	Makefile
100644 blob 52c658e	README.md
100644 blob dac9516	autoresearch.ideas.md
100644 blob b5e57e3	cali-product-workflow.json
100644 blob 35f8e39	context.md
040000 tree 9196096	docs
040000 tree 3ee5f0c	examples
100644 blob d4cb3e1	index.html
100644 blob 0b76172	mkdocs.yml
040000 tree 3e2d4a5	node_modules
100644 blob 7833901	package-lock.json
100644 blob eea0897	package.json
100644 blob 9ad8275	progress.md
100644 blob b5fa8e5	pyproject.toml
040000 tree c1b9f91	scripts
040000 tree 5439234	site_docs
040000 tree ee16991	skills
040000 tree 28200d5	src
040000 tree 429afbc	tests
100644 blob 4fc2faa	uv.lock
= skills that were deleted and NOT re-added.

This is simpler, more robust, and requires no manual file management:

- Skill deleted from hub AND repo (via push) → ever_deleted has it,
  HEAD doesn't → retired → never re-imported
- Skill re-added to hub → push puts it in HEAD → NOT retired → syncs
  as normal
- Skill never deleted → never retired

Removed ~994 lines across 11 files:
- Deleted RETIRED.md file from ~/.agents/skills/
- Removed _parse_retired_manifest() from skills.py
- Removed RETIRED_MANIFEST from paths.py
- Removed in_manifest field from SkillAuditRow / SkillExplanation
- Removed manifest-related statuses (retired_clean, conflict_*)
- Removed manifest column from audit table
- Removed manifest state from explain command
- Removed manifest filter from _prune_orphan_skills
- Removed manifest filter from _stage_skills
- Removed docs/skills-lifecycle.md
- Deleted tests/test_skills_retirement.py (13 tests)
- Simplified test_skills_audit.py, test_skills_logic.py,
  test_sync_loop.py

Left unchanged: _sync_from_repo still filters by retired (git-history
based), _find_orphans still filters by retired. These prevent
re-importing skills the user intentionally removed.

Fixed _get_retired_skill_names to reference paths.REPO_DIR instead
of the removed SyncManager.DEFAULT_REPO_DIR class attribute.

551/551 tests pass (was 574 with manifest, 23 manifest tests removed).
…firmation

The pre-confirmation orphan warning now appears BEFORE the user presses
Enter, not after. The user sees:
  1. Diff with D entries for deleted skills
  2. Yellow warning listing orphans (pre-confirmation)
  3. '[Enter] push, [q] quit' prompt
  4. User decides

AGENTS.md now has a 35-line 'Skill Lifecycle' section documenting the
git-history-based retirement flow, the 3 concrete flows (remove, re-add,
prune), and key implementation details.

551/551 tests pass.
- _stage_skills() now checks git ls-tree HEAD before deleting repo
  working tree skills not in hub. Skills tracked by HEAD are preserved.
- _sync_from_repo() now respects dry_run parameter.

Breaks the cycle: hub accidentally loses skills → push propagates
loss → skills retired forever. Now push is additive-safe by default.
…orphans

- Adds orphan warning in normal push flow when there are changes
  (previously only shown in 'no changes' and '--strict' paths)
- Replaces duplicate _warn_about_orphans definitions with single fn
- Updates message to reflect HEAD guard: orphans PRESERVED by default
- Adds warning log when HEAD git ls-tree fails (was silent)
- Removed deprecated publish group entirely (no legacy code)
- Updated test_publish_private_sync.py docstrings and comments
- Updated test_repos_commands.py invocations from publish to share
- Renamed help category from 'Share & Publish' to 'Share'
- All 551 tests passing
- Extracted _internal_backup_flow() with full push logic
- push delegates to _internal_backup_flow
- backup also calls _internal_backup_flow
- Fixes 'Context.__init__() got unexpected keyword argument dry_run' error
- Removed push CLI command (decorators + function)
- Removed push from help categories
- Removed all legacy alias mentions from README
- Updated tests to use backup instead of push
- Only backup and share commands remain
…dead code

- Remove 'state.json' from EXCLUDE_PATTERNS: too generic, caused false
  positives (e.g. agentmemory-snapshots/state.json was excluded, breaking
  memory backups). User can still exclude specific state.json paths via
  sync.exclude in config.
- Remove 21 lines of unreachable code in _should_exclude: leftover from
  commit 0a4d11e that replaced the function body but left the old version
  as dead code below the new return statement.

All 551 existing tests pass.
Three related fixes for the backup stage's exclude logic:

1. **Custom user 'sync.exclude' now recurses into subdirs.** Previously the
   user-supplied pattern list used a bare fnmatch() call, so
   'exclude: ["node_modules/"]' only matched the literal 'node_modules'
   filename — not 'node_modules/foo.js' or 'a/node_modules/b.js'. The
   hardcoded EXCLUDE_PATTERNS had special-case recursion for
   'pattern.endswith("/")' but the user list did not. Refactored to a
   single _matches_pattern() helper used by both lists, supporting:
   - 'node_modules/' (with slash) — recursive
   - 'node_modules'  (bare name) — recursive (convenience)
   - '**/name'      (any depth) — recursive
   - '*.bak', '**/*.lock' — standard globs as before
   - bare 'name' (without glob chars) — recursive in subdirs

2. **Add '.git/' to hardcoded EXCLUDE_PATTERNS.** Stops nested VCS
   metadata from leaking into backups (e.g. agentmemory-snapshots/.git/
   was being copied into the private repo, creating git-in-git).

3. **Update CHANGELOG.md with the v0.40.0-alpha and v0.41.0-alpha
   entries** that were missing (last entry was v0.32.0-alpha from 12
   days ago, with 7 intermediate alpha tags undocumented).

Tests: 8 new in TestExcludePatternMatching class covering recursion,
default .git/ exclusion, state.json preservation, glob behavior, and
defensive empty-pattern handling. 559/559 passing (was 551).
…tion

agent_discovery.py: replace literal '/Users/cali/.pi/agent/AGENTS.md' in
dataclass field example and get_available_agents() docstring with portable
'Path.home() / ".pi/agent/AGENTS.md"' form.

tests/test_publish_agents.py: same fix for portability across CI runners
(ubuntu-latest doesn't have /Users/cali).

agent_registry.yaml: annotate extra_paths section explaining the pi.dev
~/.pi/ → ~/.pi/agent/ migration. All keys are wired to code via
Agent._get_extra_paths() — do not remove. Inline comments mark which
fallbacks are legacy-only vs. still-active.
…mlink

~/.pi/extensions and ~/.pi/themes are now symlinks to
~/.pi/agent/{extensions,themes} on this machine, so the dual-path
entries in extra_paths caused:
- Double-load risk in pi (same content at two paths)
- Double-backup in private repo (global_extensions/ + global_themes/ mirrored)
- Drift risk between canonical and legacy copies

Removed:
- global_extensions key (full)
- global_themes key (full)
- Second list entry from extensions and themes (no longer needed)

Kept (real content at root, no symlink):
- global_prompts (~/.pi/prompts — currently empty but root-level)
- global_skills_local (~/.pi/skills — legacy hub, may be populated)
- pyrightconfig (~/.pi/pyrightconfig.json — root file with content)

Other agents (claude-code, gemini-cli, qwen-code) untouched: only pi.dev
ever referenced ~/.pi/.

Context doc: replaced literal /Users/cali/.pi/agent/AGENTS.md with
portable Path.home() form.
The stage function _stage_pi_extra_paths had a category_map that omitted
'prompts' and 'themes', while the matching restore function
_restore_pi_extra_paths listed both. Result: any content added to
~/.pi/agent/prompts/ or ~/.pi/agent/themes/ was never backed up to the
private repo, but restore expected to find it there.

Real-world impact: themes/opencode.json on disk (mtime Jun 21) was not
being refreshed in the private repo, leaving a stale copy from May 21
that would have been restored on a rollback.

Fix: add 'prompts' and 'themes' entries to the stage category_map.

Added test_sync_stage.py with focused coverage:
- prompts/ dir staged to configs/pi.dev/prompts/
- themes/ dir staged to configs/pi.dev/themes/
- extensions/ coverage unchanged (regression guard)
Symlinked ~/.pi/prompts -> ~/.pi/agent/prompts, then reduced registry
to single entry (matching extensions/themes treatment). Removed the
global_prompts key entirely.

Without this, ~/.pi/prompts content would still be backed up to BOTH
configs/pi.dev/prompts/ and configs/pi.dev/global_prompts/ — same
dual-path duplication bug that was fixed for extensions/themes in
commit 48f70a5.
Two changes:

1. Clarify 'No Symlinks' mandate scope. The rule was about agent-sync's
   sync code (use Native/Config/Copy methods, not symlink fallbacks).
   User-side FS symlinks to avoid dual-path duplication are out of
   scope. Without this clarification, the recent symlinks at
   ~/.pi/{extensions,themes,prompts} could be misread as violations.

2. Add 'Dev Workflow' section after Architecture Mandates. Documents
   how to verify the editable install is active (which agent-sync) and
   the override recipe when the system install shadows local edits.
   This is the workflow gap that allowed the recent 'tests pass
   against OLD code' incident to go undetected for several commits.
Two pre-commit config fixes uncovered while validating the venv setup:

1. The run-tests hook was 'python -m pytest' which used whatever python
   was on PATH. If user was not in the venv, this would test the
   SYSTEM-installed agent_sync package (v0.42.0a0, old code) rather
   than local source edits. Same trap that the CLI symlink fix
   addressed. Changed entry to '.venv/bin/python -m pytest' for
   unambiguous behavior.

2. The pygrep-primary repo (https://github.com/pycqa/pygrep-primary)
   no longer exists upstream. Its presence in the config blocked
   pre-commit initialization for ALL hooks. Removed the dead entry;
   ruff-pre-commit (already present) covers lint needs.

Also fixed: the local repo block had 'rev: v0.0.1' which is invalid
for repo: local and was causing 'InvalidConfigError' on
pre-commit run. Removed.
… orphans

- prune_skills CLI was calling git commit without staging
  the deletion via _prune_orphan_skills, causing 'nothing to commit' error
- SyncManager.push now warns about orphan skills when
  no changed files exist, so share run auto-sync doesn't silently ignore
  them
- Added test_push_returns_empty_list_when_no_changes_but_orphans
- Added TestSkillsPruneFlags smoke tests for --dry-run and --yes
- Fixed stale install: added prompts/themes to category_map
- Fixed pre-existing callable | None annotation crash on Python 3.14
Adds hooks/ to pi.dev extra_paths so ~/.pi/agent/hooks/ (hooks.yaml
and auto-install configs) is backed up to the sync repo.

- agent_registry.yaml: hooks extra_path entry
- base.py: hooks_paths property
- sync.py: hooks in stage + restore category maps
- test_sync_stage.py: coverage test for hooks dir staging
Summary:
- Rewrote `_ignore_func` in `git_publish.py` to use `fnmatch.filter` for robust pattern matching, ensuring sensitive directories like `sessions` and `cache` are correctly excluded from publish.
- Replaced `$` with `\Z` in skill name validation regexes in `local_source.py` and `external_source.py` to prevent newline injection bypass.

Severity: MEDIUM
Impact: Prevents accidental publication of private data and prevents malicious skill name injection.
Verification: Verified with `pytest` and targeted reproduction scripts.

Co-authored-by: renatocaliari <1719651+renatocaliari@users.noreply.github.com>
@google-labs-jules

Copy link
Copy Markdown
Contributor

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request improves skill name validation by updating regex patterns to use \Z instead of $ to prevent newline injection, and refactors the git ignore function to use fnmatch for robust pattern matching. A review comment suggests optimizing the ignore function by pre-compiling the patterns into regular expressions to avoid redundant checks and intermediate allocations.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +34 to +40

def _ignore(path, names):
ignored = []
for name in names:
for pattern in patterns:
if pattern.startswith('*.'):
if name.endswith(pattern[1:]):
ignored.append(name)
break
elif pattern.startswith('.'):
if name == pattern or name.startswith(pattern.rstrip('/') + '/'):
ignored.append(name)
break
return ignored
ignored = set()
for pattern in patterns:
# Use fnmatch for robust pattern matching (handles *, ?, [seq])
ignored.update(fnmatch.filter(names, pattern))
return list(ignored)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

We can optimize the ignore function by pre-compiling the patterns into regular expressions once when _ignore_func is called, rather than repeatedly calling fnmatch.filter for every directory in the tree. This also allows us to short-circuit (break early) as soon as a filename matches any pattern, avoiding redundant checks and intermediate set/list allocations.

Suggested change
def _ignore(path, names):
ignored = []
for name in names:
for pattern in patterns:
if pattern.startswith('*.'):
if name.endswith(pattern[1:]):
ignored.append(name)
break
elif pattern.startswith('.'):
if name == pattern or name.startswith(pattern.rstrip('/') + '/'):
ignored.append(name)
break
return ignored
ignored = set()
for pattern in patterns:
# Use fnmatch for robust pattern matching (handles *, ?, [seq])
ignored.update(fnmatch.filter(names, pattern))
return list(ignored)
compiled_patterns = [re.compile(fnmatch.translate(p)) for p in patterns]
def _ignore(path, names):
ignored = []
for name in names:
for regex in compiled_patterns:
if regex.match(name):
ignored.append(name)
break
return ignored

@calionauta calionauta closed this Jul 10, 2026
@calionauta
calionauta deleted the fix/security-enhancements-publish-8705118332549494391 branch July 10, 2026 14:04
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.

1 participant