diff --git a/internal/cipolicy/workflow_policy_test.go b/internal/cipolicy/workflow_policy_test.go new file mode 100644 index 00000000..5c4142d2 --- /dev/null +++ b/internal/cipolicy/workflow_policy_test.go @@ -0,0 +1,24 @@ +package cipolicy_test + +import ( + "os/exec" + "path/filepath" + "runtime" + "testing" +) + +func TestWorkflowPolicyRegressionSuite(t *testing.T) { + t.Parallel() + + _, file, _, ok := runtime.Caller(0) + if !ok { + t.Fatal("resolve test source path") + } + repoRoot := filepath.Clean(filepath.Join(filepath.Dir(file), "..", "..")) + cmd := exec.CommandContext(t.Context(), "bash", "scripts/ci/check-workflow-secrets_test.sh") + cmd.Dir = repoRoot + output, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("workflow policy regression suite failed: %v\n%s", err, output) + } +} diff --git a/pkg/logging/sanitize.go b/pkg/logging/sanitize.go index 82b32305..cd8c7604 100644 --- a/pkg/logging/sanitize.go +++ b/pkg/logging/sanitize.go @@ -1,3 +1,23 @@ +// Copyright (c) 2025 Kapu +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +// SOFTWARE. + package logging import ( @@ -7,15 +27,6 @@ import ( "strings" ) -type sanitizeHandler struct { - inner slog.Handler -} - -func newSanitizeHandler(inner slog.Handler) *sanitizeHandler { - return &sanitizeHandler{inner: inner} -} - -// 민감 키 리스트 (case-insensitive 매칭) var ( bearerTokenRegex = regexp.MustCompile(`(?i)\b(bearer\s+)[A-Za-z0-9._~+/=-]+`) querySecretRegex = regexp.MustCompile(`(?i)([?&;](?:key|api_key|apikey|token|password|pwd|passwd|client_secret|secret|private_key|secret_key)=)[^&\s]+`) @@ -24,6 +35,8 @@ var ( const ( tokenAPIKey = "api_key" tokenAPIKeyCompact = "apikey" + tokenPrivateKey = "private_key" + tokenSecretKey = "secret_key" ) // querySecretTokens는 querySecretRegex가 매치할 수 있는 키 이름 집합으로, @@ -31,7 +44,7 @@ const ( // 입력은 반드시 이 토큰 중 하나를 case-insensitive로 포함하므로 게이트는 안전하다. var querySecretTokens = []string{ "key", tokenAPIKey, tokenAPIKeyCompact, "token", "password", "pwd", "passwd", - "client_secret", "secret", "private_key", "secret_key", + "client_secret", "secret", tokenPrivateKey, tokenSecretKey, } var sensitiveExactKeys = map[string]struct{}{ @@ -46,8 +59,8 @@ var sensitiveExactKeys = map[string]struct{}{ "client_secret": {}, tokenAPIKey: {}, tokenAPIKeyCompact: {}, - "private_key": {}, - "secret_key": {}, + tokenPrivateKey: {}, + tokenSecretKey: {}, "authorization": {}, "auth_header": {}, "cookie": {}, @@ -248,9 +261,25 @@ var secretLikePrefixes = []string{ "eyj", } +var secretLikeExactValues = map[string]struct{}{ + "access-token": {}, + "access_token": {}, + "api-key": {}, + tokenAPIKey: {}, + "private-key": {}, + tokenPrivateKey: {}, + "refresh-token": {}, + "refresh_token": {}, + "secret-key": {}, + tokenSecretKey: {}, +} + const secretLikeMinLen = 24 func isSecretLikeValue(v string) bool { + if _, ok := secretLikeExactValues[strings.ToLower(strings.TrimSpace(v))]; ok { + return true + } if hasSecretLikePrefix(v) { return true } diff --git a/pkg/logging/sanitize_handler.go b/pkg/logging/sanitize_handler.go new file mode 100644 index 00000000..ba9c6c2b --- /dev/null +++ b/pkg/logging/sanitize_handler.go @@ -0,0 +1,11 @@ +package logging + +import "log/slog" + +type sanitizeHandler struct { + inner slog.Handler +} + +func newSanitizeHandler(inner slog.Handler) *sanitizeHandler { + return &sanitizeHandler{inner: inner} +} diff --git a/pkg/logging/sanitize_literal_secret_test.go b/pkg/logging/sanitize_literal_secret_test.go new file mode 100644 index 00000000..40a67932 --- /dev/null +++ b/pkg/logging/sanitize_literal_secret_test.go @@ -0,0 +1,38 @@ +package logging + +import ( + "strings" + "testing" +) + +func TestSanitizeKey_LiteralSecretNamesAreMasked(t *testing.T) { + t.Parallel() + + for _, value := range []string{ + "secret-key", + tokenSecretKey, + "api-key", + tokenAPIKey, + "private-key", + tokenPrivateKey, + "access-token", + "access_token", + "refresh-token", + "refresh_token", + " SECRET-KEY ", + "Api_Key", + } { + value := value + t.Run(value, func(t *testing.T) { + t.Parallel() + + output := keyFieldOutput(t, "key", value) + if strings.Contains(output, value) { + t.Fatalf("literal secret value %q leaked in output: %s", value, output) + } + if !strings.Contains(output, "***REDACTED***") { + t.Fatalf("literal secret value %q was not redacted: %s", value, output) + } + }) + } +} diff --git a/pkg/obsmetrics/vec.go b/pkg/obsmetrics/vec.go index 090d65ea..a5737e99 100644 --- a/pkg/obsmetrics/vec.go +++ b/pkg/obsmetrics/vec.go @@ -25,11 +25,140 @@ type seriesEntry[T any] struct { value T } +const ( + // DefaultMaxMetricSeries bounds the process-lifetime cardinality retained by + // every vector created through the compatibility constructors. + DefaultMaxMetricSeries = 1024 + // DefaultMaxMetricLabels bounds the number of labels in one series. + DefaultMaxMetricLabels = 16 + // DefaultMaxMetricLabelNameBytes and DefaultMaxMetricLabelValueBytes bound + // allocations performed while canonicalizing attacker-influenced labels. + DefaultMaxMetricLabelNameBytes = 128 + DefaultMaxMetricLabelValueBytes = 256 +) + +// VecOptions configures hard resource limits for a metric vector. Non-positive +// values select the safe defaults; there is intentionally no unbounded mode. +type VecOptions struct { + MaxSeries int + MaxLabels int + MaxLabelNameBytes int + MaxLabelValueBytes int +} + +type seriesLimiter struct { + maxSeries int64 + maxLabels int + maxLabelNameBytes int + maxLabelValueBytes int + count atomic.Int64 + dropped atomic.Uint64 +} + +func newSeriesLimiter(options VecOptions) seriesLimiter { + if options.MaxSeries <= 0 { + options.MaxSeries = DefaultMaxMetricSeries + } + if options.MaxLabels <= 0 { + options.MaxLabels = DefaultMaxMetricLabels + } + if options.MaxLabelNameBytes <= 0 { + options.MaxLabelNameBytes = DefaultMaxMetricLabelNameBytes + } + if options.MaxLabelValueBytes <= 0 { + options.MaxLabelValueBytes = DefaultMaxMetricLabelValueBytes + } + + return seriesLimiter{ + maxSeries: int64(options.MaxSeries), + maxLabels: options.MaxLabels, + maxLabelNameBytes: options.MaxLabelNameBytes, + maxLabelValueBytes: options.MaxLabelValueBytes, + } +} + +func (l *seriesLimiter) canonicalize(labels Labels) (string, []labelPair, bool) { + if len(labels) > l.maxLabels { + l.dropped.Add(1) + return "", nil, false + } + for name, value := range labels { + if len(name) > l.maxLabelNameBytes || len(value) > l.maxLabelValueBytes { + l.dropped.Add(1) + return "", nil, false + } + } + + key, pairs := canonicalLabels(labels) + return key, pairs, true +} + +func (l *seriesLimiter) reserve() bool { + for { + current := l.count.Load() + if current >= l.maxSeries { + l.dropped.Add(1) + return false + } + if l.count.CompareAndSwap(current, current+1) { + return true + } + } +} + +func (l *seriesLimiter) release() { + l.count.Add(-1) +} + +func (l *seriesLimiter) seriesCount() int { + return int(l.count.Load()) +} + +func (l *seriesLimiter) droppedSeries() uint64 { + return l.dropped.Load() +} + +func loadOrCreateSeries[T any]( + series *sync.Map, + limiter *seriesLimiter, + key string, + labels []labelPair, + create func() T, +) (T, bool) { + var zero T + if actual, ok := series.Load(key); ok { + stored, typed := actual.(*seriesEntry[T]) + if !typed { + return zero, false + } + return stored.value, true + } + if !limiter.reserve() { + return zero, false + } + + entry := &seriesEntry[T]{key: key, labels: labels, value: create()} + actual, loaded := series.LoadOrStore(key, entry) + if loaded { + limiter.release() + } + stored, ok := actual.(*seriesEntry[T]) + if !ok { + if !loaded { + series.Delete(key) + limiter.release() + } + return zero, false + } + return stored.value, true +} + // CounterVec는 동일 metric name/help를 공유하는 label-set별 counter 집합입니다. type CounterVec struct { - name string - help string - series sync.Map // map[string]*seriesEntry[*Counter] + name string + help string + series sync.Map // map[string]*seriesEntry[*Counter] + limiter seriesLimiter } // Counter는 원자적으로 증가하는 단일 counter series입니다. @@ -38,27 +167,26 @@ type Counter struct { } func NewCounterVec(name, help string) *CounterVec { - return &CounterVec{name: name, help: help} + return NewCounterVecWithOptions(name, help, VecOptions{}) +} + +func NewCounterVecWithOptions(name, help string, options VecOptions) *CounterVec { + return &CounterVec{name: name, help: help, limiter: newSeriesLimiter(options)} } func (v *CounterVec) With(labels Labels) *Counter { if v == nil { return nil } - - key, labelPairs := canonicalLabels(labels) - entry := &seriesEntry[*Counter]{ - key: key, - labels: labelPairs, - value: &Counter{}, + key, labelPairs, ok := v.limiter.canonicalize(labels) + if !ok { + return nil } - actual, _ := v.series.LoadOrStore(key, entry) - stored, ok := actual.(*seriesEntry[*Counter]) + counter, ok := loadOrCreateSeries(&v.series, &v.limiter, key, labelPairs, func() *Counter { return &Counter{} }) if !ok { return nil } - - return stored.value + return counter } func (v *CounterVec) Inc(labels Labels) { @@ -72,6 +200,20 @@ func (v *CounterVec) Add(labels Labels, delta uint64) { } } +func (v *CounterVec) SeriesCount() int { + if v == nil { + return 0 + } + return v.limiter.seriesCount() +} + +func (v *CounterVec) DroppedSeries() uint64 { + if v == nil { + return 0 + } + return v.limiter.droppedSeries() +} + func (v *CounterVec) WriteExposition(w io.Writer) bool { if v == nil { return true @@ -79,13 +221,11 @@ func (v *CounterVec) WriteExposition(w io.Writer) bool { if !writeMetricHeader(w, v.name, v.help, "counter") { return false } - for _, entry := range collectSeries[*Counter](&v.series) { if !writeMetricSample(w, v.name, entry.labels, strconv.FormatUint(entry.value.Value(), 10)) { return false } } - return true } @@ -103,15 +243,15 @@ func (c *Counter) Value() uint64 { if c == nil { return 0 } - return c.value.Load() } // GaugeVec는 동일 metric name/help를 공유하는 label-set별 float64 gauge 집합입니다. type GaugeVec struct { - name string - help string - series sync.Map // map[string]*seriesEntry[*Gauge] + name string + help string + series sync.Map // map[string]*seriesEntry[*Gauge] + limiter seriesLimiter } // Gauge는 원자적으로 설정되는 단일 gauge series입니다. @@ -120,27 +260,26 @@ type Gauge struct { } func NewGaugeVec(name, help string) *GaugeVec { - return &GaugeVec{name: name, help: help} + return NewGaugeVecWithOptions(name, help, VecOptions{}) +} + +func NewGaugeVecWithOptions(name, help string, options VecOptions) *GaugeVec { + return &GaugeVec{name: name, help: help, limiter: newSeriesLimiter(options)} } func (v *GaugeVec) With(labels Labels) *Gauge { if v == nil { return nil } - - key, labelPairs := canonicalLabels(labels) - entry := &seriesEntry[*Gauge]{ - key: key, - labels: labelPairs, - value: &Gauge{}, + key, labelPairs, ok := v.limiter.canonicalize(labels) + if !ok { + return nil } - actual, _ := v.series.LoadOrStore(key, entry) - stored, ok := actual.(*seriesEntry[*Gauge]) + gauge, ok := loadOrCreateSeries(&v.series, &v.limiter, key, labelPairs, func() *Gauge { return &Gauge{} }) if !ok { return nil } - - return stored.value + return gauge } func (v *GaugeVec) Set(labels Labels, value float64) { @@ -150,6 +289,20 @@ func (v *GaugeVec) Set(labels Labels, value float64) { } } +func (v *GaugeVec) SeriesCount() int { + if v == nil { + return 0 + } + return v.limiter.seriesCount() +} + +func (v *GaugeVec) DroppedSeries() uint64 { + if v == nil { + return 0 + } + return v.limiter.droppedSeries() +} + func (v *GaugeVec) WriteExposition(w io.Writer) bool { if v == nil { return true @@ -157,13 +310,11 @@ func (v *GaugeVec) WriteExposition(w io.Writer) bool { if !writeMetricHeader(w, v.name, v.help, "gauge") { return false } - for _, entry := range collectSeries[*Gauge](&v.series) { if !writeMetricSample(w, v.name, entry.labels, strconv.FormatFloat(entry.value.Value(), 'g', -1, 64)) { return false } } - return true } @@ -177,7 +328,6 @@ func (g *Gauge) Value() float64 { if g == nil { return 0 } - return math.Float64frombits(g.bits.Load()) } @@ -187,33 +337,34 @@ type HistogramVec struct { help string buckets []float64 series sync.Map // map[string]*seriesEntry[*Histogram] + limiter seriesLimiter } func NewHistogramVec(name, help string, buckets []float64) *HistogramVec { + return NewHistogramVecWithOptions(name, help, buckets, VecOptions{}) +} + +func NewHistogramVecWithOptions(name, help string, buckets []float64, options VecOptions) *HistogramVec { bounds := make([]float64, len(buckets)) copy(bounds, buckets) - - return &HistogramVec{name: name, help: help, buckets: bounds} + return &HistogramVec{name: name, help: help, buckets: bounds, limiter: newSeriesLimiter(options)} } func (v *HistogramVec) With(labels Labels) *Histogram { if v == nil { return nil } - - key, labelPairs := canonicalLabels(labels) - entry := &seriesEntry[*Histogram]{ - key: key, - labels: labelPairs, - value: NewHistogram(v.buckets), + key, labelPairs, ok := v.limiter.canonicalize(labels) + if !ok { + return nil } - actual, _ := v.series.LoadOrStore(key, entry) - stored, ok := actual.(*seriesEntry[*Histogram]) + histogram, ok := loadOrCreateSeries(&v.series, &v.limiter, key, labelPairs, func() *Histogram { + return NewHistogram(v.buckets) + }) if !ok { return nil } - - return stored.value + return histogram } func (v *HistogramVec) Observe(labels Labels, value float64) { @@ -223,6 +374,20 @@ func (v *HistogramVec) Observe(labels Labels, value float64) { } } +func (v *HistogramVec) SeriesCount() int { + if v == nil { + return 0 + } + return v.limiter.seriesCount() +} + +func (v *HistogramVec) DroppedSeries() uint64 { + if v == nil { + return 0 + } + return v.limiter.droppedSeries() +} + func (v *HistogramVec) WriteExposition(w io.Writer) bool { if v == nil { return true @@ -230,20 +395,17 @@ func (v *HistogramVec) WriteExposition(w io.Writer) bool { if !writeMetricHeader(w, v.name, v.help, "histogram") { return false } - for _, entry := range collectSeries[*Histogram](&v.series) { snap := entry.value.Snapshot() if len(snap.UpperBounds) != len(snap.Cumulative) { return false } - for i, ub := range snap.UpperBounds { bucketLabels := appendLabel(entry.labels, labelPair{name: "le", value: strconv.FormatFloat(ub, 'g', -1, 64)}) if !writeMetricSample(w, v.name+"_bucket", bucketLabels, strconv.FormatUint(snap.Cumulative[i], 10)) { return false } } - infLabels := appendLabel(entry.labels, labelPair{name: "le", value: "+Inf"}) if !writeMetricSample(w, v.name+"_bucket", infLabels, strconv.FormatUint(snap.Total, 10)) { return false @@ -255,7 +417,6 @@ func (v *HistogramVec) WriteExposition(w io.Writer) bool { return false } } - return true } @@ -265,13 +426,11 @@ func collectSeries[T any](series *sync.Map) []*seriesEntry[T] { if entry, ok := value.(*seriesEntry[T]); ok { entries = append(entries, entry) } - return true }) sort.Slice(entries, func(i, j int) bool { return entries[i].key < entries[j].key }) - return entries } @@ -281,7 +440,6 @@ func canonicalLabels(labels Labels) (string, []labelPair) { for _, pair := range pairs { _, _ = fmt.Fprintf(&key, "%d:%s=%d:%s;", len(pair.name), pair.name, len(pair.value), pair.value) } - return key.String(), pairs } @@ -289,7 +447,6 @@ func labelsFromMap(labels Labels) []labelPair { if len(labels) == 0 { return nil } - pairs := make([]labelPair, 0, len(labels)) for name, value := range labels { pairs = append(pairs, labelPair{name: name, value: value}) @@ -297,6 +454,5 @@ func labelsFromMap(labels Labels) []labelPair { sort.Slice(pairs, func(i, j int) bool { return pairs[i].name < pairs[j].name }) - return pairs } diff --git a/pkg/obsmetrics/vec_security_test.go b/pkg/obsmetrics/vec_security_test.go new file mode 100644 index 00000000..e6357c97 --- /dev/null +++ b/pkg/obsmetrics/vec_security_test.go @@ -0,0 +1,96 @@ +package obsmetrics + +import ( + "fmt" + "strings" + "sync" + "testing" +) + +func TestMetricVectorsEnforceSeriesLimit(t *testing.T) { + t.Parallel() + + options := VecOptions{MaxSeries: 2} + counter := NewCounterVecWithOptions("requests_total", "Requests", options) + gauge := NewGaugeVecWithOptions("active", "Active", options) + histogram := NewHistogramVecWithOptions("latency", "Latency", []float64{1}, options) + + for i := 0; i < 3; i++ { + labels := Labels{"id": fmt.Sprintf("%d", i)} + counter.Inc(labels) + gauge.Set(labels, float64(i)) + histogram.Observe(labels, float64(i)) + } + + for name, got := range map[string]int{ + "counter": counter.SeriesCount(), + "gauge": gauge.SeriesCount(), + "histogram": histogram.SeriesCount(), + } { + if got != 2 { + t.Fatalf("%s SeriesCount() = %d, want 2", name, got) + } + } + if counter.DroppedSeries() != 1 || gauge.DroppedSeries() != 1 || histogram.DroppedSeries() != 1 { + t.Fatalf("DroppedSeries() = (%d, %d, %d), want (1, 1, 1)", counter.DroppedSeries(), gauge.DroppedSeries(), histogram.DroppedSeries()) + } +} + +func TestMetricVectorRejectsOversizedLabelsBeforeRetention(t *testing.T) { + t.Parallel() + + vec := NewCounterVecWithOptions("requests_total", "Requests", VecOptions{ + MaxSeries: 8, + MaxLabels: 1, + MaxLabelNameBytes: 4, + MaxLabelValueBytes: 4, + }) + vec.Inc(Labels{"a": "1", "b": "2"}) + vec.Inc(Labels{"long-name": "1"}) + vec.Inc(Labels{"name": strings.Repeat("x", 5)}) + + if got := vec.SeriesCount(); got != 0 { + t.Fatalf("SeriesCount() = %d, want 0", got) + } + if got := vec.DroppedSeries(); got != 3 { + t.Fatalf("DroppedSeries() = %d, want 3", got) + } +} + +func TestMetricVectorConcurrentAdmissionNeverExceedsLimit(t *testing.T) { + t.Parallel() + + const limit = 8 + vec := NewCounterVecWithOptions("requests_total", "Requests", VecOptions{MaxSeries: limit}) + var wg sync.WaitGroup + for i := 0; i < 128; i++ { + wg.Add(1) + go func(i int) { + defer wg.Done() + vec.Inc(Labels{"id": fmt.Sprintf("%d", i)}) + }(i) + } + wg.Wait() + + if got := vec.SeriesCount(); got != limit { + t.Fatalf("SeriesCount() = %d, want %d", got, limit) + } + if got := vec.DroppedSeries(); got != 128-limit { + t.Fatalf("DroppedSeries() = %d, want %d", got, 128-limit) + } +} + +func TestMetricVectorCompatibilityConstructorUsesSafeDefault(t *testing.T) { + t.Parallel() + + vec := NewCounterVec("requests_total", "Requests") + for i := 0; i <= DefaultMaxMetricSeries; i++ { + vec.Inc(Labels{"id": fmt.Sprintf("%d", i)}) + } + if got := vec.SeriesCount(); got != DefaultMaxMetricSeries { + t.Fatalf("SeriesCount() = %d, want %d", got, DefaultMaxMetricSeries) + } + if got := vec.DroppedSeries(); got != 1 { + t.Fatalf("DroppedSeries() = %d, want 1", got) + } +} diff --git a/pkg/promptguard/rulepack_compile.go b/pkg/promptguard/rulepack_compile.go index 724d605e..01b42e6b 100644 --- a/pkg/promptguard/rulepack_compile.go +++ b/pkg/promptguard/rulepack_compile.go @@ -76,6 +76,7 @@ func compileRule(rule *rawRule) (compiledRule, error) { if err := compileRuleMatcher(&compiled, rule); err != nil { return compiledRule{}, fmt.Errorf("compile matcher: %w", err) } + compiled.RequiredLiteralGroups = normalizePrefilterLiteralGroups(compiled.RequiredLiteralGroups) if err := validateCompiledRule(&compiled); err != nil { return compiledRule{}, fmt.Errorf("validate rule: %w", err) @@ -84,6 +85,26 @@ func compileRule(rule *rawRule) (compiledRule, error) { return compiled, nil } +func normalizePrefilterLiteralGroups(groups [][]string) [][]string { + normalized := make([][]string, 0, len(groups)) + for _, group := range groups { + values := make([]string, 0, len(group)) + for _, literal := range group { + value := normalizeViews(literal).Norm + if value != "" { + values = append(values, value) + } + } + if len(values) == 0 { + continue + } + slices.Sort(values) + normalized = append(normalized, slices.Compact(values)) + } + + return normalized +} + func compileRuleSelectors(rule *rawRule) (ruleSelectors, error) { action := normalizeAction(rule.Action) if strings.TrimSpace(rule.Action) != "" && action == "" { diff --git a/pkg/promptguard/rulepack_raw_security_test.go b/pkg/promptguard/rulepack_raw_security_test.go new file mode 100644 index 00000000..fd0a1095 --- /dev/null +++ b/pkg/promptguard/rulepack_raw_security_test.go @@ -0,0 +1,61 @@ +package promptguard + +import "testing" + +func TestRawRulesDoNotUseNormalizedLiteralPrefilter(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + rule rawRule + }{ + { + name: "regex compatibility characters", + rule: rawRule{ + ID: "raw-fullwidth-regex", + Family: "raw-fullwidth-regex", + Type: ruleTypeRegex, + Action: hitActionScore, + View: viewRaw, + Segments: []string{string(segmentPlain)}, + Pattern: `system`, + Weight: 1, + MaxOccurrences: 1, + }, + }, + { + name: "phrase compatibility characters", + rule: rawRule{ + ID: "raw-fullwidth-phrase", + Family: "raw-fullwidth-phrase", + Type: ruleTypePhrase, + Action: hitActionScore, + View: viewRaw, + Segments: []string{string(segmentPlain)}, + Phrases: []string{"system"}, + MatchMode: phraseMatchSubstring, + Weight: 1, + MaxOccurrences: 1, + }, + }, + } + + segment := textSegment{Kind: segmentPlain, Views: normalizeViews("system")} + if segment.Views.Raw == segment.Views.Norm { + t.Fatal("test fixture must normalize to a different representation") + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + compiled, err := compileRule(&tc.rule) + if err != nil { + t.Fatalf("compileRule() error = %v", err) + } + if matches := compiled.matchSegment(segment, compilePolicy(&rawRulepack{Version: 3}), 1); len(matches) != 1 { + t.Fatalf("matchSegment() matches = %d, want 1", len(matches)) + } + }) + } +} diff --git a/scripts/ci/check-workflow-secrets.sh b/scripts/ci/check-workflow-secrets.sh index 95b8d228..f2b3fb73 100755 --- a/scripts/ci/check-workflow-secrets.sh +++ b/scripts/ci/check-workflow-secrets.sh @@ -157,7 +157,14 @@ def structural_yaml_failures(text: str) -> list[tuple[int, str]]: failures.append((line_no, "multi-line YAML flow collections are unsupported in workflow policy checks")) kv = parse_key_value(raw) if kv is not None: - _, value = kv + key, value = kv + source = strip_yaml_comment(raw).strip() + if source.startswith("- "): + source = source[2:].strip() + if source[:1] in {"'", '"'} and key.lower() in { + "permissions", "secrets", "jobs", "uses", "with", "persist-credentials" + }: + failures.append((line_no, f"quoted policy-sensitive YAML key {key!r} is unsupported")) normalized = unquote_scalar(value) if normalized.startswith("[") and not normalized.endswith("]"): failures.append((line_no, "multi-line YAML flow collections are unsupported in workflow policy checks")) @@ -400,16 +407,18 @@ def permissions_block_is_readonly(inline_value: str, entries: list[tuple[int, st for _, raw in entries: if not meaningful(raw): continue - line = strip_yaml_comment(raw) - match = re.match(r"^\s*([A-Za-z0-9_-]+)\s*:\s*([A-Za-z0-9_-]+)\s*$", line) - if not match: - continue - key = match.group(1) + parsed = parse_key_value(raw) + if parsed is None: + return False + key, raw_value = parsed + value = unquote_scalar(raw_value) + if not re.fullmatch(r"[A-Za-z0-9_-]+", value): + return False if key in seen: return False seen.add(key) saw_entry = True - if match.group(2) not in {"read", "none"}: + if value not in {"read", "none"}: return False return saw_entry diff --git a/scripts/ci/check-workflow-secrets_test.sh b/scripts/ci/check-workflow-secrets_test.sh index 60828dbb..5c4cf56e 100755 --- a/scripts/ci/check-workflow-secrets_test.sh +++ b/scripts/ci/check-workflow-secrets_test.sh @@ -492,6 +492,48 @@ write_workflow "${secrets_string_literal}" \ " - run: echo ok" expect_success "benign secrets string literal passes" "${secrets_string_literal}" +quoted_permissions="${TMP_DIR}/quoted-permissions.yml" +write_workflow "${quoted_permissions}" \ + "name: quoted-permissions" \ + "on:" \ + " pull_request:" \ + '"permissions":' \ + " contents: write" \ + "jobs:" \ + " test:" \ + " runs-on: ubuntu-latest" \ + " steps:" \ + " - run: echo ok" +expect_failure "quoted permissions key fails closed" "quoted policy-sensitive YAML key" "${quoted_permissions}" + +quoted_secrets="${TMP_DIR}/quoted-secrets.yml" +write_workflow "${quoted_secrets}" \ + "name: quoted-secrets" \ + "on:" \ + " pull_request:" \ + "permissions:" \ + " contents: read" \ + "jobs:" \ + " call:" \ + " uses: owner/repo/.github/workflows/reusable.yml@main" \ + ' "secrets": inherit' +expect_failure "quoted secrets key fails closed" "quoted policy-sensitive YAML key" "${quoted_secrets}" + +quoted_permission_scope="${TMP_DIR}/quoted-permission-scope.yml" +write_workflow "${quoted_permission_scope}" \ + "name: quoted-permission-scope" \ + "on:" \ + " pull_request:" \ + "permissions:" \ + " contents: read" \ + ' "pull-requests": write' \ + "jobs:" \ + " test:" \ + " runs-on: ubuntu-latest" \ + " steps:" \ + " - run: echo ok" +expect_failure "quoted permission scope cannot hide write" "read-only permissions" "${quoted_permission_scope}" + if (( failures > 0 )); then echo "[FAIL] workflow secret checker tests failed: ${failures}" >&2 exit 1