Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 10 additions & 11 deletions .github/skills/developer-internals/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,18 @@ graph LR
WF[Workflow] --> CV[Centralized Validation]
WF --> DV[Domain-Specific Validation]
CV --> validation.go
DV --> strict_mode.go
DV --> strict_mode_validation.go
DV --> strict_mode_permissions_validation.go
DV --> pip.go
DV --> npm.go
DV --> expression_safety.go
DV --> expression_safety_validation.go
DV --> engine.go
DV --> mcp-config.go
```

### Centralized Validation

**Location:** `pkg/workflow/validation.go` (782 lines)
**Location:** `pkg/workflow/validation.go` (core compile-time checks)

**Purpose:** General-purpose validation that applies across the entire workflow system

Expand All @@ -61,11 +62,11 @@ graph LR

### Domain-Specific Validation

Domain-specific validation is organized into separate files:
Domain-specific validation is organized into separate files in `pkg/workflow/`:

#### Strict Mode Validation

**Files:** `pkg/workflow/strict_mode.go`, `pkg/workflow/validation_strict_mode.go`
**Files:** `pkg/workflow/strict_mode_validation.go` and the `strict_mode_*.go` validators

Enforces security and safety constraints in strict mode:
- `validateStrictPermissions()` - Refuses write permissions
Expand All @@ -77,9 +78,7 @@ Enforces security and safety constraints in strict mode:

**File:** `pkg/workflow/pip.go`

Validates Python package availability on PyPI:
- `validatePipPackages()` - Validates pip packages
- `validateUvPackages()` - Validates uv packages
Validates Python package availability on PyPI.

#### NPM Package Validation

Expand All @@ -89,16 +88,16 @@ Validates NPX package availability on npm registry.

#### Expression Safety

**File:** `pkg/workflow/expression_safety.go`
**File:** `pkg/workflow/expression_safety_validation.go`

Validates GitHub Actions expression security with allowlist-based validation.
Validates GitHub Actions expression security with allowlist-based validation. The matching test coverage lives in `pkg/workflow/expression_safety_test.go`.

### Validation Decision Tree

```mermaid
graph TD
A[New Validation Requirement] --> B{Security or strict mode?}
B -->|Yes| C[strict_mode.go]
B -->|Yes| C[strict_mode_validation.go]
B -->|No| D{Only applies to one domain?}
D -->|Yes| E{Domain-specific file exists?}
E -->|Yes| F[Add to domain file]
Expand Down
37 changes: 18 additions & 19 deletions .github/skills/error-pattern-safety/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,23 +72,25 @@ With the JavaScript global flag (`/pattern/g`), zero-width matches can cause inf

## Validation Tests

All error patterns must pass these tests:
All error patterns must pass the same safety checks used by the repo’s unit suite:

### Go Tests (pkg/workflow/engine_error_patterns_infinite_loop_test.go)
### Go tests

```go
// Test that pattern doesn't match empty string
func TestPatternSafety(t *testing.T) {
pattern := "your-pattern"
regex := regexp.MustCompile(pattern)

if regex.MatchString("") {
t.Error("Pattern matches empty string!")
}
}
```

### JavaScript Tests (pkg/workflow/js/validate_errors.test.cjs)
Run the relevant package tests with `make test-unit`.

### JavaScript tests

```javascript
test("should not match empty string", () => {
Expand All @@ -97,16 +99,17 @@ test("should not match empty string", () => {
});
```

## Safety Mechanisms in validate_errors.cjs
Use the relevant `*.test.cjs` suite under `actions/setup/js/` or `pkg/workflow/js/` for the area you changed, or run the repo’s JavaScript checks via `make test-js`.

## Safety Mechanisms in the validation layer

The `validate_errors.cjs` script has built-in protections:
The repo’s validation helpers include built-in protections for dangerous regex patterns:

1. **Zero-width detection**: Checks if `regex.lastIndex` stops advancing
2. **Iteration warning**: Warns at 1000 iterations
3. **Hard limit**: Stops at 10,000 iterations to prevent hang
1. **Zero-width detection**: Checks whether a regex stops advancing across iterations
2. **Iteration warning**: Warns when repeated runs approach a hang threshold
3. **Hard limit**: Stops execution before runaway loops can lock the process

```javascript
// Safety check in validate_errors.cjs
if (regex.lastIndex === lastIndex) {
core.error(`Infinite loop detected! Pattern: ${pattern.pattern}`);
break;
Expand Down Expand Up @@ -191,17 +194,13 @@ Pattern: `\berror\b.*` // Requires word "error"
Before committing pattern changes:

- [ ] Run `make test-unit`
- [ ] Check `TestAllEnginePatternsSafe` passes
- [ ] Check `TestErrorPatternsNoInfiniteLoopPotential` passes
- [ ] Run JavaScript tests: `cd pkg/workflow/js && npm test`
- [ ] Verify pattern matches intended error messages
- [ ] Verify pattern doesn't match informational text
- [ ] Verify the relevant engine error-pattern tests still pass
- [ ] Run the JavaScript checks for the changed area with `make test-js` or the targeted Vitest suite
- [ ] Verify the pattern matches intended error messages
- [ ] Verify the pattern does not match informational text or empty-string edge cases

## References

- Go regex syntax: https://pkg.go.dev/regexp/syntax
- JavaScript regex: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_Expressions
- Test files:
- `pkg/workflow/engine_error_patterns_infinite_loop_test.go`
- `pkg/workflow/js/validate_errors.test.cjs`
- `pkg/workflow/error_pattern_tuning_test.go`
- Current repo validation: `make test-unit` and `make test-js`
Loading
Loading