Skip to content

test: cover shell quoting and sanitize helpers - #152

Merged
muhtalipdede merged 1 commit into
kprompt:mainfrom
fredrikblau:issue-109-tests
Aug 13, 2026
Merged

muhtalipdede merged 1 commit into
kprompt:mainfrom
fredrikblau:issue-109-tests

Conversation

@fredrikblau

Copy link
Copy Markdown

Summary

  • add table-driven coverage for Tekton shellQuote edge cases
  • verify injection-shaped repository values remain quoted in generated scripts
  • cover Argo workflow/template sanitization for unsafe characters, empty input, and length limits

Testing

  • GOPROXY=direct go test ./internal/tools/argo ./internal/tools/tekton

Fixes #109

@muhtalipdede muhtalipdede left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Review

Thanks for picking up #109 — direction is right (shellQuote table + Argo sanitize coverage is the valuable part).

Blocking

  1. Rebase onto main — this branch is ~50 commits behind and currently mergeable_state: dirty.
  2. Duplicate generate-path test — main already has TestGeneratePipelineRunQuotesInjectionShapedRepo (with $(…)-shaped repo + clone/echo asserts). After rebase, drop or merge your variant into that existing test rather than adding a second same-named case. Keep the new TestShellQuote and Argo sanitize tests.

Non-blocking

  • TestSanitizeTemplateName is thin; an empty-input → "main" case would match the workflow-name coverage style.
  • As a first-time contributor, full ci / CodeQL may need a maintainer workflow approval after the rebase — happy to kick that once conflicts are gone.

Happy to approve once rebased and green.

@fredrikblau

Copy link
Copy Markdown
Author

Rebased onto current main and resolved the requested overlap.

  • preserved upstream SEC-006 generate-path coverage
  • removed the duplicate Tekton generate-path test
  • retained table-driven shellQuote coverage
  • retained Argo workflow sanitizer coverage
  • added empty-input coverage for sanitizeTemplateName

Validation: GOPROXY=direct go test ./internal/tools/argo ./internal/tools/tekton passes.

@muhtalipdede muhtalipdede left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Review

Thanks for the rebase — previous concerns addressed.

  • Duplicate Tekton generate-path test dropped; upstream SEC-006 coverage preserved
  • TestShellQuote table is solid
  • Argo sanitize coverage + empty → main for template name looks good

LGTM for #109.

@muhtalipdede
muhtalipdede merged commit 4b9b2af into kprompt:main Aug 13, 2026
1 check failed
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.

test: unit tests for shellQuote / sanitize helpers (Argo + Tekton)

2 participants