aws-samples / aws-samples/sample-analytics-agent-progressive-disclosure

No data classification inventory: masking surface is opt-in by name; add a seventh 'governance coverage' check to reconcile

Open
#8 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
1
Forks
2
Avg merge
2d 10h
Merged PRs (30d)
3

Description

## Context

The two-layer governance model (GRANT + dynamic data masking) works. I negative-tested it item by item through a cross-account read-only audit role, and **every boundary held as designed**:

```
users.email → ***@masked.invalid
users.phone → 103****3386 (first 3 + last 4 kept)
user_profiles.birth_date → 1999-01-01 (DATE_TRUNC to year)
SELECT count(*) FROM user_messages → permission denied ✅
SELECT count(*) FROM orders_backup_20251201 → permission denied ✅
SELECT count(*) FROM tmp_campaign_roi_analysis → permission denied ✅
```

The gap is not in the mechanism. It is in **how the coverage was decided**.

## Problem: no data classification inventory, so the masking surface is opt-in by name rather than by survey

`database/redshift/04_governance.sql` attaches 3 policies covering 3 columns. There is no document in the repo explaining how those 3 were chosen — `grep -rniE 'data classification|classification|sensitive column'` finds nothing.

I surveyed the whole database by column-name pattern (`information_schema.columns`, 48 tables) and got 57 columns whose names suggest personal information. Several of them return plaintext to the same role:

| Column | Returned | Note |
|---|---|---|
| `events.ip_address` | `10.7.95.97` | IP is personal data under most privacy frameworks |
| `user_devices.push_token` | `tok_282700` | Device push token; usable for targeted push |
| `users.username` | `user_137681` | User identifier |
| `orders.shipping_address` | `{"province":"广东","city":"深圳"}` | Shipping address (currently city-level only) |
| `posts.content` / `post_comments.content` | `内容正文 196405` | User-generated content |

**All of these are synthetic placeholder values in this dataset, so there is no real exposure today** (`ip_address` is in the `10.x.x.x` private range, `push_token` is `tok_`, the address has only province and city). I am not filing a PII leak. The point is: **anyone who adapts this demo to their own real database will copy those 3 policies, and columns like `events.ip_address` will never come to mind**. Given that the project's value proposition is precisely "governance pushed down into the warehouse, and verifiable", *how you decide which columns to govern* should itself be a verifiable step.

## Suggestions

**1. Add a data classification inventory so the masking surface can be reconciled.**

For example `database/redshift/data_classification.yaml`, annotating each column with a `level` (public / internal / pii / restricted), a `treatment` (none / mask / no_grant), and **the reason**. `schema_manifest.yaml` is already a mature instance of exactly this declare → validate → generate pattern, so it can be followed directly.

**2. Add a seventh check category to `reconcile.py`: classification inventory ⟷ actual governance state.**

The existing six (A no card / B card column absent from Glue / C DDL column absent from Glue / D metric column absent / E DDM not effective / F catalog unreadable) all verify *whether metadata has drifted*. What is missing is a category for **whether governance covers every column it should**:

- inventory says `pii` but no matching policy in `svv_attached_masking_policy` → report
- inventory says `restricted` but the role can still SELECT it → report
- a column exists in the database but is not registered in the inventory → report (this one matters most: **new columns are unprotected by default**, and every CTAS rebuild of the derived layer can introduce them)

Then "adding a table or a column" requires registering its classification or reconciliation goes red — structurally identical to today's "adding a derived table requires a manifest entry or `render.py` fails."

**3. One wording fix.**

`database/redshift/04_governance.sql:49` reads "detail tables: needed for querying and exploring definitions, so grant SELECT. **PII columns are backstopped by layer 2 masking.**" Given that only 3 columns carry policies, that sentence reads as "PII in the detail tables is covered", when it actually means "the 3 named columns are covered." Suggest either enumerating the covered columns, or pointing at the classification inventory once it exists.

## One observation worth recording (not a defect)

Querying `svv_attached_masking_policy` and `information_schema.table_privileges` as the read-only role returns **0 rows, not an error**. Both views filter by what the current identity can see; the audit role is neither the policy owner nor the grantee itself, so it sees nothing.

That is reasonable in itself, but it means **the L4 governance checks in `docs/test-plan-v2.md` (expecting "3 policy rows", "45 tables", "user_messages 0 rows") can only be run with admin credentials**. The read-only access guide sent to external reviewers does not mention this, so a reviewer who attempts L4 will get 0 rows and may conclude governance is not in effect. Suggest noting it there, and documenting "use negative tests instead of view queries" as the correct way to verify governance under the audit role (i.e. the three `permission denied` results and three masked values shown above).

Contributor guide

Open the contributing guide

Research direction

Start with database/redshift/reconcile.py and compare its six existing check categories with schema_manifest.yaml and database/redshift/04_governance.sql. Review docs/test-plan-v2.md and the read-only access guidance for the L4 assumptions. Done means the classification inventory, reconciliation coverage checks, governance wording, and audit-role verification guidance agree.

Written by the indexing model from the issue text.

Assessment

Tech stack
python, yaml
Domain
databases, documentation, security, testing
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.